Skip to content

Commit 660bcdb

Browse files
committed
fix: Expand the schema section
1 parent 2f1b799 commit 660bcdb

File tree

1 file changed

+53
-14
lines changed

1 file changed

+53
-14
lines changed

text/3389-manifest-lint.md

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -218,20 +218,59 @@ and other fields that are more workspace related. Instead, we used
218218

219219
## Schema
220220

221-
Instead of the `[lints]` table being `lint = "level"`, we could organize
222-
it around `level = ["lint", ...]` like some other linters do (like
223-
[ruff](https://beta.ruff.rs/docs/configuration/)). This is less verbose. We
224-
chose `lint` being the keys because
225-
- Most reasons for organizing around `level` are preference from which ecosystem they came from (Python vs JS)
226-
- Organizing around `lint` makes it harder to lose track of what section you
227-
are in for large lint lists (which exist), people do with dependency tables
228-
today
229-
- TOML isn't great for nesting complex data structures and we have allow a
230-
`priority` field and might allo other lint-level configuration. Organizing
231-
around `lint` makes this cleaner.
232-
- If we add support for packages overriding inherited workspace lints, it likely
233-
better maps to a users model to just say the package lint table gets merged
234-
into the workspace lint table. This is also easier to implement correctly.
221+
In evaluating prior art, we saw two major styles for configuring lint levels:
222+
223+
Python-style:
224+
```toml
225+
[lints]
226+
warn = [
227+
{ lint = "rust_2018_idioms", priority = -1 },
228+
{ lint = "clippy::all", priority = -1 },
229+
"clippy::await_holding_lock",
230+
"clippy::char_lit_as_u8",
231+
"clippy::checked_conversions",
232+
]
233+
deny = [
234+
"unsafe_code",
235+
]
236+
```
237+
238+
Inspired by ESLint-style:
239+
```toml
240+
[lints.rust]
241+
rust_2018_idioms = { level = "warn", priority = -1}
242+
243+
unsafe_code = "deny"
244+
245+
[lints.clippy]
246+
all = { level = "warn", priority = -1 }
247+
248+
await_holding_lock = "warn"
249+
char_lit_as_u8 = "warn"
250+
checked_conversions = "warn"
251+
```
252+
- More akin to `eslint`
253+
254+
In a lot of areas, which to choose comes down to personal preference and what people are comfortable with:
255+
- If you want to lookup everything for a level, Python-style works better
256+
- If you want to look up the level for a lint, ESLint-style works better
257+
- Python-style is more succinct
258+
- Python-style has fewer jagged edges
259+
260+
We ended up favoring more of the ESLint-style because:
261+
- ESLint-style offers more syntax choices for lint config (inline tables,
262+
standard tables, dotted keys). In general, the TOML experience for deeply
263+
nested inline structures is not great.
264+
- Right now, the only other lint field beside `level` is `priority`. In the future we may add lint configuration. While we shouldn't exclusively design for this possibility, all things being equal, we shouldn't make that potential future's experience worse
265+
- ESLInt-style makes it easier to visually highlight groups and the lints related to those groups
266+
- The cargo team has seen support issues that partially arise from a user
267+
losing track of which `dependencies` table they are in because the list of
268+
dependencies is large enough to have the header far enough away (or off
269+
screen). This can similarly happen with Python-style as the context of the
270+
level is in the table header. See [EmbarkStudios's lint list as an example of where this could happen](https://github.com/EmbarkStudios/rust-ecosystem/blob/81d62539a57add13f4b0f1c503e267b6de358f70/lints.toml)
271+
- If we add support for packages to override some of the lints inherited from
272+
the workspace, it is easier for users to map out this relationship with
273+
ESLint-style.
235274

236275
## Linter Tables vs Linter Namespaces
237276

0 commit comments

Comments
 (0)