Extend manual_is_variant_and lint to check for boolean map comparisons#14646
Conversation
|
samueltardieu is on vacation. Please choose another assignee. |
0b94b9e to
8913c0e
Compare
|
Fixed fmt |
8913c0e to
f65ce46
Compare
|
Let's reroll reviewer. r? clippy |
|
r? samueltardieu |
samueltardieu
left a comment
There was a problem hiding this comment.
This can probably be simplified quite a bit, see inner comment.
Could you please also add a test with a generic type which takes a generic parameter such as bool and implements a map(), so that we can check that types are correctly filtered?
|
Could you also add a test where the |
f65ce46 to
20590c9
Compare
|
Just thought about a case where we're comparing to a function returned value. In this case we don't want to lint so I strengthened the check to cover this case. I also added macro check, not sure if it's what you had in mind though? |
20590c9 to
ed3c902
Compare
samueltardieu
left a comment
There was a problem hiding this comment.
In the following tests:
macro_rules! mac {
(some $e:expr) => { Some($e) };
(some_map $e:expr) => { Some($e).map(|x| x % 2 == 0) };
(map $e:expr) => { $e.map(|x| x % 2 == 0) };
(eq $a:expr, $b:expr) => { $a == $b };
}
let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true);
let _ = mac!(some_map 2) == Some(true);
let _ = mac!(map Some(2)) == Some(true);
let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true));the latest seems to fail. It should not lint when the equality operator comes from expansion. Comparing the contexts and parent_expr and expr should be enough to fix this.
Also, could you please merge your tests into manual_is_variant_and.rs rather than creating a new test file?
ed3c902 to
494605f
Compare
|
Sure, done! |
494605f to
634f875
Compare
Fixes #14542.
changelog: [
manual_is_variant_and]: Extend to check for boolean map comparisonsr? @samueltardieu