Skip to content

Commit

Permalink
Auto merge of #7800 - 1nF0rmed:no-lint-in-match-single-amp, r=llogiq
Browse files Browse the repository at this point in the history
Refactor `clippy::match_ref_pats` to check for multiple reference patterns

fixes #7740

When there is only one pattern, to begin with, i.e. a single deref(`&`), then in such cases we suppress `clippy::match_ref_pats`.
This is done by checking the count of the reference pattern and emitting `clippy::match_ref_pats` when more than one pattern is present.

Removed certain errors in the `stderr` tests as they would not be triggered.

changelog: Refactor `clippy::match_ref_pats` to check for multiple reference patterns
  • Loading branch information
bors committed Oct 10, 2021
2 parents 8f1555f + 1080f6c commit 9205e3d
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 86 deletions.
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

0 comments on commit 9205e3d

Please sign in to comment.