Skip to content

Commit 8024fac

Browse files
committed
chore(lib): start a strict clippy config
This starts a change to embrace a strict allowlist with Clippy. ## Why? Clippy can detect a lot of mistakes. The defaults are very good. But it has the power to detect many more _probable_ mistakes, and enforce coding patterns beyond formatting. With the increase in LLM generated code, a stricter Clippy can protect us from the LLM generating poorer code. The pedantic and restriction groups are not enabled by default. There is even a warning to not enable the restriction group blindly. But, even they have good lints. The usual recommendation is to just turn on the lints you care about. Instead, this embraces restricting everything by default, and keep an explicit allowlist. The benefits for this are that we explicitly consider every possible "bad code" lint. We decide if it's something to ignore. And we also don't accidentally not notice a new lint. Every time Clippy upgrades, we may see some new lints that could improve our code. That is excellent! When that happens, we can decide whether to adjust the code, or allow the lint. [More reading](https://billylevin.dev/posts/clippy-config/) ## How? Restricting all these lints in one go would be a large amount of changes. Some of them can be done automatically (`cargo clippy --fix`), some of them an LLM can very easily do, and some require manual inspection in each place. This starts by enabling all the groups, listing out every lint that was triggered, and then allows them explicitly for now. I've split that list into two separate smaller lists (described here in reverse order): - Lints that are expliticly allowed. - Lints that should be decided on, either by fixing the code, or removing the TODO and putting them in the explicitly allowed list (ideally explaining why). Follow up commits can address those lints, and when doing so, update the list. This will prevent rot from occurring by keeping the PR open for a long time, or conflicts.
1 parent 5dbcae7 commit 8024fac

2 files changed

Lines changed: 88 additions & 0 deletions

File tree

.github/workflows/CI.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ jobs:
1717
runs-on: ubuntu-latest
1818
needs:
1919
- style
20+
- lint
2021
- test
2122
- msrv
2223
- miri
@@ -51,6 +52,22 @@ jobs:
5152
exit 1
5253
fi
5354
55+
lint:
56+
name: Linter
57+
#needs: [style]
58+
runs-on: ubuntu-latest
59+
steps:
60+
- name: Checkout
61+
uses: actions/checkout@v6
62+
63+
- name: Install Rust
64+
uses: dtolnay/rust-toolchain@stable
65+
66+
- uses: Swatinem/rust-cache@v2
67+
68+
- name: Clippy
69+
run: cargo clippy --all-targets --features full -- -D warnings
70+
5471
test:
5572
name: Test ${{ matrix.rust }} on ${{ matrix.os }}
5673
needs: [style]

Cargo.toml

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,77 @@ check-cfg = [
105105
'cfg(hyper_unstable_ffi)'
106106
]
107107

108+
[lints.clippy]
109+
pedantic = { level = "warn", priority = -1 }
110+
restriction = { level = "warn", priority = -2 }
111+
112+
# keep an allow list of lints
113+
114+
# lints to decide on
115+
arithmetic_side_effects = "allow"
116+
as_conversions = "allow"
117+
borrow_as_ptr = "allow"
118+
empty_structs_with_brackets = "allow"
119+
indexing_slicing = "allow"
120+
missing_errors_doc = "allow"
121+
missing_panics_doc = "allow"
122+
module_inception = "allow"
123+
multiple_unsafe_ops_per_block = "allow"
124+
panic = "allow"
125+
ptr_as_ptr = "allow"
126+
single_char_lifetime_names = "allow"
127+
undocumented_unsafe_blocks = "allow"
128+
129+
# explicitly allowed
130+
absolute_paths = "allow" # sometimes its cleaner
131+
arbitrary_source_item_ordering = "allow" # order: std, deps, crate
132+
blanket_clippy_restriction_lints = "allow" # allowlist is better
133+
clone_on_ref_ptr = "allow" # Arc::clone(blah) is needlessly noisy
134+
cognitive_complexity = "allow" # is this ever useful?
135+
default_numeric_fallback = "allow" # too many false positives
136+
expect_used = "allow" # expect is self-documenting
137+
error_impl_error = "allow" # mod::Error is a fine name
138+
field_scoped_visibility_modifiers = "allow" # possibly good idea, noisy for now
139+
if_not_else = "allow" # order depends on which is more common, not truthiness
140+
if_then_some_else_none = "allow" # control flow as if better than closures
141+
implicit_return = "allow" # standard rust
142+
impl_trait_in_params = "allow" # turbofish ignored on purpose
143+
inline_modules = "allow" # common for sealed types
144+
inline_trait_bounds = "allow" # more concise if shorter bounds
145+
let_underscore_must_use = "allow" # the entire point was to ignore must_use
146+
let_underscore_untyped = "allow"
147+
mod_module_files = "allow" # easier to find than self-named modules
148+
min_ident_chars = "allow" # not to be abused, nor forced
149+
missing_assert_message = "allow" # not much value
150+
missing_docs_in_private_items = "allow" # these docs aren't rendered
151+
missing_inline_in_public_items = "allow" # bad lint
152+
missing_trait_methods = "allow"
153+
must_use_candidate = "allow" # bad lint
154+
module_name_repetitions = "allow" # sometimes it happens, not bad at all
155+
new_without_default = "allow" # not everything needs a Default impl
156+
panic_in_result_fn = "allow" # panics are for invariants
157+
pub_use = "allow"
158+
pub_with_shorthand = "allow"
159+
question_mark_used = "allow" # question mark is excellent
160+
unreachable = "allow" # self-documenting invariants
161+
ref_patterns = "allow" # TODO: perhaps deny?
162+
renamed_function_params = "allow" # we can pick clearer names
163+
same_name_method = "allow"
164+
shadow_unrelated = "allow" # shadowing is useful
165+
shadow_reuse = "allow" # shadowing is useful
166+
single_call_fn = "allow" # abstracting to a function is the point
167+
self_named_module_files = "deny" # already denied but, rly, dont do it
168+
semicolon_inside_block ="allow" # depends on context
169+
semicolon_outside_block ="allow" # depends on context
170+
std_instead_of_alloc = "allow" # std is more idiomatic
171+
std_instead_of_core = "allow" # std is more idiomatic
172+
struct_field_names = "allow" # not really helpful
173+
too_many_lines = "allow" # reconsider someday?
174+
type_complexity = "allow" # not helpful
175+
used_underscore_items = "allow"
176+
unused_trait_names = "allow" # TODO: kinda annoying, but might be good to deny
177+
unnecessary_safety_comment = "allow" # more safety comments are a good thing
178+
108179
[package.metadata.docs.rs]
109180
features = ["ffi", "full", "tracing"]
110181
rustdoc-args = ["--cfg", "hyper_unstable_ffi", "--cfg", "hyper_unstable_tracing"]

0 commit comments

Comments
 (0)