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

force full re-clippy #2604

Closed
davemilter opened this issue Apr 1, 2018 · 18 comments
Closed

force full re-clippy #2604

davemilter opened this issue Apr 1, 2018 · 18 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@davemilter
Copy link

I build clippy from git sources (4ef7238).
This is re-open of #2498 as @oli-obk suggested.
And actully nothing changes because of #2582 , and why it should?

Sample code:

use std::io;

pub fn map_err(err: io::Error) -> String {
    format!("{}", err)
}
$ cargo clippy
   Compiling foo v0.1.0 (file:///tmp/foo)
warning: this argument is passed by value, but not consumed in the function body
 --> src/lib.rs:3:21
  |
3 | pub fn map_err(err: io::Error) -> String {
  |                     ^^^^^^^^^ help: consider taking a reference instead: `&io::Error`
  |
  = note: #[warn(needless_pass_by_value)] on by default
  = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.191/index.html#needless_pass_by_value

    Finished dev [unoptimized + debuginfo] target(s) in 0.14 secs
$ cargo clippy
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs

As you use the second run of clippy gives nothing, and cargo clippy --help
have no options to force rerun.

@matthiaskrgr
Copy link
Member

I usually work around this by runnning something like touch src/main.rs ; cargo clippy

@oli-obk
Copy link
Contributor

oli-obk commented May 20, 2018

can you try with clippy 0.0.203? I included a possible fix there

@CyrilCalmels
Copy link

CyrilCalmels commented May 20, 2018

I still have the issue with clippy 0.0.203 😢

@baumanj
Copy link

baumanj commented May 25, 2018

Also seeing this with v0.0.204

@Manishearth
Copy link
Member

cc @alexcrichton is there any way we can get a mode to cargo check that always rebuilds the toplevel crate (i.e. the root, or if running -p, that crate). Preferably not clobbering artefacts unless they need to be.

@dwijnand
Copy link
Member

@killercup is fixing the same bug that cargo fix has over in rust-lang/cargo#5750, which I'm hopeful will make fixing this much easier.

@killercup
Copy link
Member

killercup commented Jul 25, 2018 via email

@JoshMcguigan
Copy link
Contributor

rust-lang/cargo#5944 was merged, which is a rebase of killercup's work. I'd be open to working on a fix for this, if there is anyone who can provide guidance on what is needed from the clippy side?

@matthiaskrgr
Copy link
Member

If the cargo folks agree, perhaps we could have commandline arg in cargo which forces the package to be rebuilt. (cargo check -f / --force or something like that).
I guess the flag then needs to be hooked up in.
https://github.com/rust-lang-nursery/rust-clippy/blob/master/src/main.rs#L103-L111

We would somehow need to make sure it only forces recheck of the crates in the current workspace and not all dependencies as well, not sure what is the best way to do this though.. :/

@BatmanAoD
Copy link
Member

Even something as simple as detecting that no work has been done and outputting a warning suggesting the user touch the root file would improve the user experience drastically, I think.

I believe the ideal solution is a diagnostic-caching mechanism, but currently users have no indication that cargo build && cargo clippy is a misuse of Clippy, or that their code may have lint warnings or errors that they're not seeing.

@JoshMcguigan
Copy link
Contributor

I just submitted rust-lang/cargo#6664 to cargo to force cargo check to rebuild. There may be some additional work needed there, but hopefully that will move this issue forward.

@JoshMcguigan
Copy link
Contributor

It seems rust-lang/cargo#6664 is close to getting merged. It would be helpful if someone familiar with how clippy integrates with cargo could check in and confirm this would solve the clippy use case.

@Manishearth
Copy link
Member

Yes, but we may need to pass a flag down

@crlf0710
Copy link
Member

crlf0710 commented Feb 23, 2019

Encountered this problem - when i run clippy twice with different parameters, the program didn't get rebuilt, so i got no output.

baumanj added a commit to habitat-sh/habitat that referenced this issue Apr 16, 2019
There's no need to retry clippy commands, they should be deterministic.
Additionally re-using a container may cause us to get false positives
due to rust-lang/rust-clippy#2604. This
change should help us avoid that.

Signed-off-by: Jon Bauman <[email protected]>
@ehuss
Copy link
Contributor

ehuss commented Jul 23, 2019

This is fixed on latest master of cargo for clippy-preview (via rust-lang/cargo#7157). I'll be updating nightly soon. Migrating to clippy-preview is tracked in #3837.

Tarnadas added a commit to Tarnadas/rustlings that referenced this issue Feb 26, 2020
- adds a new 'clippy' category for exercises
- clippy exercises should throw no warnings
- install script now also installs clippy

is related to rust-lang/rust-clippy#2604
@Livven
Copy link

Livven commented Mar 29, 2020

Still an issue for me.

cargo clippy --version
clippy 0.0.212 (4ee12063 2020-02-01)

cargo --version
cargo 1.42.0 (86334295e 2020-01-31)

rustc --version
rustc 1.42.0 (b8cedc004 2020-03-09)

rustup toolchain list
stable-x86_64-pc-windows-msvc (default)

@ehuss
Copy link
Contributor

ehuss commented Mar 29, 2020

@Livven It was fixed as a nightly-only feature, which has since been removed. We are migrating to a new approach, which should hopefully land soonish via #5363.

@Livven
Copy link

Livven commented Mar 29, 2020

@ehuss Thanks for the info, looking forward to it. Seems like it should also fix #5199.

pedantic79 pushed a commit to pedantic79/rustlings that referenced this issue Apr 11, 2020
- adds a new 'clippy' category for exercises
- clippy exercises should throw no warnings
- install script now also installs clippy

is related to rust-lang/rust-clippy#2604
MendelMonteiro pushed a commit to MendelMonteiro/rustlings that referenced this issue Jun 28, 2020
- adds a new 'clippy' category for exercises
- clippy exercises should throw no warnings
- install script now also installs clippy

is related to rust-lang/rust-clippy#2604
kodiakhq bot pushed a commit to sbdchd/squawk that referenced this issue Sep 9, 2020
Turns out `name` is an Option and that `expr` can be a more complicated
object rather than just a string.

Previously we didn't have any tests that exercised the `expr` property.

I think the issues with clippy are related to: rust-lang/rust-clippy#2604

fixes: #59
wpbrown added a commit to blockcaptain/blockcaptain that referenced this issue Sep 12, 2020
wmanley added a commit to ostreedev/ostree-oxide that referenced this issue Nov 16, 2020
They don't seem to all come at once.  This could be
rust-lang/rust-clippy#2604 .
ppp3 pushed a commit to ppp3/rustlings that referenced this issue May 23, 2022
- adds a new 'clippy' category for exercises
- clippy exercises should throw no warnings
- install script now also installs clippy

is related to rust-lang/rust-clippy#2604
FancyGeekGuru added a commit to FancyGeekGuru/rust-lings that referenced this issue May 24, 2022
- adds a new 'clippy' category for exercises
- clippy exercises should throw no warnings
- install script now also installs clippy

is related to rust-lang/rust-clippy#2604
dmoore04 pushed a commit to dmoore04/rustlings that referenced this issue Sep 11, 2022
- adds a new 'clippy' category for exercises
- clippy exercises should throw no warnings
- install script now also installs clippy

is related to rust-lang/rust-clippy#2604
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

No branches or pull requests