about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-10-10 11:13:05 +0000
committerbors <bors@rust-lang.org>2021-10-10 11:13:05 +0000
commit9205e3d800c09c826d6ef6533a00d231cfce8f5a (patch)
tree440c33e5fdbe32fa8aa769609636214ea53f36fa
parent8f1555f123be0c58390679c8aa99dca9b79c1bf3 (diff)
parent1080f6c70c132ef20bf0310c1845c92f55a3d5b5 (diff)
downloadrust-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.rs10
-rw-r--r--tests/ui/match_expr_like_matches_macro.stderr35
-rw-r--r--tests/ui/match_ref_pats.rs42
-rw-r--r--tests/ui/match_ref_pats.stderr57
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