Fix *most* clippy lints #15906
Merged
Fix *most* clippy lints #15906
Conversation
This file contains 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
alice-i-cecile
approved these changes
Oct 14, 2024
alice-i-cecile
added
C-Code-Quality
This was referenced Oct 14, 2024
BenjaminBrienen
approved these changes
Oct 14, 2024
| @@ -587,7 +587,7 @@ impl AssetSources { | |||
| AssetSourceId::Name(name) => self | |||
| .sources | |||
| .get(&name) | |||
| .ok_or_else(|| MissingAssetSourceError(AssetSourceId::Name(name))), | |||
| .ok_or(MissingAssetSourceError(AssetSourceId::Name(name))), | |||
There was a problem hiding this comment.
What's the problem with the original code? Now this value is constructed every time instead of only when needed.
There was a problem hiding this comment.
It's not actually constructed: since it's just wrapping in enum variants/newtype structs, it's basically as simple as a copy anyway, and adding an extra level of function indirection just makes it harder to optimize.
The clippy lint specifically looks for "trivial" constructions like this, since the amount of text makes it seem like a lot of work is being done, when it's actually not.
There was a problem hiding this comment.
What a useful lint :)
BenjaminBrienen
added
the
D-Trivial
kristoff3r
approved these changes
Oct 14, 2024
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.
Objective
Another clippy-lint fix: the goal is so that
ci lintsactually displays the problems that a contributor caused, and not a bunch of existing stuff in the repo. (when run on nightly)Solution
This fixes all but the
clippy::needless_lifetimeslint, which will result in substantially more fixes and be in other PR(s). I also explicitly allownon_local_definitionssince it is not working correctly, but will be fixed.A few things were manually fixed: for example, some places had an explicitly defined
div_ceilfunction that was used, which is no longer needed since this function is stable on unsigned integers. Also, empty lines in doc comments were handled individually.Testing
I ran
cargo clippy --workspace --all-targets --all-features --fix --allow-stagedwith theclippy::needless_lifetimeslint marked asallowinCargo.tomlto avoid fixing that too. It now passes with all but the listed lint.