From a6444a69e27275c69e7287fe02ba0c88d554c445 Mon Sep 17 00:00:00 2001 From: Nahua Kang <kangnahua@gmail.com> Date: Tue, 23 Aug 2022 19:05:57 +0200 Subject: [PATCH 1/3] Remove if_chain from equatable_if_let --- clippy_lints/src/equatable_if_let.rs | 59 +++++++++++++--------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/clippy_lints/src/equatable_if_let.rs b/clippy_lints/src/equatable_if_let.rs index fdfb821ac78..ba615c8c164 100644 --- a/clippy_lints/src/equatable_if_let.rs +++ b/clippy_lints/src/equatable_if_let.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::implements_trait; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -67,37 +66,33 @@ fn is_structural_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: T impl<'tcx> LateLintPass<'tcx> for PatternEquality { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if_chain! { - if !in_external_macro(cx.sess(), expr.span); - if let ExprKind::Let(let_expr) = expr.kind; - if unary_pattern(let_expr.pat); - let exp_ty = cx.typeck_results().expr_ty(let_expr.init); - let pat_ty = cx.typeck_results().pat_ty(let_expr.pat); - if is_structural_partial_eq(cx, exp_ty, pat_ty); - then { - - let mut applicability = Applicability::MachineApplicable; - let pat_str = match let_expr.pat.kind { - PatKind::Struct(..) => format!( - "({})", - snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0, - ), - _ => snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(), - }; - span_lint_and_sugg( - cx, - EQUATABLE_IF_LET, - expr.span, - "this pattern matching can be expressed using equality", - "try", - format!( - "{} == {}", - snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0, - pat_str, - ), - applicability, - ); - } + if !in_external_macro(cx.sess(), expr.span) + && let ExprKind::Let(let_expr) = expr.kind + && unary_pattern(let_expr.pat) + && let exp_ty = cx.typeck_results().expr_ty(let_expr.init) + && let pat_ty = cx.typeck_results().pat_ty(let_expr.pat) + && is_structural_partial_eq(cx, exp_ty, pat_ty) { + let mut applicability = Applicability::MachineApplicable; + let pat_str = match let_expr.pat.kind { + PatKind::Struct(..) => format!( + "({})", + snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0, + ), + _ => snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(), + }; + span_lint_and_sugg( + cx, + EQUATABLE_IF_LET, + expr.span, + "this pattern matching can be expressed using equality", + "try", + format!( + "{} == {}", + snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0, + pat_str, + ), + applicability, + ); } } } From 5ee1c24f28029162ad25614c092224cc94ba59ff Mon Sep 17 00:00:00 2001 From: Nahua Kang <kangnahua@gmail.com> Date: Tue, 23 Aug 2022 19:46:04 +0200 Subject: [PATCH 2/3] Lint suggests matches macro if PartialEq trait is not implemented --- clippy_lints/src/equatable_if_let.rs | 64 +++++++++++++++++----------- tests/ui/equatable_if_let.rs | 7 +++ 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/equatable_if_let.rs b/clippy_lints/src/equatable_if_let.rs index ba615c8c164..6f26195d105 100644 --- a/clippy_lints/src/equatable_if_let.rs +++ b/clippy_lints/src/equatable_if_let.rs @@ -68,31 +68,47 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if !in_external_macro(cx.sess(), expr.span) && let ExprKind::Let(let_expr) = expr.kind - && unary_pattern(let_expr.pat) - && let exp_ty = cx.typeck_results().expr_ty(let_expr.init) - && let pat_ty = cx.typeck_results().pat_ty(let_expr.pat) - && is_structural_partial_eq(cx, exp_ty, pat_ty) { + && unary_pattern(let_expr.pat) { + let exp_ty = cx.typeck_results().expr_ty(let_expr.init); + let pat_ty = cx.typeck_results().pat_ty(let_expr.pat); let mut applicability = Applicability::MachineApplicable; - let pat_str = match let_expr.pat.kind { - PatKind::Struct(..) => format!( - "({})", - snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0, - ), - _ => snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(), - }; - span_lint_and_sugg( - cx, - EQUATABLE_IF_LET, - expr.span, - "this pattern matching can be expressed using equality", - "try", - format!( - "{} == {}", - snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0, - pat_str, - ), - applicability, - ); + + if is_structural_partial_eq(cx, exp_ty, pat_ty) { + let pat_str = match let_expr.pat.kind { + PatKind::Struct(..) => format!( + "({})", + snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0, + ), + _ => snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(), + }; + span_lint_and_sugg( + cx, + EQUATABLE_IF_LET, + expr.span, + "this pattern matching can be expressed using equality", + "try", + format!( + "{} == {}", + snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0, + pat_str, + ), + applicability, + ); + } else { + span_lint_and_sugg( + cx, + EQUATABLE_IF_LET, + expr.span, + "this pattern matching can be expressed using `matches!`", + "try", + format!( + "matches!({}, {})", + snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0, + snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0, + ), + applicability, + ) + } } } } diff --git a/tests/ui/equatable_if_let.rs b/tests/ui/equatable_if_let.rs index 8c467d14d2a..c3626c081dd 100644 --- a/tests/ui/equatable_if_let.rs +++ b/tests/ui/equatable_if_let.rs @@ -23,6 +23,11 @@ struct Struct { b: bool, } +struct NoPartialEqStruct { + a: i32, + b: bool, +} + enum NotPartialEq { A, B, @@ -47,6 +52,7 @@ fn main() { let e = Enum::UnitVariant; let f = NotPartialEq::A; let g = NotStructuralEq::A; + let h = NoPartialEqStruct { a: 2, b: false }; // true @@ -70,6 +76,7 @@ fn main() { if let NotStructuralEq::A = g {} if let Some(NotPartialEq::A) = Some(f) {} if let Some(NotStructuralEq::A) = Some(g) {} + if let NoPartialEqStruct { a: 2, b: false } = h {} macro_rules! m1 { (x) => { From 4eaadd622d21948c488a4952b3c65fd5b9d852dc Mon Sep 17 00:00:00 2001 From: Nahua Kang <kangnahua@gmail.com> Date: Tue, 23 Aug 2022 19:50:34 +0200 Subject: [PATCH 3/3] Run cargo dev bless to update fixes & stderr --- clippy_lints/src/equatable_if_let.rs | 10 +++---- tests/ui/equatable_if_let.fixed | 11 ++++++-- tests/ui/equatable_if_let.stderr | 42 ++++++++++++++++++++-------- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/equatable_if_let.rs b/clippy_lints/src/equatable_if_let.rs index 6f26195d105..928128785f4 100644 --- a/clippy_lints/src/equatable_if_let.rs +++ b/clippy_lints/src/equatable_if_let.rs @@ -96,18 +96,18 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality { ); } else { span_lint_and_sugg( - cx, + cx, EQUATABLE_IF_LET, expr.span, - "this pattern matching can be expressed using `matches!`", - "try", + "this pattern matching can be expressed using `matches!`", + "try", format!( "matches!({}, {})", snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0, snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0, - ), + ), applicability, - ) + ); } } } diff --git a/tests/ui/equatable_if_let.fixed b/tests/ui/equatable_if_let.fixed index 687efdada6e..9af2ba96272 100644 --- a/tests/ui/equatable_if_let.fixed +++ b/tests/ui/equatable_if_let.fixed @@ -23,6 +23,11 @@ struct Struct { b: bool, } +struct NoPartialEqStruct { + a: i32, + b: bool, +} + enum NotPartialEq { A, B, @@ -47,6 +52,7 @@ fn main() { let e = Enum::UnitVariant; let f = NotPartialEq::A; let g = NotStructuralEq::A; + let h = NoPartialEqStruct { a: 2, b: false }; // true @@ -66,10 +72,11 @@ fn main() { if let Some(3 | 4) = c {} if let Struct { a, b: false } = d {} if let Struct { a: 2, b: x } = d {} - if let NotPartialEq::A = f {} + if matches!(f, NotPartialEq::A) {} if g == NotStructuralEq::A {} - if let Some(NotPartialEq::A) = Some(f) {} + if matches!(Some(f), Some(NotPartialEq::A)) {} if Some(g) == Some(NotStructuralEq::A) {} + if matches!(h, NoPartialEqStruct { a: 2, b: false }) {} macro_rules! m1 { (x) => { diff --git a/tests/ui/equatable_if_let.stderr b/tests/ui/equatable_if_let.stderr index 9c4c3cc3682..40ca75b8da2 100644 --- a/tests/ui/equatable_if_let.stderr +++ b/tests/ui/equatable_if_let.stderr @@ -1,5 +1,5 @@ error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:53:8 + --> $DIR/equatable_if_let.rs:59:8 | LL | if let 2 = a {} | ^^^^^^^^^ help: try: `a == 2` @@ -7,64 +7,82 @@ LL | if let 2 = a {} = note: `-D clippy::equatable-if-let` implied by `-D warnings` error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:54:8 + --> $DIR/equatable_if_let.rs:60:8 | LL | if let Ordering::Greater = a.cmp(&b) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `a.cmp(&b) == Ordering::Greater` error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:55:8 + --> $DIR/equatable_if_let.rs:61:8 | LL | if let Some(2) = c {} | ^^^^^^^^^^^^^^^ help: try: `c == Some(2)` error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:56:8 + --> $DIR/equatable_if_let.rs:62:8 | LL | if let Struct { a: 2, b: false } = d {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `d == (Struct { a: 2, b: false })` error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:57:8 + --> $DIR/equatable_if_let.rs:63:8 | LL | if let Enum::TupleVariant(32, 64) = e {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `e == Enum::TupleVariant(32, 64)` error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:58:8 + --> $DIR/equatable_if_let.rs:64:8 | LL | if let Enum::RecordVariant { a: 64, b: 32 } = e {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `e == (Enum::RecordVariant { a: 64, b: 32 })` error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:59:8 + --> $DIR/equatable_if_let.rs:65:8 | LL | if let Enum::UnitVariant = e {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `e == Enum::UnitVariant` error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:60:8 + --> $DIR/equatable_if_let.rs:66:8 | LL | if let (Enum::UnitVariant, &Struct { a: 2, b: false }) = (e, &d) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(e, &d) == (Enum::UnitVariant, &Struct { a: 2, b: false })` +error: this pattern matching can be expressed using `matches!` + --> $DIR/equatable_if_let.rs:75:8 + | +LL | if let NotPartialEq::A = f {} + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(f, NotPartialEq::A)` + error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:70:8 + --> $DIR/equatable_if_let.rs:76:8 | LL | if let NotStructuralEq::A = g {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `g == NotStructuralEq::A` +error: this pattern matching can be expressed using `matches!` + --> $DIR/equatable_if_let.rs:77:8 + | +LL | if let Some(NotPartialEq::A) = Some(f) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(Some(f), Some(NotPartialEq::A))` + error: this pattern matching can be expressed using equality - --> $DIR/equatable_if_let.rs:72:8 + --> $DIR/equatable_if_let.rs:78:8 | LL | if let Some(NotStructuralEq::A) = Some(g) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(g) == Some(NotStructuralEq::A)` -error: this pattern matching can be expressed using equality +error: this pattern matching can be expressed using `matches!` --> $DIR/equatable_if_let.rs:79:8 | +LL | if let NoPartialEqStruct { a: 2, b: false } = h {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(h, NoPartialEqStruct { a: 2, b: false })` + +error: this pattern matching can be expressed using equality + --> $DIR/equatable_if_let.rs:86:8 + | LL | if let m1!(x) = "abc" { | ^^^^^^^^^^^^^^^^^^ help: try: `"abc" == m1!(x)` -error: aborting due to 11 previous errors +error: aborting due to 14 previous errors