chore(clippy): clear all warnings and gate clippy in CI - #739
Merged
Merged
Conversation
Clippy is about to become a merge gate. too_many_arguments and type_complexity describe API shape, not defects, and `!(a < b)` is deliberate NaN handling that must not be rewritten to `>=`. Each crate inherits the table with `[lints] workspace = true`.
cargo clippy --fix rewrote the suggestions it marks as safe: needless borrows, unit lets, map_or forms, and nested ifs as let chains. Comparisons and NaN handling are untouched. One collapsible_match suggestion does not compile and stays for a targeted allow.
The machine-applicable pass left local rewrites and a few suggestions that would change a signature, drop a wire-format comment, or collapse a match that does not compile. Those last ones are allowed on the item, with the reason beside the attribute.
Pull requests were not linted, so warnings accumulated until -D warnings failed on the first crate. The new Windows job runs the same clippy command developers must pass, and the CI contract test reddens if that command is weakened or the job is demoted.
CoreTrack::averaged checked the divisor by hand and claimed checked_div would report zero. checked_div returns None for a zero sample count, which is the unknown state, and the same quotient otherwise. The hotkeys allow stays; its comment now says the suggested collapse does not compile.
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
1 participant
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.
What & why
CI never ran clippy, so warnings grew with every PR:
cargo clippy --workspace --all-targetsreported 464 unique warnings (moon-ui-gpui 347, moon-core 105, moon-chart 12), and with
-D warningsit stopped at moon-core, so moon-ui-gpui was never checked at all.This makes it zero and keeps it there:
[workspace.lints.clippy]allowstoo_many_arguments,type_complexityandneg_cmp_op_on_partial_ord. The!(a < b)forms are deliberate NaN handling and stay as they are.cargo clippy --fixfor everything machine-applicable, includingcollapsible_ifrewritten aslet-chains.
targeted
#[allow]with a reason:large_enum_varianton feed / command / match-heavy enums(boxing would allocate on hot paths), and
result_large_errwhere boxing would change a publicResult.cargo clippy --workspace --all-targets --locked -- -D warnings, andci_gate_contract.rspins it so the gate cannot be demoted silently. CONTRIBUTING lists the rule.No behaviour change is intended: no dependency, lock, edition or public API change. The branch is
rebased on the tuner (#738), which added no new warnings.
Commits are split so each part reviews on its own: lint table,
--fixoutput, hand fixes andallows, CI gate, and a fix for two allow reasons.
How to verify
cargo clippy --workspace --all-targets --locked -- -D warningsexits 0cargo test --workspace --lockedandcargo test -p moon-ui-gpui --test theme_contractgreencargo fmt --all -- --checkgreenwindow render normally, including wheel zoom.