diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs
index ff85623acf4..e93b494653f 100644
--- a/clippy_lints/src/matches/mod.rs
+++ b/clippy_lints/src/matches/mod.rs
@@ -667,7 +667,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
overlapping_arms::check(cx, ex, arms);
match_wild_enum::check(cx, ex, arms);
match_as_ref::check(cx, ex, arms, expr);
- needless_match::check_match(cx, ex, arms);
+ needless_match::check_match(cx, ex, arms, expr);
if self.infallible_destructuring_match_linted {
self.infallible_destructuring_match_linted = false;
diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs
index d9544e18b18..2830ae1f61d 100644
--- a/clippy_lints/src/matches/needless_match.rs
+++ b/clippy_lints/src/matches/needless_match.rs
@@ -1,37 +1,25 @@
use super::NEEDLESS_MATCH;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
-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 clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts};
+use clippy_utils::{
+ eq_expr_value, get_parent_expr_for_hir, get_parent_node, 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, FnRetTy, Node, Pat, PatKind, Path, PathSegment, QPath};
use rustc_lint::LateContext;
use rustc_span::sym;
+use rustc_typeck::hir_ty_to_ty;
-pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
- // This is for avoiding collision with `match_single_binding`.
- if arms.len() < 2 {
- return;
- }
-
- for arm in arms {
- if let PatKind::Wild = arm.pat.kind {
- let ret_expr = strip_return(arm.body);
- if !eq_expr_value(cx, ex, ret_expr) {
- return;
- }
- } else if !pat_same_as_expr(arm.pat, peel_blocks_with_stmt(arm.body)) {
- return;
- }
- }
-
- if let Some(match_expr) = get_parent_expr(cx, ex) {
+pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
+ if arms.len() > 1 && !is_coercion_casting(cx, ex, expr) && check_all_arms(cx, ex, arms) {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
NEEDLESS_MATCH,
- match_expr.span,
+ expr.span,
"this match expression is unnecessary",
"replace it with",
snippet_with_applicability(cx, ex.span, "..", &mut applicability).to_string(),
@@ -60,11 +48,8 @@ pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
/// }
/// ```
pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
- if_chain! {
- if let Some(ref if_let) = higher::IfLet::hir(cx, ex);
- if !is_else_clause(cx.tcx, ex);
- if check_if_let(cx, if_let);
- then {
+ if let Some(ref if_let) = higher::IfLet::hir(cx, ex) {
+ if !is_else_clause(cx.tcx, ex) && !is_coercion_casting(cx, if_let.let_expr, ex) && check_if_let(cx, if_let) {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
@@ -79,6 +64,19 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
}
}
+fn check_all_arms(cx: &LateContext<'_>, match_expr: &Expr<'_>, arms: &[Arm<'_>]) -> bool {
+ for arm in arms {
+ let arm_expr = peel_blocks_with_stmt(arm.body);
+ if let PatKind::Wild = arm.pat.kind {
+ return eq_expr_value(cx, match_expr, strip_return(arm_expr));
+ } else if !pat_same_as_expr(arm.pat, arm_expr) {
+ return false;
+ }
+ }
+
+ true
+}
+
fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
if let Some(if_else) = if_let.if_else {
if !pat_same_as_expr(if_let.let_pat, peel_blocks_with_stmt(if_let.if_then)) {
@@ -101,12 +99,12 @@ fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
if let ExprKind::Path(ref qpath) = ret.kind {
return is_lang_ctor(cx, qpath, OptionNone) || eq_expr_value(cx, if_let.let_expr, ret);
}
- } else {
- return eq_expr_value(cx, if_let.let_expr, ret);
+ return true;
}
- return true;
+ return eq_expr_value(cx, if_let.let_expr, ret);
}
}
+
false
}
@@ -119,14 +117,52 @@ fn strip_return<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
}
}
+/// Manually check for coercion casting by checking if the type of the match operand or let expr
+/// differs with the assigned local variable or the funtion return type.
+fn is_coercion_casting(cx: &LateContext<'_>, match_expr: &Expr<'_>, expr: &Expr<'_>) -> bool {
+ if let Some(p_node) = get_parent_node(cx.tcx, expr.hir_id) {
+ match p_node {
+ // Compare match_expr ty with local in `let local = match match_expr {..}`
+ Node::Local(local) => {
+ let results = cx.typeck_results();
+ return !same_type_and_consts(results.node_type(local.hir_id), results.expr_ty(match_expr));
+ },
+ // compare match_expr ty with RetTy in `fn foo() -> RetTy`
+ Node::Item(..) => {
+ if let Some(fn_decl) = p_node.fn_decl() {
+ if let FnRetTy::Return(ret_ty) = fn_decl.output {
+ return !same_type_and_consts(
+ hir_ty_to_ty(cx.tcx, ret_ty),
+ cx.typeck_results().expr_ty(match_expr),
+ );
+ }
+ }
+ },
+ // check the parent expr for this whole block `{ match match_expr {..} }`
+ Node::Block(block) => {
+ if let Some(block_parent_expr) = get_parent_expr_for_hir(cx, block.hir_id) {
+ return is_coercion_casting(cx, match_expr, block_parent_expr);
+ }
+ },
+ // recursively call on `if xxx {..}` etc.
+ Node::Expr(p_expr) => {
+ return is_coercion_casting(cx, match_expr, p_expr);
+ },
+ _ => {},
+ }
+ }
+
+ false
+}
+
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), tuple_params, _), ExprKind::Call(call_expr, call_params)) => {
if let ExprKind::Path(QPath::Resolved(_, call_path)) = call_expr.kind {
- return has_identical_segments(path.segments, call_path.segments)
- && has_same_non_ref_symbols(tuple_params, call_params);
+ return same_segments(path.segments, call_path.segments)
+ && same_non_ref_symbols(tuple_params, call_params);
}
},
// Example: `val => val`
@@ -145,7 +181,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
},
// 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);
+ return same_segments(p_path.segments, e_path.segments);
},
// Example: `5 => 5`
(PatKind::Lit(pat_lit_expr), ExprKind::Lit(expr_spanned)) => {
@@ -159,19 +195,21 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
false
}
-fn has_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
+fn same_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
if left_segs.len() != right_segs.len() {
return false;
}
+
for i in 0..left_segs.len() {
if left_segs[i].ident.name != right_segs[i].ident.name {
return false;
}
}
+
true
}
-fn has_same_non_ref_symbols(pats: &[Pat<'_>], exprs: &[Expr<'_>]) -> bool {
+fn same_non_ref_symbols(pats: &[Pat<'_>], exprs: &[Expr<'_>]) -> bool {
if pats.len() != exprs.len() {
return false;
}
diff --git a/tests/ui/needless_match.fixed b/tests/ui/needless_match.fixed
index 6a518f83925..d55dd589a54 100644
--- a/tests/ui/needless_match.fixed
+++ b/tests/ui/needless_match.fixed
@@ -60,8 +60,26 @@ fn result_match() {
};
}
-fn if_let_option() -> Option<i32> {
- Some(1)
+fn if_let_option() {
+ let _ = Some(1);
+
+ fn do_something() {}
+
+ // Don't trigger
+ let _ = if let Some(a) = Some(1) {
+ Some(a)
+ } else {
+ do_something();
+ None
+ };
+
+ // Don't trigger
+ let _ = if let Some(a) = Some(1) {
+ do_something();
+ Some(a)
+ } else {
+ None
+ };
}
fn if_let_result() {
@@ -122,25 +140,45 @@ mod issue8542 {
_ => ce,
};
}
+}
- fn if_let_test() {
- fn do_something() {}
+/// Lint triggered when type coercions happen.
+/// Do NOT trigger on any of these.
+mod issue8551 {
+ trait Trait {}
+ struct Struct;
+ impl Trait for Struct {}
- // Don't trigger
- let _ = if let Some(a) = Some(1) {
- Some(a)
- } else {
- do_something();
- None
+ fn optmap(s: Option<&Struct>) -> Option<&dyn Trait> {
+ match s {
+ Some(s) => Some(s),
+ None => None,
+ }
+ }
+
+ fn lint_tests() {
+ let option: Option<&Struct> = None;
+ let _: Option<&dyn Trait> = match option {
+ Some(s) => Some(s),
+ None => None,
};
- // Don't trigger
- let _ = if let Some(a) = Some(1) {
- do_something();
- Some(a)
+ let _: Option<&dyn Trait> = if true {
+ match option {
+ Some(s) => Some(s),
+ None => None,
+ }
} else {
None
};
+
+ let result: Result<&Struct, i32> = Err(0);
+ let _: Result<&dyn Trait, i32> = match result {
+ Ok(s) => Ok(s),
+ Err(e) => Err(e),
+ };
+
+ let _: Option<&dyn Trait> = if let Some(s) = option { Some(s) } else { None };
}
}
diff --git a/tests/ui/needless_match.rs b/tests/ui/needless_match.rs
index ffc6a291a15..f3c7ee01a47 100644
--- a/tests/ui/needless_match.rs
+++ b/tests/ui/needless_match.rs
@@ -83,8 +83,26 @@ fn result_match() {
};
}
-fn if_let_option() -> Option<i32> {
- if let Some(a) = Some(1) { Some(a) } else { None }
+fn if_let_option() {
+ let _ = if let Some(a) = Some(1) { Some(a) } else { None };
+
+ fn do_something() {}
+
+ // Don't trigger
+ let _ = if let Some(a) = Some(1) {
+ Some(a)
+ } else {
+ do_something();
+ None
+ };
+
+ // Don't trigger
+ let _ = if let Some(a) = Some(1) {
+ do_something();
+ Some(a)
+ } else {
+ None
+ };
}
fn if_let_result() {
@@ -159,25 +177,45 @@ mod issue8542 {
_ => ce,
};
}
+}
- fn if_let_test() {
- fn do_something() {}
+/// Lint triggered when type coercions happen.
+/// Do NOT trigger on any of these.
+mod issue8551 {
+ trait Trait {}
+ struct Struct;
+ impl Trait for Struct {}
- // Don't trigger
- let _ = if let Some(a) = Some(1) {
- Some(a)
- } else {
- do_something();
- None
+ fn optmap(s: Option<&Struct>) -> Option<&dyn Trait> {
+ match s {
+ Some(s) => Some(s),
+ None => None,
+ }
+ }
+
+ fn lint_tests() {
+ let option: Option<&Struct> = None;
+ let _: Option<&dyn Trait> = match option {
+ Some(s) => Some(s),
+ None => None,
};
- // Don't trigger
- let _ = if let Some(a) = Some(1) {
- do_something();
- Some(a)
+ let _: Option<&dyn Trait> = if true {
+ match option {
+ Some(s) => Some(s),
+ None => None,
+ }
} else {
None
};
+
+ let result: Result<&Struct, i32> = Err(0);
+ let _: Result<&dyn Trait, i32> = match result {
+ Ok(s) => Ok(s),
+ Err(e) => Err(e),
+ };
+
+ let _: Option<&dyn Trait> = if let Some(s) = option { Some(s) } else { None };
}
}
diff --git a/tests/ui/needless_match.stderr b/tests/ui/needless_match.stderr
index 67bd84d6bbc..34c5226f060 100644
--- a/tests/ui/needless_match.stderr
+++ b/tests/ui/needless_match.stderr
@@ -66,25 +66,25 @@ LL | | };
| |_____^ help: replace it with: `func_ret_err(0_i32)`
error: this if-let expression is unnecessary
- --> $DIR/needless_match.rs:87:5
+ --> $DIR/needless_match.rs:87:13
|
-LL | if let Some(a) = Some(1) { Some(a) } else { None }
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(1)`
+LL | let _ = 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:92:31
+ --> $DIR/needless_match.rs:110:31
|
LL | let _: Result<i32, 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:93:31
+ --> $DIR/needless_match.rs:111:31
|
LL | let _: Result<i32, 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:99:21
+ --> $DIR/needless_match.rs:117:21
|
LL | let _: Simple = if let Simple::A = x {
| _____________________^
@@ -97,7 +97,7 @@ LL | | };
| |_____^ help: replace it with: `x`
error: this match expression is unnecessary
- --> $DIR/needless_match.rs:138:26
+ --> $DIR/needless_match.rs:156:26
|
LL | let _: Complex = match ce {
| __________________________^