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

clippy Deny on diff only #4491

Closed
o0Ignition0o opened this issue Sep 3, 2019 · 14 comments
Closed

clippy Deny on diff only #4491

o0Ignition0o opened this issue Sep 3, 2019 · 14 comments
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise.

Comments

@o0Ignition0o
Copy link

We're trying to enable clippy::pedantic on neqo (mozilla/neqo#187 for example) and, given there's lot of refactoring to be made all across the codebase, I was wondering if there was a way to filter clippy warnings only on the "current diff", erroring out or linting only the parts of the code that have been edited when working on an issue.

I was thinking of something like the clang format diff tool

I have no idea how hard it would be to implement, but I would love to implement it if someone would like to mentor me on that one.

Thank's a lot for your time.

@flip1995
Copy link
Member

flip1995 commented Sep 3, 2019

There's currently no such script that does that. Such a script should apply on Rust warnings/errors in general though, not just on Clippy.

This could be done by getting the changed files from git and then filtering the JSON-output of the rust error messages by the file names. (cargo clippy --message-format json)


Fun-fact: this request is kind of the opposite of another long standing issue: #2604

@flip1995 flip1995 added the C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. label Sep 3, 2019
@flip1995
Copy link
Member

flip1995 commented Sep 3, 2019

Such a script should be implemented in Rust and not in Python, like the clang script, obviously. 😉

@phansch
Copy link
Member

phansch commented Sep 3, 2019

Some months ago I started a private project that does this in the form of a GitHub bot. I'll try and see if I can find some time to bring that to a MVP state and possibly open-source parts of it.

@o0Ignition0o
Copy link
Author

I have just checked the --message-format json output and it gives the line numbers, which I can then compare with a git diff, so it seems like a pet peeve I can work on, thanks for the hints!

@phansch
Copy link
Member

phansch commented Sep 3, 2019

@o0Ignition0o I just realized that my project was already open-source. You might want to have a look at

https://github.com/phansch/mend-rs/blob/09f675bac7270e59fe8875cc06742bd8b34e05cc/src/review_comment/mod.rs#L28

Feel free to copy things as you need :)

@o0Ignition0o
Copy link
Author

Wow thanks a lot, I hope I can come up with something useful this week !

@o0Ignition0o
Copy link
Author

o0Ignition0o commented Sep 10, 2019

Just published a very rough first draft of cargo-scout, that will run clippy in pedantic mode and only warn you on the lints that were raised in your patch.
The code is ugly for now, and there's still a lot of bugs, which I want to fix as soon as I figure them out :)
https://github.com/o0Ignition0o/cargo-scout

There's still quite a lot to be improved, running cargo clean is often required before a clippy run, and I've probably allocated too much, oh and there aren't any tests... But I'll hopefully get there soon !

@phansch I've used the structures you deserialize into, do you mind me adding a "Inspiration" section at the bottom of the README.md with a special thanks to you ?

@mati865
Copy link
Contributor

mati865 commented Sep 10, 2019

Sounds like an idea for great overall tool for Rust projects when it gains option to config which lints are enabled.
Another interesting idea is to include rust-lang/rustfmt#1324. Large projects like Rust could really use some formatting but enabling it everywhere is not an option.

@phansch
Copy link
Member

phansch commented Sep 10, 2019

@phansch I've used the structures you deserialize into, do you mind me adding a "Inspiration" section at the bottom of the README.md with a special thanks to you ?

Not at all, I'm glad the code found some actual use =)

@o0Ignition0o
Copy link
Author

So as a little heads up, cargo-scout 0.3.0 is now out, and has all the features I wanted as I've opened this issue:

It only warns on the lints that intersect with your diff.
It iterates over workspace members if --no-default-features is passed for example.
It even handles the neqo use case where #[deny(warnings)] is behind a feature flag, which is part of the default feature set.
cargo-scout -p --no-default-features works well everywhere (-p makes it use clippy-preview with unstable features), except when there's a build-rs file, which is the case in neqo-crypto.

I'll dig further on that, but I'm pretty satisfied with the results so far !
Thank you all for helping me kickstart the project, which has now 5 other contributors doing some amazing things <3

@flip1995
Copy link
Member

Thanks for sharing! I suggested it as Crate of the Week in TWIR: https://users.rust-lang.org/t/crate-of-the-week/2704/694

@o0Ignition0o
Copy link
Author

Cargo-scout is crate of the week in this week in rust ! I’m really happy, and I’ve already received some feedback (some suggesting the ability to use cargo fix, which was already brought up here). I’m gonna dig this way :)
Thanks a lot for the support !

@llogiq
Copy link
Contributor

llogiq commented Dec 28, 2019

Congratulations!

@blyxyas
Copy link
Member

blyxyas commented Jun 2, 2023

As cargo-scout is out, I think this issue can be closed
@o0Ignition0o

@flip1995 flip1995 closed this as completed Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise.
Projects
None yet
Development

No branches or pull requests

6 participants