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

Cargo check should allow disabling warnings #3591

Closed
Storyyeller opened this issue Jan 25, 2017 · 30 comments
Closed

Cargo check should allow disabling warnings #3591

Storyyeller opened this issue Jan 25, 2017 · 30 comments
Labels
A-lints Area: rustc lint configuration C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-check

Comments

@Storyyeller
Copy link

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
Copy link
Member

cc @nrc @jonathandturner, opinions on this?

@nrc
Copy link
Member

nrc commented Jan 25, 2017

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

@sophiajt
Copy link

Ditto. WFM

@chriskrycho
Copy link

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
Copy link
Member

sfackler commented Aug 28, 2017

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.

@carols10cents carols10cents added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-check labels Sep 29, 2017
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Nov 5, 2017

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
Copy link
Member

nrc commented Nov 5, 2017

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
Copy link

pronebird commented Jun 25, 2018

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
Copy link

brokenthorn commented Jan 13, 2019

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
Copy link

How about just putting this in your crate during development:

#![ allow( dead_code, unused_imports ) ]

@ehuss ehuss added the A-lints Area: rustc lint configuration label Nov 8, 2019
@alexkreidler
Copy link

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
Copy link
Contributor

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
Copy link

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.

gnomesysadmins pushed a commit to GNOME/librsvg that referenced this issue Nov 3, 2020
followup of !431 [1]

deny(warnings) is an anti-pattern as a newer release of
the compiler can cause working/released builds to stop
compilling for downstreams.

Having to export a RUST_FLAG is not the prettiest, but
its the best we got atm [2]

[1] https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/431
[2] rust-lang/cargo#3591
@willmcpherson2
Copy link

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

This comment has been minimized.

@jguhlin
Copy link

jguhlin commented Apr 7, 2021

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
Copy link

ghost commented May 28, 2021

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
Copy link
Contributor

djc commented Jul 25, 2021

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
Copy link
Contributor

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
Copy link

holly-hacker commented Dec 31, 2022

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
Copy link

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
Copy link
Contributor

epage commented May 24, 2023

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

@noraa-july-stoke
Copy link

noraa-july-stoke commented Sep 1, 2023

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
Copy link
Contributor

epage commented Sep 26, 2023

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

@epage epage closed this as completed Sep 26, 2023
@JeanMertz
Copy link

@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
Copy link
Member

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

There is a proposal of a new flag to control warnings: #12875

@JeanMertz
Copy link

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
Copy link
Member

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
Copy link
Contributor

epage commented Jan 11, 2024

To add, we have

  • [lints] table
  • Proposal for a --warnings <allow|warn|deny'> flag
  • A even rougher proposal for a new category of diagnostics, notices which you want to see sometimes but don't want to later turn into errors

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
Copy link

JeanMertz commented Jan 12, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: rustc lint configuration C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-check
Projects
None yet
Development

No branches or pull requests