Repository navigation
Description
Problem
Today, cargo maintainers are very cautious as to what warnings are created because they cannot be turned off by the user when they are intended. The problem is we've not had a mechanism to control it. With #12115, we'll now have a mechanism
Turning existing warnings into lints:
[patch]inconfig.tomltriggers unhelpful warnings #11782- Publish size warnings (Bundle local images in rustdoc output rfcs#3397 (comment))
- The "unused manifest key" warning doesn't print or halt the publishing with
cargo publish#7380 - Control the warning from Warn when an edition 2021 crate is in a virtual workspace with default resolver #10910
- Allow turning
Cargo.tomlwarnings into errors #2568 warning: doc tests are not supported for crate type(s) cdylibeven with explicitdoctest = false#15964
Porting clippy lints to cargo
- Expand manifest validation for all settings #4377 (see also clippy::cargo_common_metadata)
- Lint against non semver-open dependencies #5340 (see also clippy::wildcard_dependencies, deny::wildcard)
- Warn when new duplicate dependencies are introduced #13899 (see also clippy::multiple_crate_versions, deny::duplicate)
- Lint for negative feature names #15581
- Lint for redundant feature names #15580
Examples of new lints:
- cargo package should warn binary files by default #4413 (deny::detected-executable, deny::detected-executable-script)
cargo publishshould warn on invalid categories/keywords #4300- Warning when large binary files are included into the bundle #9058
- Warn on old build-directive syntax (Cannot extend build directives within cargo #11461)
- Warn in more cases than Warn when an edition 2021 crate is in a virtual workspace with default resolver #10910
- Maybe "cargo publish" should error if there are path overrides #2489
- Disable warning from path dependencies dependencies warning control, overriding path heuristic #8546
- Warn on mixed case package names (Publishing a crate is case-sensitive #6043)
- deprecate or warn about downloads over unencrypted HTTP #7081
- Lint
licensefield (old: SPDX dual-license inconsistency) #2039 (see also Restriction Lint: Crate License rust-clippy#3867, deny's licenses) - Cargo always rebuildsd if
rerun-if-changed=PATHandPATHpoints to none-existing file #6003 - Add a warning when enabling LTO for crates with both cargo-types that support LTO and ones that don't. #4611
- Cargo will silently ignore invalid .cargo/config options #5890
- add a warning when compiling a user crate named
core#7760 - Cargo allows you to have a different name in
[lib]and[package]when publishing #6827 - Redundant
package.homepage(see Add guidance on setting homepage in manifest #13293) - Lint if workspace dependency inheritance isn't used #15578
- Lint if workspace lints are available but not inherited #15579
- non-latest edition
includepatterns should report invalid paths #14390- warn if two dep info files within the same package point to the same source file (Spurious "error[E0583]: file not found for module" error. #7730).
- deny missing metadata when running
cargo packagecargo package(and publish) includestargetfolder #15398 (comment)
Proposed Solution
As package and workspace level lint control, turning existing warnings into diagnostics where possible
Tasks
- rustc-like diagnostics for parsing
Cargo.toml(feat: Addrustcstyle errors for manifest parsing #13172) - Report what set the lint level (see Unused dependencies cleanup #13778 for context)
-
im_a_teapotlint should be unstable (see Cleanup linting system #13797 for context) (implemented in Error when unstable lints are specified but not enabled #13805) - Clean up forbid handling for default/edition level (see Cleanup linting system #13797 for context)
- Documentation of all supported lints similar to
rustcandclippy(see Add tooling to document lints #14025) - Needs a stable lint at the time of stabilization
- Warning, error count
- Report as part of existing summaries?
-
build.warningscontrol (feat(diag): Supportbuild.warningsfor cargo lints #17112) - report all errors, not just first, in parse pass (fix(diag): Report all errors, in order #17095)
- Which sub-diagnostic the lint level reason goes into?
- Audit
unwrap - Make paths relative in the same way as rustc diagnostics (fix(diag): Give diagnostics the same display path behavior as rustc #17101)
- Incorrect suggestion-diff for lint
unused_workspace_dependencies#16982 - Lints for unstable features (made difficult by lack of runtime iteration of
-Z) (what should be the behavior of lints for runtime controlled unstable features likehint-mostly-unused, always run or only with-Z) - Document writing of lints' (docs(diag): Provide jumping off points for writing diagnostics and passes #17090)
- First group is primary, all others are secondary
- Emitted reason is last element of primary group
- Emitted reason is only emitted once per package
- (TBD) Helps without snippets may be elements of the primary group, otherwise make them secondary groups
- Never do a help as an annotation's label
- When OR-ing of help messages, use a common prefix to help clarify that for users
- If can't provide a
Snippet(e.g. in-memory manifests), at least provide anOrigin - Follow rustc's dev guide on grammar, etc
- Rules on padding?
- Deterministic order of linting packages
- How to handle false positives from
unused_dependencies(feat(lints): Emit unused_dependencies lint #16600). Started with anignored = ["dep_name], then ignoringdirect & transitive(fix(compile): Ignore unused deps if also transitive #16935, Revert "feat(lints): Add unused deps ignore list" #16937)
Non-blocking
- rustc-like diagnostics for parsing
Cargo.lock - rustc-like diagnostics for parsing
.cargo/config.toml - Deciding how to handle edition compatibility groups and groups in general (since refactor: Remove
rust_2024_compatibilitylint group #13740 skipped it for 2024)- Should we reuse rustc groups or not?
- rustc-like diagnostics for processing
Cargo.toml(ie port warnings off ofshell.warnfor consistency) - Diagnostic summary
- Paths are not relative to the same root as rustc's
Rejected
_::warningsdynamic group (rustorcargo?): specialized warning for specific workflows that is being superseded by other mechanisms
Open questions
- How many of the lints should be run as part of every cargo invocation vs run as part of an explicit "lint" step?
- See https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Cargo.20Lints/near/389419039
- Every invocation for now as decided at the All Hands
- Do we share namespaces when appropriate?
- See https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Sharing.20of.20lints/near/475872847
- No, at All Hands we decided that Cargo should operate on its own lints only
- Where should workspace-level lints run? On the workspace or on each package?
- Run them on the workspace, subject to the
workspace.lints(with a sometimes fallback tolintssimilar to*.resolver)
- Run them on the workspace, subject to the
Notes
There will still be lints that are not related to the package but related to the user or / invocation. At a later stage, we can consider user-level lint levels.
Showing lint reason once per package:
- When inherited, we could potentially show it once per workspace just like rustc would do for a crate level attribute
- Starting with this now as an MVP, we can always re-evaluate later
Deferred reducing boilerplate
Deferred json messages
Deferred: whether to make lint docs more like clippy
Out of scope: CLI control. That would be evaluated for [lints] as a whole, potentially through config. Note that existing runtime lint control is not officially supported in Cargo but done through an escape hatch
Activity
epage commentedon Jun 6, 2023
From #10160 (comment)
epage commentedon Jun 6, 2023
imo we need structured diagnostics which we pass to the diagnostic system which will then be turned into errors or something else
epage commentedon Jun 6, 2023
An obvious way to implement this is to shove the lint config in and our of
Configas we enter and leave the relevant contexts. This is a bit more brittle and lowers our chances for doing some operations with threading asConfigneeds to be mutable everywhereOne thought for how we could implement this
ConfigtoContext(we might want to do this anyways)Contextto allow decorating it (wrapping it)Contextwith that workspace or package's lint configurationfix(package): Avoid multiple package list entries
fix(package): Avoid multiple package list entries
Auto merge of #12399 - epage:normalize, r=weihanglo
Auto merge of #13172 - Muscraft:diagnostic-system, r=epage
Auto merge of #13172 - Muscraft:diagnostic-system, r=epage
Auto merge of #13533 - epage:edition-warn, r=weihanglo
21 remaining items
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints
feat(diag): Stabilize cargo-lints (rust-lang#17298)