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, over,
+ 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, ByRef, Expr, ExprKind, FnRetTy, Guard, Node, Pat, PatKind, Path, QPath};
+use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::LateContext;
use rustc_span::sym;
-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, 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 && expr_ty_matches_p_ty(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(),
/// some_enum
/// }
/// ```
-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 {
- let mut applicability = Applicability::MachineApplicable;
- span_lint_and_sugg(
- cx,
- NEEDLESS_MATCH,
- ex.span,
- "this if-let expression is unnecessary",
- "replace it with",
- snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
- applicability,
- );
+pub(crate) fn check_if_let<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'_>, if_let: &higher::IfLet<'tcx>) {
+ if !is_else_clause(cx.tcx, ex) && expr_ty_matches_p_ty(cx, if_let.let_expr, ex) && check_if_let_inner(cx, if_let) {
+ let mut applicability = Applicability::MachineApplicable;
+ span_lint_and_sugg(
+ cx,
+ NEEDLESS_MATCH,
+ ex.span,
+ "this if-let expression is unnecessary",
+ "replace it with",
+ snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
+ applicability,
+ );
+ }
+}
+
+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 Some(guard_expr) = &arm.guard {
+ match guard_expr {
+ // gives up if `pat if expr` can have side effects
+ Guard::If(if_cond) => {
+ if if_cond.can_have_side_effects() {
+ return false;
+ }
+ },
+ // gives up `pat if let ...` arm
+ Guard::IfLet(_) => {
+ return false;
+ },
+ };
+ }
+
+ if let PatKind::Wild = arm.pat.kind {
+ if !eq_expr_value(cx, match_expr, strip_return(arm_expr)) {
+ return false;
+ }
+ } else if !pat_same_as_expr(arm.pat, arm_expr) {
+ return false;
}
}
+
+ true
}
-fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
+fn check_if_let_inner(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)) {
return false;
}
- // Recurrsively check for each `else if let` phrase,
+ // Recursively check for each `else if let` phrase,
if let Some(ref nested_if_let) = higher::IfLet::hir(cx, if_else) {
- return check_if_let(cx, nested_if_let);
+ return check_if_let_inner(cx, nested_if_let);
}
if matches!(if_else.kind, ExprKind::Block(..)) {
let else_expr = peel_blocks_with_stmt(if_else);
+ if matches!(else_expr.kind, ExprKind::Block(..)) {
+ return false;
+ }
let ret = strip_return(else_expr);
let let_expr_ty = cx.typeck_results().expr_ty(if_let.let_expr);
if is_type_diagnostic_item(cx, let_expr_ty, sym::Option) {
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 false;
}
- return true;
+ return eq_expr_value(cx, if_let.let_expr, ret);
}
}
+
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
}
}
+/// 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 function return type.
+fn expr_ty_matches_p_ty(cx: &LateContext<'_>, expr: &Expr<'_>, p_expr: &Expr<'_>) -> bool {
+ if let Some(p_node) = get_parent_node(cx.tcx, p_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(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(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 expr_ty_matches_p_ty(cx, expr, block_parent_expr);
+ }
+ },
+ // recursively call on `if xxx {..}` etc.
+ Node::Expr(p_expr) => {
+ return expr_ty_matches_p_ty(cx, expr, p_expr);
+ },
+ _ => {},
+ }
+ }
+ false
+}
+
fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
let expr = strip_return(expr);
match (&pat.kind, &expr.kind) {
- (
- PatKind::TupleStruct(QPath::Resolved(_, path), [first_pat, ..], _),
- ExprKind::Call(call_expr, [first_param, ..]),
- ) => {
+ // 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 {
- if has_identical_segments(path.segments, call_path.segments)
- && has_same_non_ref_symbol(first_pat, first_param)
- {
- return true;
- }
+ return over(path.segments, call_path.segments, |pat_seg, call_seg| {
+ pat_seg.ident.name == call_seg.ident.name
+ }) && same_non_ref_symbols(tuple_params, call_params);
}
},
+ // Example: `val => val`
+ (
+ PatKind::Binding(annot, _, pat_ident, _),
+ ExprKind::Path(QPath::Resolved(
+ _,
+ Path {
+ segments: [first_seg, ..],
+ ..
+ },
+ )),
+ ) => {
+ return !matches!(annot, BindingAnnotation(ByRef::Yes, _)) && 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);
+ return over(p_path.segments, e_path.segments, |p_seg, e_seg| {
+ p_seg.ident.name == e_seg.ident.name
+ });
},
+ // 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;
false
}
-fn has_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
- if left_segs.len() != right_segs.len() {
+fn same_non_ref_symbols(pats: &[Pat<'_>], exprs: &[Expr<'_>]) -> bool {
+ if pats.len() != exprs.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_symbol(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
- if_chain! {
- if let PatKind::Binding(annot, _, pat_ident, _) = pat.kind;
- if !matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut);
- if let ExprKind::Path(QPath::Resolved(_, Path {segments: [first_seg, ..], .. })) = expr.kind;
- then {
- return pat_ident.name == first_seg.ident.name;
+ for i in 0..pats.len() {
+ if !pat_same_as_expr(&pats[i], &exprs[i]) {
+ return false;
}
}
- false
+ true
}
projects / rust.git / blobdiff