Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor clippy::match_ref_pats to check for multiple reference patterns #7800

Merged
merged 2 commits into from
Oct 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ where
'b: 'a,
I: Clone + Iterator<Item = &'a Pat<'b>>,
{
if !has_only_ref_pats(pats.clone()) {
if !has_multiple_ref_pats(pats.clone()) {
return;
}

Expand Down Expand Up @@ -1693,26 +1693,26 @@ fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<BindingAnnotat
None
}

fn has_only_ref_pats<'a, 'b, I>(pats: I) -> bool
fn has_multiple_ref_pats<'a, 'b, I>(pats: I) -> bool
where
'b: 'a,
I: Iterator<Item = &'a Pat<'b>>,
{
let mut at_least_one_is_true = false;
let mut ref_count = 0;
for opt in pats.map(|pat| match pat.kind {
PatKind::Ref(..) => Some(true), // &-patterns
PatKind::Wild => Some(false), // an "anything" wildcard is also fine
_ => None, // any other pattern is not fine
}) {
if let Some(inner) = opt {
if inner {
at_least_one_is_true = true;
ref_count += 1;
}
} else {
return false;
}
}
at_least_one_is_true
ref_count > 1
}

pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, &SpannedRange<T>)>
Expand Down
35 changes: 1 addition & 34 deletions tests/ui/match_expr_like_matches_macro.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -110,23 +110,6 @@ LL | | _ => false,
LL | | };
| |_________^ help: try this: `matches!(&val, &Some(ref _a))`

error: you don't need to add `&` to both the expression and the patterns
--> $DIR/match_expr_like_matches_macro.rs:166:20
|
LL | let _res = match &val {
| ____________________^
LL | | &Some(ref _a) => true,
LL | | _ => false,
LL | | };
| |_________^
|
= note: `-D clippy::match-ref-pats` implied by `-D warnings`
help: try
|
LL ~ let _res = match val {
LL ~ Some(ref _a) => true,
|

error: match expression looks like `matches!` macro
--> $DIR/match_expr_like_matches_macro.rs:178:20
|
Expand All @@ -137,21 +120,5 @@ LL | | _ => false,
LL | | };
| |_________^ help: try this: `matches!(&val, &Some(ref _a))`

error: you don't need to add `&` to both the expression and the patterns
--> $DIR/match_expr_like_matches_macro.rs:178:20
|
LL | let _res = match &val {
| ____________________^
LL | | &Some(ref _a) => true,
LL | | _ => false,
LL | | };
| |_________^
|
help: try
|
LL ~ let _res = match val {
LL ~ Some(ref _a) => true,
|

error: aborting due to 14 previous errors
error: aborting due to 12 previous errors

42 changes: 42 additions & 0 deletions tests/ui/match_ref_pats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,46 @@ mod ice_3719 {
}
}

mod issue_7740 {
macro_rules! foobar_variant(
($idx:expr) => (FooBar::get($idx).unwrap())
);

enum FooBar {
Foo,
Bar,
FooBar,
BarFoo,
}

impl FooBar {
fn get(idx: u8) -> Option<&'static Self> {
match idx {
0 => Some(&FooBar::Foo),
1 => Some(&FooBar::Bar),
2 => Some(&FooBar::FooBar),
3 => Some(&FooBar::BarFoo),
_ => None,
}
}
}

fn issue_7740() {
// Issue #7740
match foobar_variant!(0) {
&FooBar::Foo => println!("Foo"),
&FooBar::Bar => println!("Bar"),
&FooBar::FooBar => println!("FooBar"),
_ => println!("Wild"),
}

// This shouldn't trigger
if let &FooBar::BarFoo = foobar_variant!(3) {
println!("BarFoo");
} else {
println!("Wild");
}
}
}

fn main() {}
57 changes: 10 additions & 47 deletions tests/ui/match_ref_pats.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,6 @@ LL ~ Some(v) => println!("{:?}", v),
LL ~ None => println!("none"),
|

error: you don't need to add `&` to all patterns
--> $DIR/match_ref_pats.rs:18:5
|
LL | / match tup {
LL | | &(v, 1) => println!("{}", v),
LL | | _ => println!("none"),
LL | | }
| |_____^
|
help: instead of prefixing all patterns with `&`, you can dereference the expression
|
LL ~ match *tup {
LL ~ (v, 1) => println!("{}", v),
|

error: you don't need to add `&` to both the expression and the patterns
--> $DIR/match_ref_pats.rs:24:5
|
Expand All @@ -54,52 +39,30 @@ LL | if let &None = a {
|
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`

error: you don't need to add `&` to all patterns
--> $DIR/match_ref_pats.rs:36:5
|
LL | / if let &None = a {
LL | | println!("none");
LL | | }
| |_____^
|
help: instead of prefixing all patterns with `&`, you can dereference the expression
|
LL | if let None = *a {
| ~~~~ ~~

error: redundant pattern matching, consider using `is_none()`
--> $DIR/match_ref_pats.rs:41:12
|
LL | if let &None = &b {
| -------^^^^^----- help: try this: `if b.is_none()`

error: you don't need to add `&` to both the expression and the patterns
--> $DIR/match_ref_pats.rs:41:5
|
LL | / if let &None = &b {
LL | | println!("none");
LL | | }
| |_____^
|
help: try
|
LL | if let None = b {
| ~~~~ ~

error: you don't need to add `&` to all patterns
--> $DIR/match_ref_pats.rs:68:9
--> $DIR/match_ref_pats.rs:101:9
|
LL | / match foo_variant!(0) {
LL | | &Foo::A => println!("A"),
LL | / match foobar_variant!(0) {
LL | | &FooBar::Foo => println!("Foo"),
LL | | &FooBar::Bar => println!("Bar"),
LL | | &FooBar::FooBar => println!("FooBar"),
LL | | _ => println!("Wild"),
LL | | }
| |_________^
|
help: instead of prefixing all patterns with `&`, you can dereference the expression
|
LL ~ match *foo_variant!(0) {
LL ~ Foo::A => println!("A"),
LL ~ match *foobar_variant!(0) {
LL ~ FooBar::Foo => println!("Foo"),
LL ~ FooBar::Bar => println!("Bar"),
LL ~ FooBar::FooBar => println!("FooBar"),
|

error: aborting due to 8 previous errors
error: aborting due to 5 previous errors