treewide: further tighten clippy lints #7493
Merged
Conversation
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
13 tasks
alyssais
reviewed
Nov 22, 2025
phip1611
force-pushed
the
upstream-clippy-tighten-followup
branch
from
e834007 to
0a578c2
Compare
Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP [email protected]
Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP [email protected]
phip1611
force-pushed
the
upstream-clippy-tighten-followup
branch
from
0a578c2 to
ce3cc6c
Compare
Member
Author
|
@rbradford @likebreath I thoroughly went through all the |
phip1611
force-pushed
the
upstream-clippy-tighten-followup
branch
3 times, most recently
from
f9e6b8f to
b749ca9
Compare
This removes cognitive load when reading if statements. All changes were applied by clippy via `--fix`. Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP [email protected]
This helps to uncover expensive and needless clones in the code base. For example, I prevented extensive clones in the snapshot path where (nested) BTreeMap's have been cloned over and over again. Further, the lint helps devs to much better reason about the ownership of parameters. All of these changes have been done manually with the necessary caution. A few structs that are cheap to clone are now `copy` so that this lint won't trigger for them. I didn't enable the lint so far as it is a massive rabbit hole and needs much more fixes. Nevertheless, it is very useful. Signed-off-by: Philipp Schuster <[email protected]> On-behalf-of: SAP [email protected]
phip1611
force-pushed
the
upstream-clippy-tighten-followup
branch
from
b749ca9 to
048cfd2
Compare
rbradford
approved these changes
Nov 25, 2025
Member
rbradford
left a comment
rbradford
left a comment
There was a problem hiding this comment.
I think these changes move the code base to more idiomatic Rust which is probably a good thing! (Even though I often need to double check what these clever functional programming style methods do!.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Follow-up of #7483 and currently my last effort regarding clippy lints to improve developer experience and code quality. We now reached a point where the low-hanging fruits are harvested and the world of rabbit-holes begin.
I actively decided against lints
missing_const_for_fn,must_use_candidate, andmissing_panics_doc. The first two are very invasive with little benefit for CHV. They might make sense in finer-grained scopes (i.e., not treewide).missing_panics_docis helpful but very invasive and I didn't want to spend the time so far on that. Further, as we are shipping a binary rather than a (Set of) librarie(s), it is okay to not have that.Explanation for the lints I've selected (IMHO):
map_unwrap_orimprove code readabilityredundant_elseimproves code readabilityif_not_elsereduces cognitive complexity/mental load when reading codeneedless_pass_by_valueuncovers costly clones and makes it easier to reason about ownership of variablesHints for Reviewers
#7489