cargo icon indicating copy to clipboard operation
cargo copied to clipboard

User control over cargo warnings

Open epage opened this issue 2 years ago • 6 comments

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:

  • #11782
  • Publish size warnings (https://github.com/rust-lang/rfcs/pull/3397#discussion_r1213330487)
  • #7380
  • Control the warning from #10910
  • #2568

Porting clippy lints to cargo

Examples of new lints:

  • #4413 (deny::detected-executable, deny::detected-executable-script)
  • #4300
  • #9058
  • Warn on old build-directive syntax (#11461)
  • Warn in more cases than #10910
  • Maybe #2489
  • Disable warning from path dependencies #8546
  • Warn on mixed case package names (#6043)
  • #7081
  • #2039 (see also rust-lang/rust-clippy#3867, deny's licenses)
  • #6003
  • #4611
  • #5890
  • #7760
  • #6827
  • Redundant package.homepage (see #13293)
  • #15578
  • #15579
  • non-latest edition
  • #14390
  • warn if two dep info files within the same package point to the same source file (#7730).
  • deny missing metadata when running cargo package https://github.com/rust-lang/cargo/issues/15398#issuecomment-2781116158

Proposed Solution

As package and workspace level lint control, turning existing warnings into diagnostics where possible

Tasks

  • [x] rustc-like diagnostics for parsing Cargo.toml (#13172)
  • [x] Report what set the lint level (see #13778 for context)
  • [x] im_a_teapot lint should be unstable (see #13797 for context) (implemented in #13805)
  • [x] Clean up forbid handling for default/edition level (see #13797 for context)
  • [x] Documentation of all supported lints similar to rustc and clippy (see #14025)
  • [ ] Needs a stable lint at the time of stabilization

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 #13740 skipped it for 2024)
    • Should we reuse rustc groups or not?
  • rustc-like diagnostics for processing Cargo.toml (ie port warnings off of shell.warn for consistency)
  • Diagnostic summary

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
  • Do we share namespaces when appropriate?
    • See https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Sharing.20of.20lints/near/475872847

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.#

epage avatar Jun 06 '23 14:06 epage

From https://github.com/rust-lang/cargo/issues/10160#issuecomment-1573883951

Just had a talk with @Muscraft. This is mostly my idea but it would be great if we have this kind of error overhaul. It will help things like JSON format message (#8283) or developing Cargo's own lint rules with -Zlints (#12115). It would also open a door to diagonostics translation (rust-lang/rust#100717). Probably the modularization of Cargo could benefit from it as well.

epage avatar Jun 06 '23 14:06 epage

imo we need structured diagnostics which we pass to the diagnostic system which will then be turned into errors or something else

epage avatar Jun 06 '23 14:06 epage

An obvious way to implement this is to shove the lint config in and our of Config as we enter and leave the relevant contexts. This is a bit more brittle and lowers our chances for doing some operations with threading as Config needs to be mutable everywhere

One thought for how we could implement this

  • Rename Config to Context (we might want to do this anyways)
  • Refactor Context to allow decorating it (wrapping it)
  • Whenever we start down a workspace or package specific code path, we decorate the Context with that workspace or package's lint configuration

epage avatar Jun 06 '23 15:06 epage

One thought I had from #13621 was more about the linting system as a whole.

Should we use cargo::rust_2024_compatibility group or should we key off of rust::rust_2024_compatibility group?

epage avatar Mar 25 '24 18:03 epage

Should we use cargo::rust_2024_compatibility group or should we key off of rust::rust_2024_compatibility group?

What is the advantage of that? I can think of a better coherent view of lints. But if external tools don't know about cargo lints, then the behavior might be inconsistent. (Just an imaginary situation, don't know if they really exist.)

From another angle, it looks like Cargo goes beyond Cargo and starts overriding other tools' defaults. This might need a cross-team discussion.

weihanglo avatar Mar 25 '24 19:03 weihanglo

Take unused_lints. iirc rust::unused_lints affects both rustc and clippy.

So the question would be

  • Is rust::unused_lints global or specialized for rustc linters
  • If not global, does that mean cargo::unused_lints should affect all Cargo.toml linters (e.g. if we moved Cargo lints from clippy-driver to cargo clippy)

Or put another way, does a user say "I want to see 2024 Edition compatibility warnings" or do they say "I want to see Rust 2024 Edition compatibility warnings separately from Cargo 2024 Edition compatibility warnings". The Edition is the same concept, so it seems weird to split it by tool. However, rustfmt does have a distinct Edition so maybe we should keep it more loose generally?

epage avatar Mar 25 '24 21:03 epage