Feature request: flag for non-zero exit status if warnings are present

[beefsack]

Hello, awesome tool and using it has been helping me learn Rust!

I've been trying to automate executing clippy from some scripts I run across a number of crates I'm maintaining at the moment, and it's tricky to check if there was any warnings from clippy from a Bash script.

I'd love if there was a flag I could set to trigger non-zero exit statuses for any warnings so I can use clippy meaningfully from my scripts. Apologies if something like this already exists and I've just missed it, running clippy --help doesn't appear to show anything relevant though.

[mcarton]

The return code is handled by rustc itself. However in a script, it would be much easier to use JSON output:

% rustc a.rs 
warning: unused variable: `a`, #[warn(unused_variables)] on by default
 --> a.rs:2:9
  |
2 |     let a = 42;
  |         ^

% rustc --error-format json a.rs 
{"message":"unused variable: `a`, #[warn(unused_variables)] on by default","code":null,"level":"warning","spans":[{"file_name":"a.rs","byte_start":20,"byte_end":21,"line_start":2,"line_end":2,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":"    let a = 42;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[],"rendered":null}

(you get one line per error/warning, which is a json object describing the error)

[sanmai-NL]

@mcarton: that does not clearly separate between messages generated by Clippy and other messages. For instance, the compiler aborting due to previous Clippy-induced errors is considered an error in itself.

Moreover, Cargo writes these machine readable JSON messages on standard error instead of standard output, resulting in unparseable about (a mixture of exclusively human and machine readable output).

[oli-obk]

You can pass -Dwarnings to ensure that warnings are treated as errors. This way you'll also get the non-zero exit code.

[sanmai-NL]

@oli-obk: Does that work if you set the lint level to warn for every lint (group), via attributes?

[oli-obk]

@sanmai-NL yes

[sanmai-NL]

@oli-obk:
It doesn’t work for me. something is a crate with one bin target, member of a virtual workspace.

something/src/main.rs:

#![feature(stmt_expr_attributes)]
#![warn(bad_style)]
#![warn(future_incompatible)]
#![warn(unused)]
#![warn(warnings)]
#![allow(missing_docs)]
#![cfg_attr(feature = "cargo-clippy", warn(clippy))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy_pedantic))]
#![cfg_attr(feature = "cargo-clippy", allow(missing_docs_in_private_items))]

// ...

cargo +nightly clippy --frozen --all-features --package something -- -Dwarnings

   Compiling something v0.1.0 (file:///tmp/something)
warning: used unwrap() on a Result value. If you don't want to handle the Err case gracefully, consider using expect() to provide a better panic message
    --> something/src/main.rs:19:41
     |
19   |             let service = const_service(create_new_service(data).unwrap());
     |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
note: lint level defined here
    --> something/src/main.rs:8:44
     |
8    | #![cfg_attr(feature = "cargo-clippy", warn(clippy_pedantic))]
     |                                            ^^^^^^^^^^^^^^^
     = note: #[warn(result_unwrap_used)] implied by #[warn(clippy_pedantic)]
     = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#result_unwrap_used

    Finished dev [unoptimized + debuginfo] target(s) in 48.81 secs

echo $?

0

[oli-obk]

It works fine on my machine :/

oliver@iai-matthes1:~/Projects/rust/cargo_metadata$ cargo +nightly clippy --frozen --all-features --package cargo_metadata -- -Dwarnings
lib: cargo_metadata
   Compiling cargo_metadata v0.5.0 (file:///home/oliver/Projects/rust/cargo_metadata)
error: this expression borrows a reference that is immediately dereferenced by the compiler
   --> src/lib.rs:199:46
    |
199 |         let version = semver::Version::parse(&version).map_err(de::Error::custom)?;
    |                                              ^^^^^^^^ help: change this to: `version`
    |
    = note: `-D needless-borrow` implied by `-D warnings`
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#needless_borrow

error: redundant field names in struct initialization
   --> src/lib.rs:204:13
    |
204 |             version: version,
    |             ^^^^^^^^^^^^^^^^ help: replace it with: `version`
    |
    = note: `-D redundant-field-names` implied by `-D warnings`
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.186/index.html#redundant_field_names

error: aborting due to 2 previous errors

error: Could not compile `cargo_metadata`.

To learn more, run the command again with --verbose.

oliver@iai-matthes1:~/Projects/rust/cargo_metadata$ echo $?
101

It was in a lib project without workspaces though.

[sanmai-NL]

@oli-obk: what can we do here? Can you or someone else who wishes to confirm take some more time to test with a virtual workspace? If this snag is resolved then this GitHub issue could, as far as I am concerned, be closed. So it seems worthwhile to test and even add a test case if there turns out to be a bug.

[oli-obk]

@sanmai-NL If you give me a repro example (upload your example to github?) I can investigate further

[niedhui]

@oli-obk I had the same problem as @sanmai-NL , the output said
error: aborting due to 12 previous errors, which followed by Finished dev [unoptimized + debuginfo] target(s) in 1m 18s, and the returned status is 0 (checked on my machine). here is the ci job link https://travis-ci.org/niedhui/rust-prometheus/jobs/394541190#L1948

[cpick]

I ran into this as well.
I can reproduce with the following Dockerfile:

FROM rust:1.26

RUN USER='root' cargo init --vcs=none --name=foo \
    && echo 'fn unused() {}' >> src/main.rs \
    && rustup --version \
    && rustup toolchain install nightly \
    && cargo +nightly install clippy \
    && cargo --version && cargo +nightly clippy --version \
    && cargo +nightly clippy -- -D warnings

Which when built with sudo docker build . erroneously succeeds:

...
  Installing /usr/local/cargo/bin/cargo-clippy
  Installing /usr/local/cargo/bin/clippy-driver
cargo 1.26.0 (0e7c5a931 2018-04-06)
0.0.209
    Checking foo v0.1.0 (file:///)
error: function is never used: `unused`
 --> src/main.rs:4:1
  |
4 | fn unused() {}
  | ^^^^^^^^^^^
  |
  = note: `-D dead-code` implied by `-D warnings`

error: aborting due to previous error

    Finished dev [unoptimized + debuginfo] target(s) in 0.21s
Removing intermediate container 2255f616fd2c
 ---> a0b758bbad36
Successfully built a0b758bbad36

[oli-obk]

Fixed in #2863

Now we just need a regression test. Probably just adding something to travis is enough.

[mkpankov]

I don't think forbidding warnings is enough. For example, when I have two packages in workspace, -D warnings causes clippy to stop after some warnings are encountered in first package, thus not checking the second one at all.

Moreover this is awkward to use in CI in non-blocking mode. I'd like to know number of hard errors and number of warnings. Hard errors should block CI, but not the warnings.

[mleonhard]

I'm on macOS 10.15.6 x86 and clippy will not exit with non-zero status.

$ cargo clippy -- -D warnings ; echo $?
...
warning: 3 warnings emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
0
$ cargo version
cargo 1.47.0-beta (f3c7e066a 2020-08-28)
$ rustup toolchain list
beta-x86_64-apple-darwin (default)
$ rustc --version
rustc 1.47.0-beta.3 (aa30bf344 2020-09-10)

I'm trying to make my local dev tools work the same as the project's continuous integration scripts on GitHub.

I tried configuring CLion 2020.2.4 with a single run configuration that runs both cargo clippy and cargo test. I made a cargo test run configuration. Then I made a cargo clippy run configuration and added it to the "Before launch" section of the cargo test config. Executing the cargo test config does execute the cargo clippy, but doesn't stop on clippy errors because the exit value is always zero.

I would really appreciate advice on setting this up.

[mleonhard]

Strangely, the problem just went away:

$ cargo clippy -- -D warnings ; echo $?
...
error: aborting due to 2 previous errors

error: could not compile `fixed-buffer`.

To learn more, run the command again with --verbose.
101
$

And the cargo test CLion run config is now properly failing when cargo clippy fails.

Perhaps clippy is caching some results on disk and returning 0 in some strange edge case?

[imeoer]

Maybe we can try RUSTFLAGS="-D warnings" cargo build ....

[amarshall]

Indeed this seems to be due to some persistent state (setting RUSTFLAGS triggers a recompile so is similar to rm -r target):

cargo clippy -- -D warnings &>/dev/null ; echo $? #=> 0
rm -r target
cargo clippy -- -D warnings &>/dev/null ; echo $? #=> 1
cargo clippy -- -D warnings &>/dev/null ; echo $? #=> 1
cargo clippy                &>/dev/null ; echo $? #=> 0
cargo clippy -- -D warnings &>/dev/null ; echo $? #=> 0

[lnshi]

You can pass -Dwarnings to ensure that warnings are treated as errors. This way you'll also get the non-zero exit code.

@oli-obk the problem is that if i run with the option --deny=warnings then it exit on the fist warning it encountered, i would like clippy to still run until to the end to list down all the issues, and if there are some warnings then set the none-zero status code, not fail fast.

How can i achieve that? @mcarton @oli-obk

[oli-obk]

You're in luck: rust-lang/rust#87337 just got merged and should be in stable in 7 or 8 weeks, then this should just work exactly as you expect

[jyn514]

Note that rust-lang/rust#87337 only avoids failing fast within the crate; if you have multiple crates in a workspace, cargo will still stop after the first one with a denied warning.

[lnshi]

@jyn514 i see...., may i know what is the motivation of this design? I feel either make it configurable that totally not failing fast, or something like only after encountered certain number of denies then fail is better? Instead of like currently fail at 1st, then push then fail at another, so on and on, a bit annoying

[jyn514]

@lnshi "it's hard" and no one has done the work to implement it.

[lquenti]

Why is this issue still open? Seems pretty fixed to me

[jyn514]

This still doesn't work between crates in a workspace, only within individual crates. Doing it for the whole workspace requires cargo support: rust-lang/rust#82761 (comment)

[kpreid]

Here are two more reasons why a separate flag would be an improvement over -Dwarnings for this purpose:

• It invalidates the build cache vs. running cargo clippy, so you have to wait longer if the other command is run for any reason.
• It makes all warnings labeled as errors in the output.

An option that changed the exit status behavior only would not have these disadvantages.

[Alexendoo]

Tracking #1209 (comment) in #12623

For the cargo feature that will be rust-lang/cargo#8424