clippy -D warnings doesn't fail on warnings

[leeola]

I have a internal project (cannot share, unfortunately) with a series of lints defined in the lib.rs, main.rs, etc. Eg:

#![warn(
    rust_2018_idioms,
    nonstandard_style,
    future_incompatible,
    clippy::all,
    clippy::restriction,
    clippy::pedantic,
    clippy::nursery
)]

I am attempting to get CI to fail when linting this project, via: cargo clippy -Z unstable-options -- --deny "warnings". However, clippy is printing warnings and not exiting.

Having recently struggled with Clippy and Caching (ala #4612) I even resorted to cargo clean && cargo clippy -Z unstable-options -- --deny "warnings", however clippy still prints warnings without failing.

Unfortunately I cannot seem to reproduce this issue in another crate; something I could post here as a reproduction. The same Rust & Clippy version in another crate, with the same #[warn(clippy::restriction)], is correctly failing on -D warnings, where as my private crate is incorrectly not failing. My CI pipeline is currently running -D warnings while printing warnings and not failing. This issue is not isolated to just my PC.

I'm seeking some advice here on how to debug this, and provide a reproducible bug report. Any thoughts are appreciated, thank you.

Meta

me ~/t/2/foo> cargo clippy -V
clippy 0.0.212 (5fd2f06e9 2020-05-31)

me ~/t/2/foo> rustc -Vv
rustc 1.45.0-nightly (5fd2f06e9 2020-05-31)
binary: rustc
commit-hash: 5fd2f06e99a985dd896684cb2c9f8c7090eca1ab
commit-date: 2020-05-31
host: x86_64-apple-darwin
release: 1.45.0-nightly
LLVM version: 10.0

[leeola]

For context, this is example code that correctly fails on warning,

#![warn(clippy::restriction)]
#![allow(clippy::print_stdout)]
fn main() {
    println!("Hello, world!");
    docless_fn();
}
fn docless_fn() {
    println!("I have no docs!");
}

And running with: cargo clippy -Z unstable-options -- -D warnings fails as I would expect. Unfortunately my private crate is only throwing warnings and not failing. I am seeking additional thoughts on how to further debug my CI pipelines not failing when they should via cargo clippy -Z unstable-options -- -D warnings.

[flip1995]

Copying my reply of another issue (#5733 (comment)):

FYI: enabling the complete restriction group is not recommended, since it includes really restrictive lints, that sometimes are in contrast with other lints or even go against idiomatic rust. These lints should only be enabled on a lint-by-lint basis and with careful consideration.

---

Regarding your issue: In the above post you wrote, that you used --D warnings, instead of -D warnings (note the additional -). Maybe this is causing the lint group not to be set correctly?

[leeola]

FYI: enabling the complete restriction group is not recommended, since it includes really restrictive lints, that sometimes are in contrast with other lints or even go against idiomatic rust. These lints should only be enabled on a lint-by-lint basis and with careful consideration.

Appreciate the syntax note. I originally started to browse lints and apply many, but it seemed easier to disable lints than to try and pick large amounts of them. I currently have ~7 or so disabled, some at the source of warnings where it makes sense.

Regarding your issue: In the above post you wrote, that you used --D warnings, instead of -D warnings (note the additional -). Maybe this is causing the lint group not to be set correctly?

Good catch, thank you. I have edited it to correctness. For context I actually use --deny warnings, though and my command from CI is more like:

cargo clippy -Z unstable-options --manifest-path "foo/Cargo.toml" --features "$BUILD_FEATURES" -- --deny "warnings"

Because I need to work around this problem while needing my CI to actually fail when it should, I've resorted to some (poorly written) bash:

cargo clippy -Z unstable-options --manifest-path "foo/Cargo.toml" --features "$BUILD_FEATURES" -- --deny "warnings"
# A hack to work around the above command passing when it shouldn't.
CLIPPY_STDERR_LINES=`cargo clippy -Z unstable-options --manifest-path "foo/Cargo.toml" --features "$BUILD_FEATURES" -- --deny "warnings" 2>&1 | wc -l | tr -d '[:space:]'`
echo CLIPPY_STDERR_LINES=$CLIPPY_STDERR_LINES
if [ ! "$CLIPPY_STDERR_LINES" = "1" ]; then
  echo "Failing due to lint warnings"
  exit 1
fi

(Note, the above bash may fail in undesired circumstances, it is incomplete. I'm only just adding it now, we'll see if it works with some editing /shrug)

In very limited tests so far, this properly fails when clippy does not. So, at least it can probably be worked around with some external scripting.

[flip1995]

Can you try to use RUSTFLAGS="-Dwarnings" cargo clippy -Z unstable-options. If that works the clippy args may be broken for some reason 🤔 (Maybe we should deprecate these arguments and recommend to use RUSTFLAGS also for Clippy)

[leeola]

Same deal:

me ~/w/foo.rs> env RUSTFLAGS="-Dwarnings" cargo clippy -Z unstable-options --manifest-path "foo/Cargo.toml"
warning: missing documentation for a method
   --> foo/src/foo.rs:54:5
    |
54  | /     pub fn foo(&self) -> i32 {
114 | |     }
    | |_____^
    |
note: the lint level is defined here
   --> foo/src/lib.rs:39:5
    |
39  |     clippy::restriction,
    |     ^^^^^^^^^^^^^^^^^^^
    = note: `#[warn(clippy::missing_docs_in_private_items)]` implied by `#[warn(clippy::restriction)]`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.36s
me ~/w/foo.rs>

(the above is highly edited to remove code, project names, etc. Trying to keep my employer happy lol. Sorry for the silliness.)

cargo clean before running the above results in the same behavior. Same result as well, if I'm in the crate root (ie not the workspace). Note that the workspace setup is something I've not yet tried to reproduce. I'll try that now.

edit: no luck, neither removing the other workspace members or setting up my example reproduction as a workspace caused a change in either repositories behavior.

[flip1995]

Uh I don't think this is a Clippy issue then, but a cargo issue, where the lint levels doesn't get passed through the workspace members. This may be related to rust-lang/cargo#5015, where the same problem with features existed.

@ehuss do you know if RUSTFLAGS="-Dwarnings" gets passed to workspace members by cargo?

@leeola Can you try if the same thing happens with RUSTFLAGS="-Dwarnings" cargo check (maybe with a cargo clean before´) when you have for example an unused variable in the code?

[ehuss]

Hm, that's a tricky issue.

RUSTFLAGS will get passed to everything, though I wouldn't expect it to be different.

Some things I would check:

• Run with -v and verify the correct flags are passed to the crate in question.
  • Make sure --cap-lints is not passed. It shouldn't be passed for any workspace members.
  • Make sure there aren't any other flags like -W or --warn that would affect lints. (Unfortunately the actual clippy args get hidden in an environment variable.)
• Make sure there aren't any attributes that override your crate-level attributes anywhere in the module or any parent module. (probably unlikely, since I think "allow" is the only option here, and that would completely silence warnings).
• Next I would try to circumvent the cargo-clippy interface and use clippy-driver directly. I would copy and paste the command-line from the -v output, add -Dwarnings and remove the JSON options, and see how it behaved. If your crate has environment variables it requires, you can run cargo with -vv to print those as well.
  • For example: clippy-driver --crate-type lib --crate-name foo foo/src/lib.rs -Dwarnings, with all the necessary --extern flags.

[leeola]

Run with -v and verify the correct flags are passed to the crate in question.

Interesting. I'm not exactly sure what to look for, but it looks like there may be a problem here. In my test repo, the one where -D warnings is functioning correctly, I see (in short):

   Checking foo v0.1.0 (/Users/me/tmp/foo/foo)
     Running `/Users/me/.rustup/toolchains/nightly-foo06-25-x86_64-apple-darwin/bin/clippy-driver rustc --crate-name foo --edition=2018 foo/src/main.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,metadata -Cembed-bitcode=no -C debuginfo=2 -C metadata=15224b04dae8c8fc -C extra-filename=-15224b04dae8c8fc --out-dir /Users/me/tmp/foo/target/debug/deps -C incremental=/Users/me/tmp/foo/target/debug/incremental -L dependency=/Users/me/tmp/foo/target/debug/deps`

Which I imagine is what you're referencing. However, I cannot get this to display in my bugged repo. In my bugged repo, passing -v to cargo clippy only results in various fresh statuses, eg:

       Fresh autocfg v1.0.0
       Fresh unicode-xid v0.2.0
....

Not a single clippy-driver rustc line is printed. Same exact version of clippy and cargo between the two repos. Running -vv likewise only shows Fresh statuses.

Good call, this seems a strong suspect for the root cause.

Next I would try to circumvent the cargo-clippy interface and use clippy-driver directly. I would copy and paste the command-line from the -v output, add -Dwarnings and remove the JSON options, and see how it behaved. If your crate has environment variables it requires, you can run cargo with -vv to print those as well.

I'll try manually running it by using the test crate as a reference, but since I cannot actually see the -v output, this one may be difficult to implement until -v works.

Thanks for the help all!

[leeola]

Oh, and:

@leeola Can you try if the same thing happens with RUSTFLAGS="-Dwarnings" cargo check (maybe with a cargo clean before´) when you have for example an unused variable in the code?

Same deal (ran with cargo clean, but I'm not including all the compilation lines here):

me ~/w/p/foo> env RUSTFLAGS="-Dwarnings" cargo check
warning: unused variable: `foo`
  --> foo/src/foo.rs:6:13
   |
6  |         let foo = FooBuilder::default();
   |             ^^^^^^ help: if this is intentional, prefix it with an underscore: `_foo`
   |
note: the lint level is defined here
  --> foo/src/lib.rs:36:5
   |
36 |     unused,
   |     ^^^^^^
   = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`

warning: 1 warning emitted

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

[flip1995]

@leeola Sorry for the late reply, I stayed away from OSS for a bit.

What I suspect from the fact, that the same happens with cargo check is, that there is something going on with cargo or rustc and not really clippy.

What I think might be the problem is, that you have a #![warn(clippy::all)] in the lib.rs of your workspace, which overrides the -Dwarnings passed through the commandline/RUSTFLAGS env var. At least this is what I get from the note of your last comment:

   = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`

[bmc-msft]

I've been able to reliably reproduce -D warnings working (and then breaking) locally with a simple code sample with a single warning.

Once I've ever run clippy without -D warnings, until I rebuild it will no longer exit with a non-zero exit code.

src/main.c

fn main() {
    let s: String = format!("hello").into();
    println!("{}", s);
}

My testing:

❯ cargo clean
❯ cargo clippy -- -D warnings 2> /dev/null
❯ echo $?
101
❯ cargo clippy -- -D warnings 2> /dev/null
❯ echo $?
101
❯ cargo clippy  2> /dev/null
❯ echo $?
0
❯ cargo clippy -- -D warnings 2> /dev/null
❯ echo $?
0
❯ cargo clippy -- -D warnings 2> /dev/null
❯ echo $?
0
❯

In this example, clippy with -D warnings unexpectedly exits with a zero exit code after running it without -D warnings once.