diff --git a/clippy_lints/src/equatable_if_let.rs b/clippy_lints/src/equatable_if_let.rs index 72f5eaf8a4bc..58311257f58b 100644 --- a/clippy_lints/src/equatable_if_let.rs +++ b/clippy_lints/src/equatable_if_let.rs @@ -1,11 +1,16 @@ +use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_in_const_context; +use clippy_utils::msrvs::Msrv; +use clippy_utils::qualify_min_const_fn::is_stable_const_fn; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::implements_trait; use rustc_errors::Applicability; +use rustc_hir::def_id::DefId; use rustc_hir::{Expr, ExprKind, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::ty::Ty; -use rustc_session::declare_lint_pass; +use rustc_middle::ty::{Instance, Ty, TyCtxt}; +use rustc_session::impl_lint_pass; declare_clippy_lint! { /// ### What it does @@ -37,7 +42,47 @@ declare_clippy_lint! { "using pattern matching instead of equality" } -declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]); +impl_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]); + +pub(super) struct PatternEquality { + eq_trait: Option, + eq_method: Option, + msrv: Msrv, +} + +impl PatternEquality { + pub(super) fn new(tcx: TyCtxt<'_>, conf: &Conf) -> Self { + let eq_trait = tcx.lang_items().eq_trait(); + let eq_method = eq_trait.and_then(|eq_trait| tcx.associated_item_def_ids(eq_trait).first().copied()); + + Self { + eq_trait, + eq_method, + msrv: conf.msrv, + } + } + + fn is_structural_partial_eq<'tcx>(&self, cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> bool { + let is_partial_eq = || { + self.eq_trait + .is_some_and(|eq_trait| implements_trait(cx, ty, eq_trait, &[other.into()])) + }; + + let eq_method_is_const = || { + if let Some(eq_method) = self.eq_method + && let args = cx.tcx.mk_args(&[ty.into(), other.into()]) + && let Ok(Some(instance)) = Instance::try_resolve(cx.tcx, cx.typing_env(), eq_method, args) + && is_stable_const_fn(cx, instance.def_id(), self.msrv) + { + true + } else { + false + } + }; + + is_partial_eq() && (!is_in_const_context(cx) || eq_method_is_const()) + } +} /// detects if pattern matches just one thing fn unary_pattern(pat: &Pat<'_>) -> bool { @@ -60,14 +105,6 @@ fn unary_pattern(pat: &Pat<'_>) -> bool { } } -fn is_structural_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> bool { - if let Some(def_id) = cx.tcx.lang_items().eq_trait() { - implements_trait(cx, ty, def_id, &[other.into()]) - } else { - false - } -} - /// Check if the pattern has any type mismatch that would prevent it from being used in an equality /// check. This can happen if the expr has a reference type and the corresponding pattern is a /// literal. @@ -110,7 +147,7 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality { let pat_ty = cx.typeck_results().pat_ty(let_expr.pat); let mut applicability = Applicability::MachineApplicable; - if is_structural_partial_eq(cx, exp_ty, pat_ty) && !contains_type_mismatch(cx, let_expr.pat) { + if self.is_structural_partial_eq(cx, exp_ty, pat_ty) && !contains_type_mismatch(cx, let_expr.pat) { let pat_str = match let_expr.pat.kind { PatKind::Struct(..) => format!( "({})", diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d468993e7444..f327389cd698 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -648,7 +648,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(move |_| Box::new(large_futures::LargeFuture::new(conf))); store.register_late_pass(|_| Box::new(if_let_mutex::IfLetMutex)); store.register_late_pass(|_| Box::new(if_not_else::IfNotElse)); - store.register_late_pass(|_| Box::new(equatable_if_let::PatternEquality)); + store.register_late_pass(|tcx| Box::new(equatable_if_let::PatternEquality::new(tcx, conf))); store.register_late_pass(|_| Box::new(manual_async_fn::ManualAsyncFn)); store.register_late_pass(|_| Box::new(panic_in_result_fn::PanicInResultFn)); store.register_early_pass(move || Box::new(non_expressive_names::NonExpressiveNames::new(conf))); diff --git a/tests/ui/equatable_if_let.fixed b/tests/ui/equatable_if_let.fixed index ce8b67f9ca7b..d98e78a5c765 100644 --- a/tests/ui/equatable_if_let.fixed +++ b/tests/ui/equatable_if_let.fixed @@ -7,6 +7,7 @@ clippy::needless_if )] #![warn(clippy::equatable_if_let)] +#![feature(const_trait_impl, const_cmp)] extern crate proc_macros; use proc_macros::{external, inline_macros}; @@ -139,3 +140,32 @@ mod issue8710 { } } } + +#[allow(irrefutable_let_patterns, clippy::partialeq_to_none, clippy::eq_op)] +fn issue15376() { + struct NonConstEq; + + impl PartialEq for NonConstEq { + fn eq(&self, _other: &Self) -> bool { + true + } + } + + const _: u32 = if matches!(NonConstEq, NonConstEq) { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using `matches!` + const _: u32 = if matches!(None, Some(NonConstEq)) { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using `matches!` + + struct ConstEq; + + impl const PartialEq for ConstEq { + fn eq(&self, _other: &Self) -> bool { + true + } + } + + const _: u32 = if ConstEq == ConstEq { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using equality + const _: u32 = if None == Some(ConstEq) { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using equality +} diff --git a/tests/ui/equatable_if_let.rs b/tests/ui/equatable_if_let.rs index ff09533f2651..35a772ea4bd5 100644 --- a/tests/ui/equatable_if_let.rs +++ b/tests/ui/equatable_if_let.rs @@ -7,6 +7,7 @@ clippy::needless_if )] #![warn(clippy::equatable_if_let)] +#![feature(const_trait_impl, const_cmp)] extern crate proc_macros; use proc_macros::{external, inline_macros}; @@ -139,3 +140,32 @@ mod issue8710 { } } } + +#[allow(irrefutable_let_patterns, clippy::partialeq_to_none, clippy::eq_op)] +fn issue15376() { + struct NonConstEq; + + impl PartialEq for NonConstEq { + fn eq(&self, _other: &Self) -> bool { + true + } + } + + const _: u32 = if let NonConstEq = NonConstEq { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using `matches!` + const _: u32 = if let Some(NonConstEq) = None { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using `matches!` + + struct ConstEq; + + impl const PartialEq for ConstEq { + fn eq(&self, _other: &Self) -> bool { + true + } + } + + const _: u32 = if let ConstEq = ConstEq { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using equality + const _: u32 = if let Some(ConstEq) = None { 0 } else { 1 }; + //~^ ERROR: this pattern matching can be expressed using equality +} diff --git a/tests/ui/equatable_if_let.stderr b/tests/ui/equatable_if_let.stderr index dd1832ad68b2..a6ca96e55ca4 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 - --> tests/ui/equatable_if_let.rs:64:8 + --> tests/ui/equatable_if_let.rs:65:8 | LL | if let 2 = a {} | ^^^^^^^^^ help: try: `a == 2` @@ -8,100 +8,124 @@ LL | if let 2 = a {} = help: to override `-D warnings` add `#[allow(clippy::equatable_if_let)]` error: this pattern matching can be expressed using equality - --> tests/ui/equatable_if_let.rs:66:8 + --> tests/ui/equatable_if_let.rs:67: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 - --> tests/ui/equatable_if_let.rs:68:8 + --> tests/ui/equatable_if_let.rs:69:8 | LL | if let Some(2) = c {} | ^^^^^^^^^^^^^^^ help: try: `c == Some(2)` error: this pattern matching can be expressed using equality - --> tests/ui/equatable_if_let.rs:70:8 + --> tests/ui/equatable_if_let.rs:71: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 - --> tests/ui/equatable_if_let.rs:72:8 + --> tests/ui/equatable_if_let.rs:73: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 - --> tests/ui/equatable_if_let.rs:74:8 + --> tests/ui/equatable_if_let.rs:75: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 - --> tests/ui/equatable_if_let.rs:76:8 + --> tests/ui/equatable_if_let.rs:77:8 | LL | if let Enum::UnitVariant = e {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `e == Enum::UnitVariant` error: this pattern matching can be expressed using equality - --> tests/ui/equatable_if_let.rs:78:8 + --> tests/ui/equatable_if_let.rs:79: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!` - --> tests/ui/equatable_if_let.rs:88:8 + --> tests/ui/equatable_if_let.rs:89:8 | LL | if let NotPartialEq::A = f {} | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(f, NotPartialEq::A)` error: this pattern matching can be expressed using equality - --> tests/ui/equatable_if_let.rs:90:8 + --> tests/ui/equatable_if_let.rs:91:8 | LL | if let NotStructuralEq::A = g {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `g == NotStructuralEq::A` error: this pattern matching can be expressed using `matches!` - --> tests/ui/equatable_if_let.rs:92:8 + --> tests/ui/equatable_if_let.rs:93: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 - --> tests/ui/equatable_if_let.rs:94:8 + --> tests/ui/equatable_if_let.rs:95:8 | LL | if let Some(NotStructuralEq::A) = Some(g) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(g) == Some(NotStructuralEq::A)` error: this pattern matching can be expressed using `matches!` - --> tests/ui/equatable_if_let.rs:96:8 + --> tests/ui/equatable_if_let.rs:97: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 - --> tests/ui/equatable_if_let.rs:99:8 + --> tests/ui/equatable_if_let.rs:100:8 | LL | if let inline!("abc") = "abc" { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"abc" == inline!("abc")` error: this pattern matching can be expressed using `matches!` - --> tests/ui/equatable_if_let.rs:109:12 + --> tests/ui/equatable_if_let.rs:110:12 | LL | if let Some('i') = cs.iter().next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(cs.iter().next(), Some('i'))` error: this pattern matching can be expressed using `matches!` - --> tests/ui/equatable_if_let.rs:117:12 + --> tests/ui/equatable_if_let.rs:118:12 | LL | if let Some(1) = cs.iter().next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(cs.iter().next(), Some(1))` error: this pattern matching can be expressed using `matches!` - --> tests/ui/equatable_if_let.rs:135:12 + --> tests/ui/equatable_if_let.rs:136:12 | LL | if let Some(MyEnum::B) = get_enum() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(get_enum(), Some(MyEnum::B))` -error: aborting due to 17 previous errors +error: this pattern matching can be expressed using `matches!` + --> tests/ui/equatable_if_let.rs:154:23 + | +LL | const _: u32 = if let NonConstEq = NonConstEq { 0 } else { 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(NonConstEq, NonConstEq)` + +error: this pattern matching can be expressed using equality + --> tests/ui/equatable_if_let.rs:156:23 + | +LL | const _: u32 = if let Some(NonConstEq) = None { 0 } else { 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `None == Some(NonConstEq)` + +error: this pattern matching can be expressed using equality + --> tests/ui/equatable_if_let.rs:167:23 + | +LL | const _: u32 = if let ConstEq = ConstEq { 0 } else { 1 }; + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `ConstEq == ConstEq` + +error: this pattern matching can be expressed using equality + --> tests/ui/equatable_if_let.rs:169:23 + | +LL | const _: u32 = if let Some(ConstEq) = None { 0 } else { 1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `None == Some(ConstEq)` + +error: aborting due to 21 previous errors