internal: Add clippy to CI
#16413
internal: Add clippy to CI
#16413
Conversation
rustbot
added
the
S-waiting-on-review
Urhengulas
force-pushed
the
clippy-ci
branch
2 times, most recently
from
4a4f41d to
c79f0f3
Compare
Urhengulas
changed the title
clippy and rustfmt to CIclippy to CI
.github/workflows/ci.yaml
Outdated
| @@ -103,6 +103,10 @@ jobs: | |||
| RUSTC_BOOTSTRAP: 1 | |||
| run: target/${{ matrix.target }}/debug/rust-analyzer analysis-stats --with-deps $(rustc --print sysroot)/lib/rustlib/src/rust/library/std | |||
|
|
|||
| - name: clippy | |||
| if: matrix.os == 'ubuntu-latest' | |||
| run: cargo clippy --all-targets -- -D warnings | |||
There was a problem hiding this comment.
Does this enable all clippy warnings or specifically only the ones we list in the Cargo.toml files? We only want the latter as we want direct control of the lints.
There was a problem hiding this comment.
It does enable all warnings.
The approach I took is enabling all warnings, and then ignoring (aka. "allow"ing) all warnings we do not care about. Some warnings gonna be ignored forever, but some others are gonna be worked on (as happened in e.g. #16404).
There was a problem hiding this comment.
We could also only enable ("deny" in clippy jargon) the lints we care about, but I prefer the approach of considering all clippy lints, except we find them to be not fitting to our needs.
There was a problem hiding this comment.
Ah wait now I see. I'm confused as to why we pass -D warnings here, thats not a clippy lint group and clippy should check normal rustc warnings by default no? All the other groups that are relevant we have enabled in the lints table already.
There was a problem hiding this comment.
Good point. Gonna remove that.
There was a problem hiding this comment.
And the purpose of that flag is to make clippy error out if there is any warning. But we can configure that more fine grained in the lint table.
Veykril
added
S-waiting-on-author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
Fixed the new clippy errors which landed on master while this PR is open. Let's try to merge it soon, otherwise we need to fix more clippy errors. |
We configure that in the lint table in `Cargo.toml`.
|
@bors r+ |
|
☀️ Test successful - checks-actions |
|
Our builds are a little lopsided, anyone opposed to running Clippy on Windows?
|
|
👍 |
|
Clippy really belongs on Windows anyway 😄 |
internal: run Clippy on Windows CC #16413 (comment) 
Follow-up to #16401