Add clippy at workspace level #766
Conversation
tcoratger
requested review from
DaniPopes,
gakonst,
mattsse,
onbjerg,
prestwich and
Evalir
as code owners
| [workspace.lints] | ||
| rust.missing_debug_implementations = "warn" | ||
| rust.missing_docs = "warn" | ||
| rust.unreachable_pub = "warn" | ||
| rust.unused_must_use = "deny" | ||
| rust.rust_2018_idioms = "deny" | ||
| rustdoc.all = "warn" | ||
|
|
||
| [workspace.lints.clippy] |
There was a problem hiding this comment.
unsure about these, defer to @prestwich and @mattsse if we should just keep this default or not
obviously some of them should be kept to allow us to remove some of the #![warn] and #![deny]s
There was a problem hiding this comment.
this is a big list to review.
some of them, like missing_const_for_fn are definitely not set correctly. we do want to enforce missing_const_for_fn, in spite of the bug wrt const fn destructors
For nursery lints, the comment needs to be dated with when the list was reproduced here, and link to documentation of the full list of nursery lints. It needs to be maintained regularly or will fall out of date with the nursery. This is the sort of annoying churn and manual overhead that I try to avoid and is why we stopped using nightly clippy in the first place
Overall, I am in favor of dropping the whole list and keeping defaults
There was a problem hiding this comment.
@prestwich But if we keep only the default, this means the following list
rust.missing_debug_implementations = "warn"
rust.missing_docs = "warn"
rust.unreachable_pub = "warn"
rust.unused_must_use = "deny"
rust.rust_2018_idioms = "deny"
rustdoc.all = "warn"
and for example we omit use_self which was quite useful here or missing_const_for_fn that I can add to the warn list (my bad). As a ref, I used the same list as in reth.
There was a problem hiding this comment.
@prestwich I've just updated by pushing all (all lints that are on by default (correctness, suspicious, style, complexity, perf))
I added 3 others as supplements which seem stable and quite useful compared to what we have in the codebase:
missing_const_for_fn = "warn"
use_self = "warn"
option_if_let_else = "warn"
tell me what you think?
There was a problem hiding this comment.
Cool, this looks fine for me. Can merge once we re-apply the lints on latest main
| @@ -3,16 +3,7 @@ | |||
| html_logo_url = "https://raw.githubusercontent.com/alloy-rs/core/main/assets/alloy.jpg", | |||
| html_favicon_url = "https://raw.githubusercontent.com/alloy-rs/core/main/assets/favicon.ico" | |||
| )] | |||
| #
|
will give this another pass in the morning |
|
sorry, i thought i left a comment days ago. i'm good with the approach and will approve and merge once re-applied to main |
Just did a rebase :) |
Description
This pull request adds the Clippy tool at the workspace level. By enabling Clippy at the workspace level, it will run on all projects within the workspace, providing consistent linting across the entire codebase.
Changes Made
Cargo.tomlfile.clippy.tomlfile to customize linting rules and configurations.