rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.66k stars 2.4k forks source link

Cargo check should allow disabling warnings #3591

Closed Storyyeller closed 1 year ago

Storyyeller commented 7 years ago

There are a lot of compiler warnings that are noisy and useless during development, such as dead_code. Unfortunately, cargo check does not have any way of disabling these warnings. There needs to be a way to disable specific warnigns when running cargo check.

So far, the only workaround I've found is export RUSTFLAGS='-A dead_code', which is ugly and undiscoverable, and you have to remember to undo it when you are ready for release.

alexcrichton commented 7 years ago

cc @nrc @jonathandturner, opinions on this?

nrc commented 7 years ago

I would not object to having this. Its not something that I've noticed being super-painful though.

sophiajt commented 7 years ago

Ditto. WFM

chriskrycho commented 7 years ago

I'll give an example of where I'd find it useful: I'm building up a library and I have a bunch of code I know I'll be using, because I've written them to use elsewhere in the library. But I get dead-code warnings constantly because I haven't written those implementations yet, and it's just noise while I'm working on fixing a broken test, or on seeing actual build errors elsewhere in the library, etc.

sfackler commented 7 years ago

On the other side of things, I leave warnings as warnings when working on a crate, but then want to turn them into errors on CI. cargo build -D warnings is a bit nicer than env RUSTFLAGS="-D warnings" cargo build IMO. It's way easer to see them pop up in cargo build --help than knowing about RUSTFLAGS. Even if you did know about it, I don't want to forbid warnings on dependencies and didn't realize that cargo's lint capping would still work with RUSTFLAGS for a while.

nikomatsakis commented 6 years ago

I strongly want this too. I came to the repo looking to file an issue requesting cargo build -A warnings, but I think what I really want is for both cargo check and cargo build to support -A and -D. I think I would not want to change the defaults for either of them.

nrc commented 6 years ago

If we do anything, we should do it for both check and build, and I think we should not change the defaults. But I think it would be nice to have the option (I've recently been doing more experimental work, and I think I'd like this).

pronebird commented 6 years ago

That would be awesome for cargo build too. I am seeing lots of warnings recently due to some changes in doc comments. This totally clogs the output and there is no way to find actual errors without going through the 20 screens of warnings.

brokenthorn commented 5 years ago

Where I find it annoying is when the output is not colored (ie. IDE terminal doesn't support it) and I just want to check and fix errors quickly when making incremental changes to code or when I'm just testing or playing around, like in a "test-some-posibility" branch. It's a bit harder on my eyes to stop at the first error when warnings are printed first. I have to traverse all the warnings first.

najamelan commented 5 years ago

How about just putting this in your crate during development:

#![ allow( dead_code, unused_imports ) ]
alexkreidler commented 4 years ago

I also saw this: cargo test 2>/dev/null, which might work similarly for cargo bulid.

Source: https://users.rust-lang.org/t/suppress-warnings-from-the-cargo-command/10536/6

nikomatsakis commented 4 years ago

Note that we have RFC 2383 which proposed a way to "expect" a lint that is actually what I really want during development (a way to say "I know this code is dead, I expect that, and warn me if it becomes not dead so I can remove the annotation").

panstromek commented 4 years ago

This is what I have been doing recently - this will catch compile errors with enough info to fix them and it silences warnings but still says how many warnings there are.

cargo check 2>&1 | rg -i --multiline "(^error.*\n.*)|(aborting)|(warnings)"

cargo check -A warnings would be amazing. It really is noisy and scrolling through hundreds of warnings to find an error is really annoying.

willmcpherson2 commented 3 years ago

Thought I'd add my solution.

I use this check alias so I can just check if my code compiles without a bunch of warnings. It doesn't affect normal invocations of cargo check.

alias check='RUSTFLAGS=-Awarnings cargo check'
randall-coding commented 3 years ago

4 years later still the same.

jguhlin commented 3 years ago

Cargo command-line disabling of warnings would provide better ergonomics for many workflows. Windows console scrolls very slowly, and my IDE already displays warnings in-line, cargo check/test have no need to display them. My workflow is to have cargo watch -x test running in one terminal, and rust-analyzer in the IDE. The redundancy and lag are not desirable here.

ghost commented 3 years ago

Maybe using Cargo.toml for putting the configuration of such things could be useful. Python does this with [tool.TOOLNAME] in pyproject.tom, to consolidate the configuration of tooling in a single file. It's useful imo.

In this case it could be

[tool.rustc]

allow_warnings = "dead_code"

Edit: or add the configuration option to .cargo/config.toml.

At the moment it can be indirectly configured with

[target.'cfg(target_family = "unix")']
rustflags = [
    "-Adead_code"
]
djc commented 3 years ago

My proposal would be to add options like clippy has for build-like commands (check, build, etc):

    -W --warn OPT       Set lint warnings
    -A --allow OPT      Set lint allowed
    -D --deny OPT       Set lint denied
    -F --forbid OPT     Set lint forbidden

(Then we could also have these directly in cargo clippy rather than as arguments after --.)

I'd be willing to implement this if a Cargo team member approves of the plan.

cyqsimon commented 1 year ago

I would love to see this feature added.

I'd also like to point out that using RUSTFLAGS to disable/allow a lint causes all dependencies to be recompiled, which is completely overkill for this purpose since the compiled binary stays the same. Doubled with rustc's infamously long compile times, this gets annoying quickly for larger projects.

A potential solution to improve the UX is to suppress and/or level-set the messages on cargo's level, i.e. an additional layer of message filtering and/or mapping. This way rustc still sees the same inputs and doesn't do unnecessary work. Though I'm not familiar with how cargo talks with rustc so I'm not sure how much work this will entail.

holly-hacker commented 1 year ago

I would expect cargo's -q flag to do this. Currently, it will only quieten the "Finished ... target(s) in ..." and "Running ..." lines, but this could be extended to include non-fatal warnings.

Edit for clarity due to downvotes: I often need to check the commandline output of my programs and the clutter from warnings distracts from that. cargo run -q is an easily discoverable way to temporarily turn that off and focus only on only the output of the program. I hope that the accepted solution is equally discoverable for newer users.

brokenthorn commented 1 year ago

While it would be great to have an option to remove the visual clutter that happens when you have both compile errors and warnings, where you try to scan the errors out of the bunch, I think it would be nice to not completely remove the information that you have warnings, because those warnings should not be overlooked.

For example, we could see a summary on the last tine with a count of the number of warnings, even if those warnings are not displayed.

epage commented 1 year ago

12115 adds a new [lints] table for controlling lint levels.

noraa-july-stoke commented 1 year ago

I'm here cause I was wanting a similar thing... I wish there was a way to run something like cargo run --silence-warnings or cargo build --silence-warnings that silences all warnings except fatal errors. Maybe might print a single line telling you that warnings exist without the visual clutter of the warnings. It should still allow output logs from the compiled binary.

The main reason is that I work on some massive code bases and there's dead code and unused imports, etc everywhere on the dev branches and when I need to only focus on either actual fatal Errors. Sometimes there's so many warnings that it exceeds my terminal's max history and I can't even read the actual Errors that are thrown that my terminal that are preventing my code from compiling or running. Having to go around and put #![allow(foo_bar)] everywhere and then remove it would just take too much time.But that's currently one of my only options. Please, please, please do something about this. Not having this feature is one of the current banes of my existence :D

epage commented 1 year ago

@noraa-july-stoke for me, that sounds slightly different than the request in this issue. I'd recommend creating a dedicated issue for it (or highjacking #8424 as I could see us having a --warnings <deny|allow>).

Since #12115 is merged, I'm closing this as resolved.

JeanMertz commented 9 months ago

@epage Is the lints option expected to work with cargo --config?

I tried cargo check --config 'lints.rust.dead_code = "allow"', but I still get warnings for this lint specifically.

I'm asking, since this issue specifically asks for flags, so I was hoping #12115 would resolve that with --config. Maybe it has, and I'm just using it wrong?

weihanglo commented 9 months ago

@JeanMertz --config is for setting config values of config.toml not Cargo.toml.

There is a proposal of a new flag to control warnings: https://github.com/rust-lang/cargo/pull/12875

JeanMertz commented 9 months ago

Thank you for that insight @weihanglo. I wasn't aware --config only works for config.toml (I never had a use for it, except in this case).

The --help also doesn't make that particularly obvious:

--config <KEY=VALUE>  Override a configuration value

It doesn't look like #12875 would solve this particular issue, though, as it proposes a blanket disable/enable feature, instead of fine-grained control over which specific lint is set to which level.

I'll use RUSTFLAGS for now, but as discussed before, it triggers a recompile, and is less ergonomic, so I do feel like this issue, while related to the new lints option in Cargo.toml, isn't completely solved currently?

weihanglo commented 9 months ago

Terminology is hard. Cargo.toml is the manifest file, while config.toml is the configuration. If you run cargo help build, you'll get a more detailed help manual :)

It doesn't look like #12875 would solve this particular issue, though, as it proposes a blanket disable/enable feature, instead of fine-grained control over which specific lint is set to which level.

Does cargo clippy -- -D some_lint work for you? BTW, for further discussion, I'd recommend continuing in #8424 instead of this closed issue, or create a new one.

epage commented 9 months ago

To add, we have

If a new issue is created, I would recommend focusing on the use case for why cargo needs to expose the flags more directly to the user. If there is discussion on the solution, a comparison with the above would be beneficial to help highlight the unique need here.

Keep in mind that if a flag is added, a full recompilation will be needed so rustc can respond to the different diagnostic flag. The most we can benefit from this is that adding the flag won't flush caches of previously builds like RUSTFLAGS does. Maybe another shortcut that can be made is to limit it based on --cap-lints or hard code --cap-lints like logic and only apply it to workspace members, reducing how much has to get rebuilt.

JeanMertz commented 9 months ago

Thank you for the input @epage.

The most we can benefit from this is that adding the flag won't flush caches of previously builds like RUSTFLAGS does. Maybe another shortcut that can be made is to limit it based on --cap-lints or hard code --cap-lints like logic and only apply it to workspace members, reducing how much has to get rebuilt.

This is precisely what I'm looking for. A way to conveniently configure specific lints in specific situations (local dev, pre-commit, CI, etc.), and the behavior you describe (no cache flushing, limited to my personal crate or workspace).

None of the proposals you listed above would tackle that, but this issue seems to specifically request exactly that (e.g. have a way for cargo check (and presumably any cargo command that shows diagnostics) to be configured on a per-run basis using command-line flags.

I'll see if I can find some time over the weekend to file a new issue for this 👍.