Auto merge of #10831 - disco07:master, r=flip1995

Fix redundant_pattern_match on matches! macro This PR solve this issue #10803 r? `@flip1995` changelog: none
rust-lang · May 26, 2023 · 2422594 · 2422594
2 parents 05740ad + 0b507c6
commit 2422594
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 48 deletions.
diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs
@@ -25,7 +25,7 @@ mod wild_in_or_pats;
 
 use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::{snippet_opt, walk_span_to_context};
-use clippy_utils::{higher, in_constant, is_span_match, tokenize_with_text};
+use clippy_utils::{higher, in_constant, is_direct_expn_of, is_span_match, tokenize_with_text};
 use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
 use rustc_lexer::TokenKind;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -974,12 +974,16 @@ impl_lint_pass!(Matches => [
 
 impl<'tcx> LateLintPass<'tcx> for Matches {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if in_external_macro(cx.sess(), expr.span) {
+        if is_direct_expn_of(expr.span, "matches").is_none() && in_external_macro(cx.sess(), expr.span) {
             return;
         }
         let from_expansion = expr.span.from_expansion();
 
         if let ExprKind::Match(ex, arms, source) = expr.kind {
+            if is_direct_expn_of(expr.span, "matches").is_some() {
+                redundant_pattern_match::check_match(cx, expr, ex, arms);
+            }
+
             if source == MatchSource::Normal && !is_span_match(cx, expr.span) {
                 return;
             }

diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs
@@ -1,10 +1,10 @@
 use super::REDUNDANT_PATTERN_MATCHING;
-use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet, walk_span_to_context};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop};
 use clippy_utils::visitors::any_temporaries_need_ordered_drop;
-use clippy_utils::{higher, is_trait_method};
+use clippy_utils::{higher, is_expn_of, is_trait_method};
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -190,24 +190,19 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
         let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
 
         if let Some(good_method) = found_good_method(cx, arms, node_pair) {
-            let span = expr.span.to(op.span);
+            let span = is_expn_of(expr.span, "matches").unwrap_or(expr.span.to(op.span));
             let result_expr = match &op.kind {
                 ExprKind::AddrOf(_, _, borrowed) => borrowed,
                 _ => op,
             };
-            span_lint_and_then(
+            span_lint_and_sugg(
                 cx,
                 REDUNDANT_PATTERN_MATCHING,
-                expr.span,
+                span,
                 &format!("redundant pattern matching, consider using `{good_method}`"),
-                |diag| {
-                    diag.span_suggestion(
-                        span,
-                        "try this",
-                        format!("{}.{good_method}", snippet(cx, result_expr.span, "_")),
-                        Applicability::MaybeIncorrect, // snippet
-                    );
-                },
+                "try this",
+                format!("{}.{good_method}", snippet(cx, result_expr.span, "_")),
+                Applicability::MachineApplicable,
             );
         }
     }

diff --git a/tests/ui/redundant_pattern_matching_option.fixed b/tests/ui/redundant_pattern_matching_option.fixed
@@ -47,6 +47,7 @@ fn main() {
 
     issue6067();
     issue10726();
+    issue10803();
 
     let _ = if gen_opt().is_some() {
         1
@@ -107,3 +108,14 @@ fn issue10726() {
         _ => false,
     };
 }
+
+fn issue10803() {
+    let x = Some(42);
+
+    let _ = x.is_some();
+
+    let _ = x.is_none();
+
+    // Don't lint
+    let _ = matches!(x, Some(16));
+}
diff --git a/tests/ui/redundant_pattern_matching_option.rs b/tests/ui/redundant_pattern_matching_option.rs
@@ -56,6 +56,7 @@ fn main() {
 
     issue6067();
     issue10726();
+    issue10803();
 
     let _ = if let Some(_) = gen_opt() {
         1
@@ -134,3 +135,14 @@ fn issue10726() {
         _ => false,
     };
 }
+
+fn issue10803() {
+    let x = Some(42);
+
+    let _ = matches!(x, Some(_));
+
+    let _ = matches!(x, None);
+
+    // Don't lint
+    let _ = matches!(x, Some(16));
+}
diff --git a/tests/ui/redundant_pattern_matching_option.stderr b/tests/ui/redundant_pattern_matching_option.stderr
@@ -77,49 +77,49 @@ LL |     let _ = if let Some(_) = opt { true } else { false };
    |             -------^^^^^^^------ help: try this: `if opt.is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:60:20
+  --> $DIR/redundant_pattern_matching_option.rs:61:20
    |
 LL |     let _ = if let Some(_) = gen_opt() {
    |             -------^^^^^^^------------ help: try this: `if gen_opt().is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:62:19
+  --> $DIR/redundant_pattern_matching_option.rs:63:19
    |
 LL |     } else if let None = gen_opt() {
    |            -------^^^^------------ help: try this: `if gen_opt().is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:68:12
+  --> $DIR/redundant_pattern_matching_option.rs:69:12
    |
 LL |     if let Some(..) = gen_opt() {}
    |     -------^^^^^^^^------------ help: try this: `if gen_opt().is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:83:12
+  --> $DIR/redundant_pattern_matching_option.rs:84:12
    |
 LL |     if let Some(_) = Some(42) {}
    |     -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:85:12
+  --> $DIR/redundant_pattern_matching_option.rs:86:12
    |
 LL |     if let None = None::<()> {}
    |     -------^^^^------------- help: try this: `if None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:87:15
+  --> $DIR/redundant_pattern_matching_option.rs:88:15
    |
 LL |     while let Some(_) = Some(42) {}
    |     ----------^^^^^^^----------- help: try this: `while Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:89:15
+  --> $DIR/redundant_pattern_matching_option.rs:90:15
    |
 LL |     while let None = None::<()> {}
    |     ----------^^^^------------- help: try this: `while None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:91:5
+  --> $DIR/redundant_pattern_matching_option.rs:92:5
    |
 LL | /     match Some(42) {
 LL | |         Some(_) => true,
@@ -128,7 +128,7 @@ LL | |     };
    | |_____^ help: try this: `Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:96:5
+  --> $DIR/redundant_pattern_matching_option.rs:97:5
    |
 LL | /     match None::<()> {
 LL | |         Some(_) => false,
@@ -137,19 +137,19 @@ LL | |     };
    | |_____^ help: try this: `None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:104:12
+  --> $DIR/redundant_pattern_matching_option.rs:105:12
    |
 LL |     if let None = *(&None::<()>) {}
    |     -------^^^^----------------- help: try this: `if (&None::<()>).is_none()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:105:12
+  --> $DIR/redundant_pattern_matching_option.rs:106:12
    |
 LL |     if let None = *&None::<()> {}
    |     -------^^^^--------------- help: try this: `if (&None::<()>).is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:111:5
+  --> $DIR/redundant_pattern_matching_option.rs:112:5
    |
 LL | /     match x {
 LL | |         Some(_) => true,
@@ -158,7 +158,7 @@ LL | |     };
    | |_____^ help: try this: `x.is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:116:5
+  --> $DIR/redundant_pattern_matching_option.rs:117:5
    |
 LL | /     match x {
 LL | |         None => true,
@@ -167,7 +167,7 @@ LL | |     };
    | |_____^ help: try this: `x.is_none()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:121:5
+  --> $DIR/redundant_pattern_matching_option.rs:122:5
    |
 LL | /     match x {
 LL | |         Some(_) => false,
@@ -176,13 +176,25 @@ LL | |     };
    | |_____^ help: try this: `x.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:126:5
+  --> $DIR/redundant_pattern_matching_option.rs:127:5
    |
 LL | /     match x {
 LL | |         None => false,
 LL | |         _ => true,
 LL | |     };
    | |_____^ help: try this: `x.is_some()`
 
-error: aborting due to 26 previous errors
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/redundant_pattern_matching_option.rs:142:13
+   |
+LL |     let _ = matches!(x, Some(_));
+   |             ^^^^^^^^^^^^^^^^^^^^ help: try this: `x.is_some()`
+
+error: redundant pattern matching, consider using `is_none()`
+  --> $DIR/redundant_pattern_matching_option.rs:144:13
+   |
+LL |     let _ = matches!(x, None);
+   |             ^^^^^^^^^^^^^^^^^ help: try this: `x.is_none()`
+
+error: aborting due to 28 previous errors
 
diff --git a/tests/ui/redundant_pattern_matching_result.fixed b/tests/ui/redundant_pattern_matching_result.fixed
@@ -44,6 +44,7 @@ fn main() {
     issue6067();
     issue6065();
     issue10726();
+    issue10803();
 
     let _ = if gen_res().is_ok() {
         1
@@ -133,3 +134,17 @@ fn issue10726() {
         _ => true,
     };
 }
+
+fn issue10803() {
+    let x: Result<i32, i32> = Ok(42);
+
+    let _ = x.is_ok();
+
+    let _ = x.is_err();
+
+    // Don't lint
+    let _ = matches!(x, Ok(16));
+
+    // Don't lint
+    let _ = matches!(x, Err(16));
+}
diff --git a/tests/ui/redundant_pattern_matching_result.rs b/tests/ui/redundant_pattern_matching_result.rs
@@ -56,6 +56,7 @@ fn main() {
     issue6067();
     issue6065();
     issue10726();
+    issue10803();
 
     let _ = if let Ok(_) = gen_res() {
         1
@@ -163,3 +164,17 @@ fn issue10726() {
         _ => true,
     };
 }
+
+fn issue10803() {
+    let x: Result<i32, i32> = Ok(42);
+
+    let _ = matches!(x, Ok(_));
+
+    let _ = matches!(x, Err(_));
+
+    // Don't lint
+    let _ = matches!(x, Ok(16));
+
+    // Don't lint
+    let _ = matches!(x, Err(16));
+}