diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 2239b5196..6ecd738d2 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -4,8 +4,8 @@ use crate::utils::usage::is_unused; use crate::utils::{ expr_block, get_arg_name, get_parent_expr, implements_trait, in_macro, indent_of, is_allowed, is_expn_of, is_refutable, is_type_diagnostic_item, is_wild, match_qpath, match_type, match_var, meets_msrv, multispan_sugg, - remove_blocks, snippet, snippet_block, snippet_opt, snippet_with_applicability, span_lint_and_help, - span_lint_and_note, span_lint_and_sugg, span_lint_and_then, + peel_hir_pat_refs, peel_mid_ty_refs, peeln_hir_expr_refs, remove_blocks, snippet, snippet_block, snippet_opt, + snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, }; use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash}; use if_chain::if_chain; @@ -717,28 +717,6 @@ fn check_single_match_single_pattern( } } -fn peel_pat_refs(pat: &'a Pat<'a>) -> (&'a Pat<'a>, usize) { - fn peel(pat: &'a Pat<'a>, count: usize) -> (&'a Pat<'a>, usize) { - if let PatKind::Ref(pat, _) = pat.kind { - peel(pat, count + 1) - } else { - (pat, count) - } - } - peel(pat, 0) -} - -fn peel_ty_refs(ty: Ty<'_>) -> (Ty<'_>, usize) { - fn peel(ty: Ty<'_>, count: usize) -> (Ty<'_>, usize) { - if let ty::Ref(_, ty, _) = ty.kind() { - peel(ty, count + 1) - } else { - (ty, count) - } - } - peel(ty, 0) -} - fn report_single_match_single_pattern( cx: &LateContext<'_>, ex: &Expr<'_>, @@ -752,9 +730,9 @@ fn report_single_match_single_pattern( }); let (msg, sugg) = if_chain! { - let (pat, pat_ref_count) = peel_pat_refs(arms[0].pat); + let (pat, pat_ref_count) = peel_hir_pat_refs(arms[0].pat); if let PatKind::Path(_) | PatKind::Lit(_) = pat.kind; - let (ty, ty_ref_count) = peel_ty_refs(cx.typeck_results().expr_ty(ex)); + let (ty, ty_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(ex)); if let Some(trait_id) = cx.tcx.lang_items().structural_peq_trait(); if ty.is_integral() || ty.is_char() || ty.is_str() || implements_trait(cx, ty, trait_id, &[]); then { @@ -764,19 +742,28 @@ fn report_single_match_single_pattern( PatKind::Lit(Expr { kind: ExprKind::Lit(lit), .. }) if lit.node.is_str() => pat_ref_count + 1, _ => pat_ref_count, }; - let msg = "you seem to be trying to use match for an equality check. Consider using `if`"; + // References are only implicitly added to the pattern, so no overflow here. + // e.g. will work: match &Some(_) { Some(_) => () } + // will not: match Some(_) { &Some(_) => () } + let ref_count_diff = ty_ref_count - pat_ref_count; + + // Try to remove address of expressions first. + let (ex, removed) = peeln_hir_expr_refs(ex, ref_count_diff); + let ref_count_diff = ref_count_diff - removed; + + let msg = "you seem to be trying to use `match` for an equality check. Consider using `if`"; let sugg = format!( "if {} == {}{} {}{}", snippet(cx, ex.span, ".."), // PartialEq for different reference counts may not exist. - "&".repeat(ty_ref_count - pat_ref_count), + "&".repeat(ref_count_diff), snippet(cx, arms[0].pat.span, ".."), expr_block(cx, &arms[0].body, None, "..", Some(expr.span)), els_str, ); (msg, sugg) } else { - let msg = "you seem to be trying to use match for destructuring a single pattern. Consider using `if let`"; + let msg = "you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`"; let sugg = format!( "if let {} = {} {}{}", snippet(cx, arms[0].pat.span, ".."), diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 8f54cad77..8f8c681ec 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -1668,6 +1668,44 @@ where match_expr_list } +/// Peels off all references on the pattern. Returns the underlying pattern and the number of +/// references removed. +pub fn peel_hir_pat_refs(pat: &'a Pat<'a>) -> (&'a Pat<'a>, usize) { + fn peel(pat: &'a Pat<'a>, count: usize) -> (&'a Pat<'a>, usize) { + if let PatKind::Ref(pat, _) = pat.kind { + peel(pat, count + 1) + } else { + (pat, count) + } + } + peel(pat, 0) +} + +/// Peels off up to the given number of references on the expression. Returns the underlying +/// expression and the number of references removed. +pub fn peeln_hir_expr_refs(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'a>, usize) { + fn f(expr: &'a Expr<'a>, count: usize, target: usize) -> (&'a Expr<'a>, usize) { + match expr.kind { + ExprKind::AddrOf(_, _, expr) if count != target => f(expr, count + 1, target), + _ => (expr, count), + } + } + f(expr, 0, count) +} + +/// Peels off all references on the type. Returns the underlying type and the number of references +/// removed. +pub fn peel_mid_ty_refs(ty: Ty<'_>) -> (Ty<'_>, usize) { + fn peel(ty: Ty<'_>, count: usize) -> (Ty<'_>, usize) { + if let ty::Ref(_, ty, _) = ty.kind() { + peel(ty, count + 1) + } else { + (ty, count) + } + } + peel(ty, 0) +} + #[macro_export] macro_rules! unwrap_cargo_metadata { ($cx: ident, $lint: ident, $deps: expr) => {{ diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index 02266308f..ca884b41c 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -81,7 +81,8 @@ fn single_match_know_enum() { } } -fn issue_173() { +// issue #173 +fn if_suggestion() { let x = "test"; match x { "test" => println!(), @@ -106,6 +107,18 @@ fn issue_173() { FOO_C => println!(), _ => (), } + + match &&x { + Foo::A => println!(), + _ => (), + } + + let x = &x; + match &x { + Foo::A => println!(), + _ => (), + } + enum Bar { A, B, diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index 5eca07ab1..7ea6955b7 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -1,4 +1,4 @@ -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:8:5 | LL | / match x { @@ -17,7 +17,7 @@ LL | println!("{:?}", y); LL | }; | -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:16:5 | LL | / match x { @@ -29,7 +29,7 @@ LL | | _ => (), LL | | } | |_____^ help: try this: `if let Some(y) = x { println!("{:?}", y) }` -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:25:5 | LL | / match z { @@ -38,7 +38,7 @@ LL | | _ => {}, LL | | }; | |_____^ help: try this: `if let (2..=3, 7..=9) = z { dummy() }` -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:54:5 | LL | / match x { @@ -47,7 +47,7 @@ LL | | None => (), LL | | }; | |_____^ help: try this: `if let Some(y) = x { dummy() }` -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:59:5 | LL | / match y { @@ -56,7 +56,7 @@ LL | | Err(..) => (), LL | | }; | |_____^ help: try this: `if let Ok(y) = y { dummy() }` -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:66:5 | LL | / match c { @@ -65,8 +65,8 @@ LL | | Cow::Owned(..) => (), LL | | }; | |_____^ help: try this: `if let Cow::Borrowed(..) = c { dummy() }` -error: you seem to be trying to use match for an equality check. Consider using `if` - --> $DIR/single_match.rs:86:5 +error: you seem to be trying to use `match` for an equality check. Consider using `if` + --> $DIR/single_match.rs:87:5 | LL | / match x { LL | | "test" => println!(), @@ -74,8 +74,8 @@ LL | | _ => (), LL | | } | |_____^ help: try this: `if x == "test" { println!() }` -error: you seem to be trying to use match for an equality check. Consider using `if` - --> $DIR/single_match.rs:99:5 +error: you seem to be trying to use `match` for an equality check. Consider using `if` + --> $DIR/single_match.rs:100:5 | LL | / match x { LL | | Foo::A => println!(), @@ -83,8 +83,8 @@ LL | | _ => (), LL | | } | |_____^ help: try this: `if x == Foo::A { println!() }` -error: you seem to be trying to use match for an equality check. Consider using `if` - --> $DIR/single_match.rs:105:5 +error: you seem to be trying to use `match` for an equality check. Consider using `if` + --> $DIR/single_match.rs:106:5 | LL | / match x { LL | | FOO_C => println!(), @@ -92,8 +92,26 @@ LL | | _ => (), LL | | } | |_____^ help: try this: `if x == FOO_C { println!() }` -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match.rs:121:5 +error: you seem to be trying to use `match` for an equality check. Consider using `if` + --> $DIR/single_match.rs:111:5 + | +LL | / match &&x { +LL | | Foo::A => println!(), +LL | | _ => (), +LL | | } + | |_____^ help: try this: `if x == Foo::A { println!() }` + +error: you seem to be trying to use `match` for an equality check. Consider using `if` + --> $DIR/single_match.rs:117:5 + | +LL | / match &x { +LL | | Foo::A => println!(), +LL | | _ => (), +LL | | } + | |_____^ help: try this: `if x == &Foo::A { println!() }` + +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match.rs:134:5 | LL | / match x { LL | | Bar::A => println!(), @@ -101,5 +119,5 @@ LL | | _ => (), LL | | } | |_____^ help: try this: `if let Bar::A = x { println!() }` -error: aborting due to 10 previous errors +error: aborting due to 12 previous errors diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr index 3a07c2ec5..20be4fa22 100644 --- a/tests/ui/single_match_else.stderr +++ b/tests/ui/single_match_else.stderr @@ -1,4 +1,4 @@ -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match_else.rs:14:5 | LL | / match ExprNode::Butterflies { @@ -19,7 +19,7 @@ LL | None LL | } | -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match_else.rs:70:5 | LL | / match Some(1) { @@ -39,7 +39,7 @@ LL | return LL | } | -error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match_else.rs:79:5 | LL | / match Some(1) {