Skip to content

Commit 83af40c

Browse files
ada4aprofetia
andcommitted
allow when Partial::eq is const
turns out `is_in_const_context` wasn't even supposed to be put in the main let-chain, since it's only relevant for the `is_structural_partial_eq` case Co-authored-by: yanglsh <[email protected]>
1 parent f59f0e1 commit 83af40c

File tree

5 files changed

+43
-10
lines changed

5 files changed

+43
-10
lines changed

clippy_lints/src/equatable_if_let.rs

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
1+
use clippy_config::Conf;
12
use clippy_utils::diagnostics::span_lint_and_sugg;
23
use clippy_utils::is_in_const_context;
4+
use clippy_utils::msrvs::Msrv;
5+
use clippy_utils::qualify_min_const_fn::is_stable_const_fn;
36
use clippy_utils::source::snippet_with_context;
47
use clippy_utils::ty::implements_trait;
58
use rustc_errors::Applicability;
69
use rustc_hir::{Expr, ExprKind, Pat, PatKind};
710
use rustc_lint::{LateContext, LateLintPass, LintContext};
811
use rustc_middle::ty::Ty;
9-
use rustc_session::declare_lint_pass;
12+
use rustc_session::impl_lint_pass;
1013

1114
declare_clippy_lint! {
1215
/// ### What it does
@@ -38,7 +41,17 @@ declare_clippy_lint! {
3841
"using pattern matching instead of equality"
3942
}
4043

41-
declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);
44+
impl_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);
45+
46+
pub(super) struct PatternEquality {
47+
msrv: Msrv,
48+
}
49+
50+
impl PatternEquality {
51+
pub(super) fn new(conf: &Conf) -> Self {
52+
Self { msrv: conf.msrv }
53+
}
54+
}
4255

4356
/// detects if pattern matches just one thing
4457
fn unary_pattern(pat: &Pat<'_>) -> bool {
@@ -61,9 +74,22 @@ fn unary_pattern(pat: &Pat<'_>) -> bool {
6174
}
6275
}
6376

64-
fn is_structural_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> bool {
65-
if let Some(def_id) = cx.tcx.lang_items().eq_trait() {
66-
implements_trait(cx, ty, def_id, &[other.into()])
77+
fn is_structural_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>, msrv: Msrv) -> bool {
78+
if let Some(eq_trait) = cx.tcx.lang_items().eq_trait()
79+
&& implements_trait(cx, ty, eq_trait, &[other.into()])
80+
{
81+
if !is_in_const_context(cx) {
82+
return true;
83+
}
84+
85+
// TODO: add a MSRV test once `eq` becomes stably-const
86+
if let Some(eq_method) = cx.tcx.provided_trait_methods(eq_trait).next()
87+
&& is_stable_const_fn(cx, eq_method.def_id, msrv)
88+
{
89+
true
90+
} else {
91+
false
92+
}
6793
} else {
6894
false
6995
}
@@ -106,13 +132,12 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality {
106132
if let ExprKind::Let(let_expr) = expr.kind
107133
&& unary_pattern(let_expr.pat)
108134
&& !expr.span.in_external_macro(cx.sess().source_map())
109-
&& !is_in_const_context(cx)
110135
{
111136
let exp_ty = cx.typeck_results().expr_ty(let_expr.init);
112137
let pat_ty = cx.typeck_results().pat_ty(let_expr.pat);
113138
let mut applicability = Applicability::MachineApplicable;
114139

115-
if is_structural_partial_eq(cx, exp_ty, pat_ty) && !contains_type_mismatch(cx, let_expr.pat) {
140+
if is_structural_partial_eq(cx, exp_ty, pat_ty, self.msrv) && !contains_type_mismatch(cx, let_expr.pat) {
116141
let pat_str = match let_expr.pat.kind {
117142
PatKind::Struct(..) => format!(
118143
"({})",

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
647647
store.register_late_pass(move |_| Box::new(large_futures::LargeFuture::new(conf)));
648648
store.register_late_pass(|_| Box::new(if_let_mutex::IfLetMutex));
649649
store.register_late_pass(|_| Box::new(if_not_else::IfNotElse));
650-
store.register_late_pass(|_| Box::new(equatable_if_let::PatternEquality));
650+
store.register_late_pass(|_| Box::new(equatable_if_let::PatternEquality::new(conf)));
651651
store.register_late_pass(|_| Box::new(manual_async_fn::ManualAsyncFn));
652652
store.register_late_pass(|_| Box::new(panic_in_result_fn::PanicInResultFn));
653653
store.register_early_pass(move || Box::new(non_expressive_names::NonExpressiveNames::new(conf)));

tests/ui/equatable_if_let.fixed

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,5 +142,6 @@ mod issue8710 {
142142

143143
fn issue15376() {
144144
// PartialEq is not stable in consts yet
145-
const _: u32 = if let Some(true) = None { 0 } else { 1 };
145+
const _: u32 = if matches!(None, Some(true)) { 0 } else { 1 };
146+
//~^ ERROR: this pattern matching can be expressed using `matches!`
146147
}

tests/ui/equatable_if_let.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,5 @@ mod issue8710 {
143143
fn issue15376() {
144144
// PartialEq is not stable in consts yet
145145
const _: u32 = if let Some(true) = None { 0 } else { 1 };
146+
//~^ ERROR: this pattern matching can be expressed using `matches!`
146147
}

tests/ui/equatable_if_let.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,11 @@ error: this pattern matching can be expressed using `matches!`
103103
LL | if let Some(MyEnum::B) = get_enum() {
104104
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(get_enum(), Some(MyEnum::B))`
105105

106-
error: aborting due to 17 previous errors
106+
error: this pattern matching can be expressed using `matches!`
107+
--> tests/ui/equatable_if_let.rs:145:23
108+
|
109+
LL | const _: u32 = if let Some(true) = None { 0 } else { 1 };
110+
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(None, Some(true))`
111+
112+
error: aborting due to 18 previous errors
107113

0 commit comments

Comments
 (0)