From 086b045822b008c5b4e6eb34bac10bfa8d5a7ea1 Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Thu, 10 Mar 2022 14:46:58 +0800 Subject: [PATCH] add checking for `x -> x` and `ref x -> x` and related test cases. --- clippy_lints/src/matches/needless_match.rs | 29 ++++++++++- tests/ui/needless_match.fixed | 11 +++- tests/ui/needless_match.rs | 25 +++++++-- tests/ui/needless_match.stderr | 59 +++++++++++++++++----- 4 files changed, 105 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index 8d209c628..76131d307 100644 --- 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 index eb6494a4d..ece18ad73 100644 --- 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 index 43f1391d6..36649de35 100644 --- 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 index d11d4fd33..ad1525406 100644 --- 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 = 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 = 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 = 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