diff options
| author | bors <bors@rust-lang.org> | 2021-10-10 11:13:05 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-10-10 11:13:05 +0000 |
| commit | 9205e3d800c09c826d6ef6533a00d231cfce8f5a (patch) | |
| tree | 440c33e5fdbe32fa8aa769609636214ea53f36fa | |
| parent | 8f1555f123be0c58390679c8aa99dca9b79c1bf3 (diff) | |
| parent | 1080f6c70c132ef20bf0310c1845c92f55a3d5b5 (diff) | |
| download | rust-9205e3d800c09c826d6ef6533a00d231cfce8f5a.tar.gz rust-9205e3d800c09c826d6ef6533a00d231cfce8f5a.zip | |
Auto merge of #7800 - 1nF0rmed:no-lint-in-match-single-amp, r=llogiq
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
| -rw-r--r-- | clippy_lints/src/matches.rs | 10 | ||||
| -rw-r--r-- | tests/ui/match_expr_like_matches_macro.stderr | 35 | ||||
| -rw-r--r-- | tests/ui/match_ref_pats.rs | 42 | ||||
| -rw-r--r-- | tests/ui/match_ref_pats.stderr | 57 |
4 files changed, 58 insertions, 86 deletions
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 56d4163a6b3..b643fba5d32 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -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; } @@ -1693,12 +1693,12 @@ 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 @@ -1706,13 +1706,13 @@ where }) { 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>)> diff --git a/tests/ui/match_expr_like_matches_macro.stderr b/tests/ui/match_expr_like_matches_macro.stderr index 366ef36c367..d7cedf9f9f1 100644 --- a/tests/ui/match_expr_like_matches_macro.stderr +++ b/tests/ui/match_expr_like_matches_macro.stderr @@ -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 | @@ -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 diff --git a/tests/ui/match_ref_pats.rs b/tests/ui/match_ref_pats.rs index 6cbb4d32b0d..50246486bb6 100644 --- a/tests/ui/match_ref_pats.rs +++ b/tests/ui/match_ref_pats.rs @@ -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() {} diff --git a/tests/ui/match_ref_pats.stderr b/tests/ui/match_ref_pats.stderr index 072aff445e9..901820077e2 100644 --- a/tests/ui/match_ref_pats.stderr +++ b/tests/ui/match_ref_pats.stderr @@ -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 | @@ -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 |
