add checking for x -> x and ref x -> x and related test cases.

rust-lang · Mar 10, 2022 · 086b045 · 086b045
1 parent ec91164
commit 086b045
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 19 deletions.
diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs
@@ -5,7 +5,7 @@ use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::{eq_expr_value, get_parent_expr, higher, is_else_clause, is_lang_ctor, peel_blocks_with_stmt};
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::OptionNone;
-use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, Path, PathSegment, QPath};
+use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, Path, PathSegment, QPath, UnOp};
 use rustc_lint::LateContext;
 use rustc_span::sym;
 
@@ -107,6 +107,7 @@ fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
     false
 }
 
+/// Strip `return` keyword if the expression type is `ExprKind::Ret`.
 fn strip_return<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
     if let ExprKind::Ret(Some(ret)) = expr.kind {
         ret
@@ -118,6 +119,7 @@ fn strip_return<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
 fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
     let expr = strip_return(expr);
     match (&pat.kind, &expr.kind) {
+        // Example: `Some(val) => Some(val)`
         (
             PatKind::TupleStruct(QPath::Resolved(_, path), [first_pat, ..], _),
             ExprKind::Call(call_expr, [first_param, ..]),
@@ -130,9 +132,34 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
                 }
             }
         },
+        // Example: `val => val`, or `ref val => *val`
+        (PatKind::Binding(annot, _, pat_ident, _), _) => {
+            let new_expr = if let (
+                BindingAnnotation::Ref | BindingAnnotation::RefMut,
+                ExprKind::Unary(UnOp::Deref, operand_expr),
+            ) = (annot, &expr.kind)
+            {
+                operand_expr
+            } else {
+                expr
+            };
+
+            if let ExprKind::Path(QPath::Resolved(
+                _,
+                Path {
+                    segments: [first_seg, ..],
+                    ..
+                },
+            )) = new_expr.kind
+            {
+                return pat_ident.name == first_seg.ident.name;
+            }
+        },
+        // Example: `Custom::TypeA => Custom::TypeB`, or `None => None`
         (PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => {
             return has_identical_segments(p_path.segments, e_path.segments);
         },
+        // Example: `5 => 5`
         (PatKind::Lit(pat_lit_expr), ExprKind::Lit(expr_spanned)) => {
             if let ExprKind::Lit(pat_spanned) = &pat_lit_expr.kind {
                 return pat_spanned.node == expr_spanned.node;

diff --git a/tests/ui/needless_match.fixed b/tests/ui/needless_match.fixed
@@ -11,8 +11,15 @@ enum Choice {
     D,
 }
 
-fn useless_match(x: i32) {
-    let _: i32 = x;
+#[allow(unused_mut)]
+fn useless_match() {
+    let mut i = 10;
+    let _: i32 = i;
+    let _: i32 = i;
+    let mut _i_mut = i;
+
+    let s = "test";
+    let _: &str = s;
 }
 
 fn custom_type_match(se: Choice) {

diff --git a/tests/ui/needless_match.rs b/tests/ui/needless_match.rs
@@ -11,12 +11,31 @@ enum Choice {
     D,
 }
 
-fn useless_match(x: i32) {
-    let _: i32 = match x {
+#[allow(unused_mut)]
+fn useless_match() {
+    let mut i = 10;
+    let _: i32 = match i {
         0 => 0,
         1 => 1,
         2 => 2,
-        _ => x,
+        _ => i,
+    };
+    let _: i32 = match i {
+        0 => 0,
+        1 => 1,
+        ref i => *i,
+    };
+    let mut _i_mut = match i {
+        0 => 0,
+        1 => 1,
+        ref mut i => *i,
+    };
+
+    let s = "test";
+    let _: &str = match s {
+        "a" => "a",
+        "b" => "b",
+        s => s,
     };
 }
 

diff --git a/tests/ui/needless_match.stderr b/tests/ui/needless_match.stderr
@@ -1,19 +1,52 @@
 error: this match expression is unnecessary
-  --> $DIR/needless_match.rs:15:18
+  --> $DIR/needless_match.rs:17:18
    |
-LL |       let _: i32 = match x {
+LL |       let _: i32 = match i {
    |  __________________^
 LL | |         0 => 0,
 LL | |         1 => 1,
 LL | |         2 => 2,
-LL | |         _ => x,
+LL | |         _ => i,
 LL | |     };
-   | |_____^ help: replace it with: `x`
+   | |_____^ help: replace it with: `i`
    |
    = note: `-D clippy::needless-match` implied by `-D warnings`
 
 error: this match expression is unnecessary
-  --> $DIR/needless_match.rs:24:21
+  --> $DIR/needless_match.rs:23:18
+   |
+LL |       let _: i32 = match i {
+   |  __________________^
+LL | |         0 => 0,
+LL | |         1 => 1,
+LL | |         ref i => *i,
+LL | |     };
+   | |_____^ help: replace it with: `i`
+
+error: this match expression is unnecessary
+  --> $DIR/needless_match.rs:28:22
+   |
+LL |       let mut _i_mut = match i {
+   |  ______________________^
+LL | |         0 => 0,
+LL | |         1 => 1,
+LL | |         ref mut i => *i,
+LL | |     };
+   | |_____^ help: replace it with: `i`
+
+error: this match expression is unnecessary
+  --> $DIR/needless_match.rs:35:19
+   |
+LL |       let _: &str = match s {
+   |  ___________________^
+LL | |         "a" => "a",
+LL | |         "b" => "b",
+LL | |         s => s,
+LL | |     };
+   | |_____^ help: replace it with: `s`
+
+error: this match expression is unnecessary
+  --> $DIR/needless_match.rs:43:21
    |
 LL |       let _: Choice = match se {
    |  _____________________^
@@ -25,7 +58,7 @@ LL | |     };
    | |_____^ help: replace it with: `se`
 
 error: this match expression is unnecessary
-  --> $DIR/needless_match.rs:46:26
+  --> $DIR/needless_match.rs:65:26
    |
 LL |       let _: Option<i32> = match x {
    |  __________________________^
@@ -35,7 +68,7 @@ LL | |     };
    | |_____^ help: replace it with: `x`
 
 error: this match expression is unnecessary
-  --> $DIR/needless_match.rs:62:31
+  --> $DIR/needless_match.rs:81:31
    |
 LL |       let _: Result<i32, i32> = match Ok(1) {
    |  _______________________________^
@@ -45,7 +78,7 @@ LL | |     };
    | |_____^ help: replace it with: `Ok(1)`
 
 error: this match expression is unnecessary
-  --> $DIR/needless_match.rs:66:31
+  --> $DIR/needless_match.rs:85:31
    |
 LL |       let _: Result<i32, i32> = match func_ret_err(0_i32) {
    |  _______________________________^
@@ -55,25 +88,25 @@ LL | |     };
    | |_____^ help: replace it with: `func_ret_err(0_i32)`
 
 error: this if-let expression is unnecessary
-  --> $DIR/needless_match.rs:73:5
+  --> $DIR/needless_match.rs:92:5
    |
 LL |     if let Some(a) = Some(1) { Some(a) } else { None }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(1)`
 
 error: this if-let expression is unnecessary
-  --> $DIR/needless_match.rs:77:30
+  --> $DIR/needless_match.rs:96:30
    |
 LL |     let _: Result<(), i32> = if let Err(e) = x { Err(e) } else { x };
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x`
 
 error: this if-let expression is unnecessary
-  --> $DIR/needless_match.rs:78:30
+  --> $DIR/needless_match.rs:97:30
    |
 LL |     let _: Result<(), i32> = if let Ok(val) = x { Ok(val) } else { x };
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x`
 
 error: this if-let expression is unnecessary
-  --> $DIR/needless_match.rs:84:21
+  --> $DIR/needless_match.rs:103:21
    |
 LL |       let _: Choice = if let Choice::A = x {
    |  _____________________^
@@ -85,5 +118,5 @@ LL | |         x
 LL | |     };
    | |_____^ help: replace it with: `x`
 
-error: aborting due to 9 previous errors
+error: aborting due to 12 previous errors