Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Add a [lints] table to Cargo.toml #3389

Merged
merged 64 commits into from
May 9, 2023
Merged
Changes from 1 commit
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
b80c2ff
chore: Start manifest-lint RFC
epage Feb 14, 2023
28f51c3
feat: Start manifest-lint RFC
epage Feb 14, 2023
6ac518e
fix: Remove stray paren
epage Feb 14, 2023
aa8695b
fix: Pluralize the table
epage Feb 14, 2023
680fe11
fix: Make lints table top-level
epage Feb 14, 2023
054de51
fix: Call out workspace lint name reservation
epage Feb 14, 2023
3796056
fix: Link to user-defined attribute RFC
epage Feb 14, 2023
cfbd231
feat: Add a couple more brainstorming ideas
epage Feb 15, 2023
c57a510
fix: Use correct --forbid syntax
epage Feb 15, 2023
42010e8
fix: Apply ehuss' feedback
epage Feb 15, 2023
b79b792
fix: Add RFC number
epage Feb 15, 2023
4f4107f
fix: Link out to rubocop
epage Feb 15, 2023
bf6d9fc
fix: Typos
epage Feb 15, 2023
d136690
fix: Include cargo-cranky as prior art
epage Feb 15, 2023
75198c2
fix: Typo
epage Feb 15, 2023
4a267a6
fix: Be more explicit on workspace inheritance
epage Feb 15, 2023
c726cf3
feat: Add external file possibility
epage Feb 15, 2023
508e92f
feat: Add cargo lints as a future possibility
epage Feb 15, 2023
1c7ef77
fix: Note that cargo-metadata support is needed for configurable lints
epage Feb 15, 2023
4bfc545
fix: Update to lints.tool.lint
epage Feb 15, 2023
f8071a3
feat: Add open question about rustfmt
epage Feb 15, 2023
1e43928
fix: Discuss all supported lint tools
epage Feb 17, 2023
3cd6125
fix: Update for latest conversation
epage Feb 21, 2023
b02771a
fix: Typo
epage Feb 21, 2023
5a70d45
feat: Take a stab at lint precedence
epage Feb 21, 2023
08f7190
fix: Make priority signed, giving a clear center value
epage Feb 22, 2023
7d81bf1
fix: Add another reason against 'rules'
epage Feb 22, 2023
31c9587
fix: Some TOML formatting
epage Feb 22, 2023
cf9a148
fix: Clarify we are overriding lint groups
epage Feb 23, 2023
1adde1b
fix: Typo
epage Feb 23, 2023
78083ed
fix: Document future idea for lint-level source
epage Feb 23, 2023
72cdd44
fix: Typos
epage Feb 23, 2023
7073266
fix: Spelling and language
epage Feb 27, 2023
86b0b64
fix: remindme priority with multiple lint sources
epage Feb 27, 2023
a270d28
fix: Be explicit that lints does not affect dependencies
epage Feb 27, 2023
78ab70d
fix: Typo
epage Feb 27, 2023
cf93e59
fix: Expand on lint source future
epage Feb 28, 2023
e92a52b
fix: Isolate array precedence
epage Mar 7, 2023
ef224ec
fix: Document 'auto-priority' alternative
epage Mar 7, 2023
ef4a490
fix: Add missing namespacing of rust lints
epage Mar 18, 2023
86932bd
fix: Call out rust/rustc category confusion
epage Mar 24, 2023
5afa0cf
fix: Be more explicit in how the lints table is loaded
epage Mar 24, 2023
a54d985
fix: Clarified this isn't limited to rustc/clippy
epage Mar 24, 2023
de44058
fix: Explicitly call out why rust table exists'
epage Mar 24, 2023
1557767
fix: Clarify I meant lint levels, not general lint configuration
epage Mar 24, 2023
e1230cd
fix: Clarify clippy.toml isn't going away yet
epage Mar 24, 2023
2f5f873
fix: Add clarification that an example is only an example
epage Mar 24, 2023
3543967
fix: Be explicit that lint configuration is a future possibility
epage Mar 24, 2023
45766c4
fix: Update now that we have confirmation on ruff's design choice
epage Mar 29, 2023
fbf6f48
fix: Expand on why not `::` but separate tables
epage Mar 30, 2023
935593f
fix: Expand more on why not level=lint
epage Mar 30, 2023
c3f932c
fix: Remove confusion over :: and tool-config
epage Mar 31, 2023
e73e6b9
fix: Add auto-sort as a future possibility
epage Apr 10, 2023
d32801b
fix: Add high-level guidance
epage Apr 10, 2023
330782a
refactor: Break up Rationale / Alts into smaller sections
epage Apr 10, 2023
9cbc977
fix: Rewrite :: section
epage Apr 10, 2023
2f1b799
fix: Typos
epage Apr 10, 2023
660bcdb
fix: Expand the schema section
epage Apr 10, 2023
28f14ef
fix: Expand on precedence options
epage Apr 10, 2023
51f4984
fix: Move some discussion to stablization
epage Apr 18, 2023
2b10a5e
fix: Be more precise when talking about disjoint groups
epage Apr 19, 2023
4cb8421
fix: Clarify dependency situation
epage Apr 27, 2023
a47520f
fix: Call out reducing rebuilds for filtering lints
epage Apr 27, 2023
7aab0bd
Add tracking issue.
ehuss May 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: Apply ehuss' feedback
epage committed Feb 15, 2023
commit 42010e8df73990268bb25ffe36b4280dd676ab32
38 changes: 32 additions & 6 deletions text/0000-manifest-lint.md
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ See also
- [rust-lang/rust-clippy#1313](https://github.com/rust-lang/rust-clippy/issues/1313)
- [rust-lang/cargo#5034](https://github.com/rust-lang/cargo/issues/5034)
- [EmbarkStudios/rust-ecosystem#59](https://github.com/EmbarkStudios/rust-ecosystem/issues/59)
- [Proposal: Cargo Lint configuration](https://internals.rust-lang.org/t/proposal-cargo-lint-configuration/9135)

# Guide-level explanation
[guide-level-explanation]: #guide-level-explanation
@@ -117,7 +118,8 @@ When parsing a manifest, cargo will resolve workspace inheritance for

When running rustc, cargo will transform the lints from `lint = level` to
`--level lint` and pass them on the command line before `RUSTFLAGS`, allowing
user configuration to override package configuration.
user configuration to override package configuration. These flags will be
finterprinted so changing them will cause a rebuild.

**Note:** This reserves the lint name `workspace` to allow workspace inheritance.

@@ -131,6 +133,8 @@ to "undefined lint" warnings when used on earlier versions, requiring that
warning to also be suppressed, reducing its value. However, in the "Future
possibility's", we mention direct support for tying lints to rust versions.

This does not allow sharing lints across workspaces.

# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

@@ -142,6 +146,12 @@ and other fields that are more workspace related. Instead, we used

`[lints]` could be `[lint]` but we decided to follow the precedence of `[dependencies]`.

Instead of using `::` as a separator between tool and lint (e.g.
`clippy::enum_glob_use`), we could use TOML dotted keys for this (e.g.
`clippy.enum_glob_use`). This has the advantage of allowing unquoted keys at
the cost of not being able to copy/paste the lint name from the tool's output
into the fileV

We could support platform or feature specific settings, like with
`[lints.<target>]` or `[target.<target>.lints]` but
- There isn't a defined use case for this yet besides having support for `cfg(feature = "clippy")` or
@@ -164,7 +174,7 @@ inherit with `workspace = true`, we could have `[workspace.lints.<preset>]`
which defines presets and the user could do `lints.<preset> = true`. The user
could then name them as they wish to avoid collision with rustc lints.

Instead of the `[package.lints]` table being `lint = "level"`, we could organize
Instead of the `[lints]` table being `lint = "level"`, we could organize
it around `level = ["lint", ...]` like some other linters do (like
[ruff](https://beta.ruff.rs/docs/configuration/)) but this works better for
logically organizing lints, highlighting what changed in diffs, and for
@@ -192,29 +202,45 @@ Go
# Unresolved questions
[unresolved-questions]: #unresolved-questions

How does this affect fingerprinting / recompilation and how should it?

How should we hand rustdoc lint levels or, in the future, cargo lint levels?
The current proposal takes all lints and passes them to rustc like `RUSTFLAGS`
but rustdoc uses `RUSTDOCFLAGS` and cargo would use neither. This also starts
to get into
[user-defined tool attributes](https://rust-lang.github.io/rfcs/2103-tool-attributes.html).

Should we only apply/fingerprint lints for the appropriate tool? For example,
we would not include and fingerprint `clippy::` lints when running builds,
allowing them to change without forcing a rebuild. We likely already need to
be tool-aware for built-in tools to handle `rustdoc::` lints (see above) so
this isn't much more of a step.

How do we allow controling precedence between lints and lint groups? We are
using a TOML table with the keys as lint names which does not allow controlling
ordering. Even if we switched to `level = [lint, ...]`, you get a hard coded
precedence between levels that the user can't control.

# Future possibilities
[future-possibilities]: #future-possibilities

## Configurable lints

We can extend basic lint syntax:
```toml
[package.lints]
[lints]
cyclomatic_complexity = "allow"
```
to support configuration, whether for cargo or the lint tool:
```toml
[package.lints]
[lints]
cyclomatic_complexity = { level = "allow", rust-version = "1.23.0", threshold = 30 }
```
Where `rust-version` is used by cargo to determine whether to pass along this
lint and `threshold` is used by the tool. We'd need to define how to
distinguish between reserved and unreserved field names.

## Extending the syntax to `.cargo/config.toml`

Similar to `profile` and `patch` being in both files, we could support
`[lints]` in both files. This allows more flexibility for experimentation with
this feature, like conditionally applying them or applying them via environment
variables. For now, users still have the option of using `rustflags`.