add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings #6469

matthiaskrgr · 2020-12-18T21:37:48Z

cargo dev crater now does the following:
build clippy in debug mode
for a fixed set of crates:
download and extract the crate
run compiled clippy on the crate
dump the warnings into a file that is inside the repo

We can then do a "git diff" and see what effects our clippy changes had on a tiny fraction of the rust ecosystem and can see when an change unexpectedly added or silenced a lot of warnings.

Checking all the crates took less than 5 minutes on my system.

Should help with #6429

Please write a short comment explaining your change (or "none" for internal only changes)
changelog: extend cargo dev to run clippy against a fixed set of crates and compare warnings

rust-highfive · 2020-12-18T21:37:52Z

r? @Manishearth

(rust-highfive has picked a reviewer for you, use r? to override)

flip1995

Some ideas how to expand this.

clippy_dev/src/crater.rs

flip1995 · 2020-12-19T13:50:34Z

clippy_dev/src/crater.rs

+                "--message-format=short",
+                "--",
+                "--cap-lints=warn",
+                "-Wclippy::pedantic",


It would also be nice if we can specify the lint groups with a flag. I think this default is fine though.

Hm, why not just lint everything?
That way we have greater coverage.

Yeah, by default that is good, but for future testing, we might want to only run exactly one lint, to make it easier to check the output.

clippy_dev/src/main.rs

clippy_dev/src/crater.rs

clippy_dev/Cargo.toml

clippy_dev/src/crater.rs

camelid

By the way, sorry I left several 'single comments' – it always bothers me when people do that :)

clippy_dev/src/crater.rs

flip1995

None of my enhancement suggestions should block this PR. I you feel like you don't want to add this (yet), we can merge this as is and revisit possible enhancements later when we need them.

clippy_dev/Cargo.toml

clippy_dev/src/crater.rs

flip1995

This LGTM now! Please squash some of the commits.

Two more ideas I got from this:

maybe it would make sense to dump the lint statistics in the log file for a quick overview of the changes.
Something for the future: We could implement an disabled by default lint that looks for allow(clippy::_) attributes and then use this tool to find out how often (and which) Clippy lints are allowed in crates that use Clippy.

This can be done in future PRs, so r=me once squashed.

phansch · 2021-01-04T11:32:27Z

Potential bikeshed: Would it maybe make sense to rename this to something else than crater? I could see someone getting confused with rust-lang/crater which also has Clippy related features.

bors · 2021-01-05T08:14:03Z

☔ The latest upstream changes (presumably #6547) made this pull request unmergeable. Please resolve the merge conflicts.

flip1995 · 2021-01-22T15:41:28Z

@matthiaskrgr The only thing to address for this to get merged is the request for renaming this to $something else.

matthiaskrgr · 2021-01-22T22:01:51Z

Yes, sorry, i'll try to wrap this over the course of the weekend.

matthiaskrgr · 2021-01-22T23:55:58Z

I've renamed to "lintcheck" now, hope that's better.

…every run

…ic and cargo lints, ignore tokei for now.

Also sort lint results alphabetically.

…get the lint name of a warning

…riggered)

…d crates with --only crate

… in the .toml list

make serde a feature-dep save clippy version in the crater log

flip1995

@phansch r=me if you're also ok with the new name.

phansch · 2021-01-26T06:57:57Z

@bors r=flip1995

bors · 2021-01-26T06:57:59Z

📌 Commit 5b6a183 has been approved by flip1995

bors · 2021-01-26T06:58:09Z

⌛ Testing commit 5b6a183 with merge be0125b...

bors · 2021-01-26T07:09:43Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing be0125b to master...

rust-highfive assigned Manishearth Dec 18, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 18, 2020

matthiaskrgr added the S-needs-discussion Status: Needs further discussion before merging or work can be started label Dec 18, 2020

matthiaskrgr force-pushed the clippy_dev_crater branch from a0e0baf to 3404e4c Compare December 18, 2020 22:31

flip1995 reviewed Dec 19, 2020

View reviewed changes

camelid reviewed Dec 20, 2020

View reviewed changes

clippy_dev/Cargo.toml Outdated Show resolved Hide resolved

camelid reviewed Dec 20, 2020

View reviewed changes

clippy_dev/src/crater.rs Outdated Show resolved Hide resolved

camelid reviewed Dec 20, 2020

View reviewed changes

clippy_dev/src/crater.rs Outdated Show resolved Hide resolved

camelid reviewed Dec 20, 2020

View reviewed changes

clippy_dev/src/crater.rs Outdated Show resolved Hide resolved

clippy_dev/src/crater.rs Outdated Show resolved Hide resolved

matthiaskrgr force-pushed the clippy_dev_crater branch 2 times, most recently from fe8f897 to 202c46c Compare December 23, 2020 12:12

flip1995 approved these changes Dec 27, 2020

View reviewed changes

clippy_dev/Cargo.toml Outdated Show resolved Hide resolved

matthiaskrgr force-pushed the clippy_dev_crater branch from 913caa1 to 98f8c4d Compare December 27, 2020 15:21

flip1995 reviewed Dec 27, 2020

View reviewed changes

clippy_dev/Cargo.toml Outdated Show resolved Hide resolved

clippy_dev/src/crater.rs Outdated Show resolved Hide resolved

matthiaskrgr force-pushed the clippy_dev_crater branch 2 times, most recently from 590e046 to 00e36d2 Compare December 29, 2020 16:04

flip1995 approved these changes Jan 4, 2021

View reviewed changes

flip1995 mentioned this pull request Jan 22, 2021

[Roadmap] False Positive Rate #6623

Closed

5 tasks

matthiaskrgr force-pushed the clippy_dev_crater branch 2 times, most recently from 7499cc8 to 253740a Compare January 23, 2021 01:17

matthiaskrgr added 3 commits January 23, 2021 02:18

cargo dev crater: lay out the base plan

bec916d

cargo dev crater: work on downloading and extracting crate sources

5353591

crater: hook into main.rs

30d8594

matthiaskrgr added 22 commits January 23, 2021 02:18

print all clippy warnings in the end

734d205

cargo dev crater: cleanup, don't re-download and reextract crates on …

7314133

…every run

cargo dev crater: save all warnings into a file

dbb8c00

add the log file

728dc06

cargo dev crater: add more crates to be checked

1e5ac1d

cargo dev crater: share target dir between clippy runs, enable pedant…

ccfaa33

…ic and cargo lints, ignore tokei for now.

allow clippy::filter_map

a9fce6d

use a .toml file to list the crates we want to check

588efa7

Also sort lint results alphabetically.

cargo dev crater: support multiple versions per crate

f986d78

rename symbols: krate -> crate

22824d2

remove duplicate code and other cleanup

62337f2

clippy dev crater: use and parse clippy messages as json message, to …

6c5bf27

…get the lint name of a warning

cargo dev crater: refactor to get a list of all ClippyWarnings

4ec9cb8

cargo dev crater: gather and save lint statistics (how often a lint t…

e56c9a5

…riggered)

make stats stable

d257101

clippy dev crater: add option to only check a single one of the liste…

b6ef1e2

…d crates with --only crate

cargo dev crater: throw an error if we can't find our specified crate…

ec1902c

… in the .toml list

add shortcut "dev-crater" command to build and run "cargo dev crater"

94a73d7

clippy dev crater: address more review commetns

48fc948

make serde a feature-dep save clippy version in the crater log

rename cargo dev crater to cargo dev lintcheck

83fcf95

update lintcheck-logs

d0d28b1

lintcheck: fix paths in the logs

5b6a183

matthiaskrgr force-pushed the clippy_dev_crater branch from 253740a to 5b6a183 Compare January 23, 2021 01:18

flip1995 approved these changes Jan 25, 2021

View reviewed changes

bors merged commit be0125b into rust-lang:master Jan 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings #6469

add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings #6469

matthiaskrgr commented Dec 18, 2020 •

edited

Loading

rust-highfive commented Dec 18, 2020

flip1995 left a comment

flip1995 Dec 19, 2020

matthiaskrgr Dec 23, 2020

flip1995 Dec 27, 2020

camelid left a comment

flip1995 left a comment

flip1995 left a comment

phansch commented Jan 4, 2021

bors commented Jan 5, 2021

flip1995 commented Jan 22, 2021

matthiaskrgr commented Jan 22, 2021

matthiaskrgr commented Jan 22, 2021

flip1995 left a comment •

edited

Loading

phansch commented Jan 26, 2021

bors commented Jan 26, 2021

bors commented Jan 26, 2021

bors commented Jan 26, 2021

add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings #6469

add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings #6469

Conversation

matthiaskrgr commented Dec 18, 2020 • edited Loading

rust-highfive commented Dec 18, 2020

flip1995 left a comment

Choose a reason for hiding this comment

flip1995 Dec 19, 2020

Choose a reason for hiding this comment

matthiaskrgr Dec 23, 2020

Choose a reason for hiding this comment

flip1995 Dec 27, 2020

Choose a reason for hiding this comment

camelid left a comment

Choose a reason for hiding this comment

flip1995 left a comment

Choose a reason for hiding this comment

flip1995 left a comment

Choose a reason for hiding this comment

phansch commented Jan 4, 2021

bors commented Jan 5, 2021

flip1995 commented Jan 22, 2021

matthiaskrgr commented Jan 22, 2021

matthiaskrgr commented Jan 22, 2021

flip1995 left a comment • edited Loading

Choose a reason for hiding this comment

phansch commented Jan 26, 2021

bors commented Jan 26, 2021

bors commented Jan 26, 2021

bors commented Jan 26, 2021

matthiaskrgr commented Dec 18, 2020 •

edited

Loading

flip1995 left a comment •

edited

Loading