Repository navigation
Enable pedantic Clippy lints one at a time - #426
Merged
Merged
Conversation
Convert numbers that always fit with `From` (`f64::from(width)`) instead
of `as`. The lint skips `const` initializers, where `From` cannot be
called, and conversions to `usize` and `isize`, whose width depends on
the target, so those keep `as`.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Inline named arguments in format strings: `format!("{name}: {err}")`.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Use `map_or`, `map_or_else`, `is_some_and`, and `is_ok_and` instead of
`map(..).unwrap_or(..)`.
Mostly mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Written by hand:
- The four caret-movement arms in caret_edit.rs are a `match`. The
`map_or_else` form needs `caret` and `anchor` mutably in both
closures.
- `sub_ring_children` is a `match`: the `&[]` default does not infer
as a slice in `map_or`.
- The configurator's recorded-device mapping matches on
`Option<InputSource>` directly, instead of a `map_or` with a nested
`match`.
The translations guard lists the `tr(...)` arguments it accepts without
a literal as source text, so its two action-metadata entries follow the
new spelling.
Put identifiers and Wayland protocol names in doc comments in backticks
(`wl_output`, `zwp_tablet_v2`). Product names that docs write plainly
(GLib, HarfBuzz, HiDPI, JetBrains, MSBuild, StatusNotifierItem, VSync)
go in `doc-valid-idents` in clippy.toml, next to Clippy's default list.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Two comments that the fix could not mark up cleanly were reworded by
hand: the surface module summary and the zoom-chip invariant.
End unit-valued trailing expressions with a semicolon.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Write a `match` with one pattern and a fallback as `if let ... else`.
Mostly mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Four matches with comments in their arms were converted by hand, so the
comments stay with their branch: config edit completion, the properties
toggle knob, a toolbar preset-slot test, and the configurator caret
restore.
Take raw pointers with `&raw const` and `&raw mut` instead of casting a
reference, and change pointer constness with `cast_mut()`. The FFI calls
receive the same pointers, without an intermediate reference.
Mechanical, with the lints enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Nest or-patterns inside the shared constructor:
`Some(A | B)` instead of `Some(A) | Some(B)`.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Use `midpoint` instead of `(a + b) / 2`. The result is the same
wherever the sum does not overflow. Integer `midpoint` rounds toward
zero like `/ 2`. `f64::midpoint` computes `(a + b) * 0.5` when both
inputs are at most `f64::MAX / 2` in magnitude. Only where the sum
would overflow does `midpoint` differ: it still returns a value
between the two inputs.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Put the positive condition first: `if x { a } else { b }` instead of
`if !x { b } else { a }`.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
Give a raw string only the `#` delimiters its content needs: `r"..."`
when it contains no `"`, and `r#"..."#` when it contains `"` but not
`"#`.
Mechanical, with the lint enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
duration_suboptimal_units, elidable_lifetime_names, explicit_iter_loop,
ignored_unit_patterns, implicit_clone, manual_is_variant_and,
manual_string_new, option_as_ref_cloned, and redundant_else.
Mechanical, with the lints enabled in Cargo.toml:
cargo clippy --fix --workspace --all-targets --all-features
cargo clippy --fix --workspace --all-targets --no-default-features
cargo fmt --all
The tray shutdown calls return `()`, so they are plain `.await;` rather
than the `let () = ...` that the unit-pattern fix writes.
Use `let ... else` for a `match` or `if let` that only unwraps a value when the other branch diverges, for example with `return`, `continue`, or `panic!`. Clippy's suggestions for this lint are not machine-applicable, so they were applied and each site was read. One suggestion lost a binding name in the context-menu focus test; that site was fixed by hand. Every `panic!` and `bail!` in the new `else` blocks ends with a semicolon.
Write the anonymous lifetime in paths: `BindingInserter<'_>`, not `BindingInserter`, so a borrowed type is visible in the signature. The lint goes in a new `[workspace.lints.rust]` table. Applied from the compiler's suggestions, then `cargo fmt --all`.
comparison_chain, ignore_without_reason, and
unnecessary_debug_formatting.
- The one-edit keybinding comparison matches on `a.len().cmp(&b.len())`.
- The Wayland smoke test says why it is ignored.
- The configurator search text includes session paths with `display()`
instead of `{:?}`, so searches match the path without debug quotes or
escapes.
State the rules behind the pedantic lints in plain terms, and note that pedantic lints are enabled one at a time, never as the whole group.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enable 25 pedantic Clippy lints and the
elided_lifetimes_in_pathsrustc lint,one commit per lint or small group, and fix every site.
[workspace.lints.clippy]inCargo.toml(or thenew
[workspace.lints.rust]table) together with its fixes, so every commitpasses Clippy on its own.
cargo clippy --fixfor both feature configurations,followed by
cargo fmt --all. Each commit message lists the sites changed by hand.clippy.tomlgainsdoc-valid-identsfor product names that docs write plainly(GLib, HarfBuzz, HiDPI, JetBrains, MSBuild, StatusNotifierItem, VSync).
AGENTS.md"Rust Style" section states the rules the new lints enforce.Pedantic lints stay enabled one at a time, never as the whole group, so a
toolchain update does not turn new ones on.
Lints:
borrow_as_ptr,cast_lossless,comparison_chain,doc_markdown,duration_suboptimal_units,elidable_lifetime_names,explicit_iter_loop,if_not_else,ignore_without_reason,ignored_unit_patterns,implicit_clone,manual_is_variant_and,manual_let_else,manual_midpoint,manual_string_new,map_unwrap_or,needless_raw_string_hashes,option_as_ref_cloned,ptr_cast_constness,redundant_else,semicolon_if_nothing_returned,single_match_else,uninlined_format_args,unnecessary_debug_formatting,unnested_or_patterns.