Clippy doesn't report error/warnings after cargo check

[b1zzu]

Environment

Project: https://github.com/b1zzu/wifiscanner/tree/clippy-bug (use the clippy-bug branch`)
Cargo: cargo 1.38.0 (23ef9a4ef 2019-08-20)
Clippy: clippy 0.0.212 (3aea860 2019-09-03)

Steps to reproduce

Run cargo clippy after cargo check

cargo check
cargo clippy

Clippy doesn't fail or display any errors.

Expected result

cargo clippy should fails with this errors:

warning: Constants have by default a `'static` lifetime
  --> src/lib.rs:87:22
   |
87 |     const PATH_ENV: &'static str = "PATH";
   |                     -^^^^^^^---- help: consider removing `'static`: `&str`
   |
   = note: `#[warn(clippy::redundant_static_lifetimes)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes

error: approximate value of `f{32, 64}::consts::PI` found. Consider using it directly
  --> src/lib.rs:85:14
   |
85 |     let _x = 3.14;
   |              ^^^^
   |
   = note: `#[deny(clippy::approx_constant)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant

warning: single-character string constant used as pattern
   --> src/lib.rs:157:16
    |
157 |         .split("\n")
    |                ^^^^ help: try using a char instead: `'\n'`
    |
    = note: `#[warn(clippy::single_char_pattern)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern

warning: single-character string constant used as pattern
   --> src/lib.rs:168:36
    |
168 |     for line in network_list.split("\n") {
    |                                    ^^^^ help: try using a char instead: `'\n'`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern

error: aborting due to previous error

error: Could not compile `wifiscanner`.

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

Workaround

cargo check
cargo clean
cargo clippy

[mati865]

The only reasonable way to fix it is #3837

With nightly toolchain you can use cargo clippy-preview -Z unstable-options as workaround.

[b1zzu]

@mati865 for now I will use cargo clean before clippy

[matthiaskrgr]

It works better if you touch all files of your root crate, this way you don't have to rebuild all the dependencies every time:

find . | grep "\.rs$" | xargs touch ; cargo clippy

[flip1995]

I think we should document this (find . | grep "\.rs$" | xargs touch ; cargo clippy) until cargo clippy (in cargo) is stable.

[b1zzu]

@flip1995 where would you document it? In a new separate Troubleshooting section or as a comment/tips to the step-3-run-clippy

[flip1995]

I think at step-3-run-clippy is enough, since it will be removed anyway in the near future.

[maxbla]

This issue can also be triggered by running clippy multiple times with different options.
main.rs

fn main() {
    let _1 = 1_usize;
    println!("{}", _1);
}

$ cargo clean
$ cargo clippy -- --deny clippy::just_underscores_and_digits
    Compiling ...
error: consider choosing a more descriptive name
...
$ cargo clippy -- --allow clippy::all
$ cargo clippy -- --deny clippy::just_underscores_and_digits

cargo clean fixes this manifestation of the problem also

[sourcefrog]

It looks like this is now fixed?

cargo check
cargo clippy

shows the right errors every time I run it.

clippy 0.0.212 (69f99e7 2019-12-14)
cargo 1.41.0 (626f0f40e 2019-12-03)

[flip1995]

That would surprise me, because we didn't change anything.

I just tested #4612 (comment) and the behavior didn't change there.

[humb1t]

Clippy still depends on check: clippy 0.0.212 (4ee1206 2020-02-01)

[najamelan]

The only reasonable way to fix it is #3837

What about having cargo clippy run the touch **.rs workaround itself? It would depend on platform support for the touch command and globbing, but I feel this issue is important enough to try this.

[mati865]

What about having cargo clippy run the touch **.rs workaround itself?

You can just make a script that will call touch and Clippy.

[humb1t]

@mati865 don't you think that this is not a solution?
Imagine that cargo test will not run tests if cargo build was already executed.

[mati865]

@humb1t neither modifying files without permission is the solution.

[flip1995]

This is now fixed when running Clippy with

cargo clippy -Zunstable-options

on nightly.

[humb1t]

@mati865 totally agree with you

[matthiaskrgr]

The feature should be available as soon as nightly 2020-04-18 rolls out.

[saleemrashid]

If I run the following commands (in this order) in my workspace, which has some dead_code, all the commands work as expected

# dead_code errors
cargo clippy -Z unstable-options -- -D warnings
# dead_code + clippy::restriction errors
cargo clippy -Z unstable-options -- -D warnings -D clippy::restriction
# dead_code errors
cargo clippy -Z unstable-options -- -D warnings

However, if I use warnings instead of errors, the first invocation's output is cached.

# dead_code warnings
cargo clippy -Z unstable-options
# dead_code warnings
cargo clippy -Z unstable-options -- -W clippy::restriction
find -name "*.rs" -exec touch {} \;
# dead_code + clippy::restriction warnings
cargo clippy -Z unstable-options -- -W clippy::restriction
# dead_code + clippy::restriction warnings
cargo clippy -Z unstable-options

Not sure if I'm using an outdated Clippy version, none of the comments seem to specify which is the "fixed" nightly, or even link to the relevant pull request.

---

rustc 1.44.0-nightly (b2e36e6c2 2020-04-22)
clippy 0.0.212 (891e1a8 2020-04-18)

[flip1995]

You should be using the nightly where this is already included.

@yaahc any ideas?

[saleemrashid]

Here's a shorter reproduction case

echo "fn unused() {}" > src/lib.rs
cargo clippy -Z unstable-options
cargo clippy -Z unstable-options -- --allow dead_code
touch src/lib.rs
cargo clippy -Z unstable-options -- --allow dead_code
cargo clippy -Z unstable-options

---

Note that it works fine with errors, but not warnings.

echo "pub fn bad() -> bool { 0 <= 0 }" > src/lib.rs
cargo clippy -Z unstable-options
cargo clippy -Z unstable-options -- --allow clippy::eq_op
touch src/lib.rs
cargo clippy -Z unstable-options -- --warn clippy::all
cargo clippy -Z unstable-options -- --allow clippy::eq_op

---

rustc 1.44.0-nightly (b2e36e6c2 2020-04-22)
cargo 1.44.0-nightly (8751eb301 2020-04-21)
clippy 0.0.212 (891e1a8 2020-04-18)

[yaahc]

This is probably an issue with cargo, I'll check it out tomorrow.

cc @ehuss

[ehuss]

The caching with different flags is rust-lang/cargo#7862. The issue is twofold. One is that clippy hides the flags from cargo (via __CLIPPY_HACKERY__). The other is that Cargo doesn't provide a way to hash flags.

EDIT: I haven't looked into this, but I'm uncertain why clippy uses the flag hack. If it used RUSTFLAGS instead, it might just work? Something worth investigating.

[flip1995]

This is now fixed in Clippy and will land in some nightly over the weekend. This means the fix will hit stable (if nothing goes wrong) in 1.52.0.

[tesuji]

Hi, I'm happy this issue fixed.
From the changelog it seems that

cargo clippy no longer shares the same build cache as cargo check.

One issue for me is that clippy run slower than cargo-check command.
Before this change, I run cargo check and touch main.rs or lib.rs to redo the lints by clippy.

There are some reasons why clippy is slower than cargo-check:

1. clippy has many more lints to run compare to just official lints supported by rust in cargo-check.
2. even though warnings/errors by lints for transitive dependencies are silent by default,
   the lints actually run for all the code in dependencies. Don't run allowed lints rust#74704

Should I opened an issue to track this on clippy side ?

Those said, my personal issue should not be a reason to revert the fix.
Thank you for working on this.

[flip1995]

One issue for me is that clippy run slower than cargo-check command.

Yes this is true for your crate, not for the dependencies. This is because Clippy runs more lints and analyzes your code more than cargo check. It shouldn't be that noticeable though.

Clippy lints are not run for dependencies (except local path dependencies, but you can disable this behavior with cargo clippy -- --no-deps). The issue you linked isn't really about linting dependencies - that's just an effect of this issue. There's nothing from the Clippy side that could be done.

Before this change, I run cargo check and touch main.rs or lib.rs to redo the lints by clippy.

This change didn't even land yet. The effect of this change is solely that you don't have to touch those files anymore to run cargo clippy after cargo check.

[hellow554]

This issue is still pinned. Is this on purpose or just a leftover? I could understand it if you want people who aren't using the latest stable version to see this in the first place. But as said: is it on purpose?

[flip1995]

Since last Thursday it isn't on purpose anymore, since this hit stable 🎉 Thanks again @everyone who worked on this! I removed the pin, thanks for reminder.