Fix several clippy lints #70
Merged
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
Clippy doesn't like calls to `std::mem::drop` with references --- since the pointed value is not actually dropped. In this case, this *is* the correct behavior, but clippy views this as a footgun if the user means to actually drop the value behind the reference. Here, we're using `drop` to ignore values in no-op default impls for trait methods. I've changed those methods to use `let _` to ignore parameters instead. This doesn't trigger the clippy lint and is maybe more idiomatic anyway. Signed-off-by: Eliza Weisman <[email protected]>
The tests will make assertions that particular values are recorded in a particular order. When testing with floating-point values, clippy emits a warning that we should be using epsilon comparisons rather than bit equality. However, this isn't an issue here, since the tests just using float literals, and if the right float is recorded in the right order, they will be precisely equal. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Clippy lints on explicitly using the `'static` lifetime for values in a static, and when passing a reference to a reference that is automatically dereferenced (into a single reference) by the compiler. This commit fixes those lints. Signed-off-by: Eliza Weisman <[email protected]>
This fixes a clippy lint. Signed-off-by: Eliza Weisman <[email protected]>
Clippy lints about types which have a public `len` method but no corresponding `is_empty` method. Therefore, I've added an `is_empty` method that returns `true` if `self.len() > 0`. Since this is a new API that we'll have to support, it would also be fine to solve this by _not_ adding the method and just suppressing the warning...but it seems worth having for consistency with the underlying slice's methods? And, I don't see how this could really pose a forward-compatibility hazard, since `Slice` is...always going to be backed by an actual slice...
taiki-e
reviewed
Oct 16, 2021
Comment on lines
+198
to
+206
| // When testing floating-point values, the | ||
| // actual value will be the exact same float | ||
| // value as the expected, if everything is | ||
| // working correctly. So, it's not strictly | ||
| // necessary to use epsilon comparisons here, | ||
| // and modifying the macro to use epsilon | ||
| // comparisons for floats would make it | ||
| // significantly more complex... | ||
| #[allow(clippy::float_cmp)] |
There was a problem hiding this comment.
Well, float_cmp will soon be allowed by default. rust-lang/rust-clippy#7692
Co-authored-by: Taiki Endo <[email protected]>
taiki-e
approved these changes
Oct 16, 2021
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.
This branch contains a number of small fixes for several clippy lints:
00f5715 Add
Slice::is_empty()methodClippy lints about types which have a public
lenmethod but nocorresponding
is_emptymethod. Therefore, I've added anis_emptymethod that returns
trueifself.len() > 0.Since this is a new API that we'll have to support, it would also be
fine to solve this by not adding the method and just suppressing the
warning...but it seems worth having for consistency with the
underlying slice's methods? And, I don't see how this could really
pose a forward-compatibility hazard, since
Sliceis...always goingto be backed by an actual slice...
fb8c985 don't call
cloneonValue, which isCopydd3018e fix clippy lints about static lifetimes
Clippy lints on explicitly using the
'staticlifetime for values ina static, and when passing a reference to a reference that is
automatically dereferenced (into a single reference) by the compiler.
This commit fixes those lints.
fbc5daa silence clippy warning on approximate values of pi
c81f5b9 silence warning on non-epsilon float comparisons
The tests will make assertions that particular values are recorded in
a particular order. When testing with floating-point values, clippy
emits a warning that we should be using epsilon comparisons rather
than bit equality. However, this isn't an issue here, since the tests
just using float literals, and if the right float is recorded in the
right order, they will be precisely equal.
0e12f69 Fix
clippy::drop_reflintClippy doesn't like calls to
std::mem::dropwith references ---since the pointed value is not actually dropped. In this case, this
is the correct behavior, but clippy views this as a footgun if the
user means to actually drop the value behind the reference.
Here, we're using
dropto ignore values in no-op default impls fortrait methods. I've changed those methods to use
let _to ignoreparameters instead. This doesn't trigger the clippy lint and is maybe
more idiomatic anyway.