Skip to content

Commit bfcc766

Browse files
HKalbasiHKalbasi
committed
upgrade equatable_if_let to style
1 parent 4996e17 commit bfcc766

38 files changed

+862
-179
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2696,6 +2696,7 @@ Released 2018-09-13
26962696
[`enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names
26972697
[`eq_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#eq_op
26982698
[`equatable_if_let`]: https://rust-lang.github.io/rust-clippy/master/index.html#equatable_if_let
2699+
[`equatable_matches`]: https://rust-lang.github.io/rust-clippy/master/index.html#equatable_matches
26992700
[`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op
27002701
[`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
27012702
[`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision

‎clippy_lints/src/casts/cast_lossless.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,15 @@ fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to
6565

6666
(true, false) => {
6767
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
68-
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
68+
let to_nbits = if cast_to.kind() == &ty::Float(FloatTy::F32) {
6969
32
7070
} else {
7171
64
7272
};
7373
from_nbits < to_nbits
7474
},
7575

76-
(_, _) => {
77-
matches!(cast_from.kind(), ty::Float(FloatTy::F32)) && matches!(cast_to.kind(), ty::Float(FloatTy::F64))
78-
},
76+
(_, _) => cast_from.kind() == &ty::Float(FloatTy::F32) && cast_to.kind() == &ty::Float(FloatTy::F64),
7977
}
8078
}
8179

‎clippy_lints/src/casts/cast_possible_truncation.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, ca
4040
},
4141

4242
(_, _) => {
43-
if matches!(cast_from.kind(), &ty::Float(FloatTy::F64))
44-
&& matches!(cast_to.kind(), &ty::Float(FloatTy::F32))
45-
{
43+
if cast_from.kind() == &ty::Float(FloatTy::F64) && cast_to.kind() == &ty::Float(FloatTy::F32) {
4644
"casting `f64` to `f32` may truncate the value".to_string()
4745
} else {
4846
return;

‎clippy_lints/src/equatable_if_let.rs

+203-52
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,23 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::source::snippet_with_context;
1+
use clippy_utils::source::{snippet_with_context, snippet_with_applicability};
32
use clippy_utils::ty::implements_trait;
3+
use clippy_utils::{diagnostics::span_lint_and_sugg, higher::MatchesExpn};
44
use if_chain::if_chain;
55
use rustc_errors::Applicability;
6-
use rustc_hir::{Expr, ExprKind, Pat, PatKind};
7-
use rustc_lint::{LateContext, LateLintPass};
8-
use rustc_middle::ty::Ty;
9-
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
use rustc_hir::{
7+
def::{DefKind, Res},
8+
Arm, Expr, ExprKind, Pat, PatKind, QPath,
9+
};
10+
use rustc_lint::{LateContext, LateLintPass, Lint};
11+
use rustc_middle::ty::{Adt, Ty};
12+
use rustc_session::{declare_tool_lint, impl_lint_pass};
13+
use rustc_span::{Span, SyntaxContext};
14+
15+
use crate::utils::conf::EquatablePatternLevel;
1016

1117
declare_clippy_lint! {
1218
/// ### What it does
13-
/// Checks for pattern matchings that can be expressed using equality.
19+
/// Checks for `if let <pat> = <expr>` (and `while let` and similars) that can be expressed
20+
/// using `if <expr> == <pat>`.
1421
///
1522
/// ### Why is this bad?
1623
///
@@ -33,68 +40,212 @@ declare_clippy_lint! {
3340
/// }
3441
/// ```
3542
pub EQUATABLE_IF_LET,
36-
nursery,
37-
"using pattern matching instead of equality"
43+
style,
44+
"using if let instead of if with a equality condition"
3845
}
3946

40-
declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);
47+
declare_clippy_lint! {
48+
/// ### What it does
49+
/// Checks for `matches!(<expr>, <pat>)` that can be expressed
50+
/// using `<expr> == <pat>`.
51+
///
52+
/// ### Why is this bad?
53+
///
54+
/// It is less concise and less clear.
55+
///
56+
/// ### Example
57+
/// ```rust,ignore
58+
/// let condition = matches!(x, Some(2));
59+
/// ```
60+
/// Should be written
61+
/// ```rust,ignore
62+
/// let condition = x == Some(2);
63+
/// ```
64+
pub EQUATABLE_MATCHES,
65+
pedantic,
66+
"using `matches!` instead of equality"
67+
}
68+
69+
pub struct PatternEquality {
70+
level: EquatablePatternLevel,
71+
}
4172

42-
/// detects if pattern matches just one thing
43-
fn unary_pattern(pat: &Pat<'_>) -> bool {
44-
fn array_rec(pats: &[Pat<'_>]) -> bool {
45-
pats.iter().all(unary_pattern)
73+
impl PatternEquality {
74+
pub fn new(level: EquatablePatternLevel) -> PatternEquality {
75+
PatternEquality { level }
4676
}
47-
match &pat.kind {
48-
PatKind::Slice(_, _, _) | PatKind::Range(_, _, _) | PatKind::Binding(..) | PatKind::Wild | PatKind::Or(_) => {
77+
}
78+
79+
impl_lint_pass!(PatternEquality => [EQUATABLE_IF_LET, EQUATABLE_MATCHES]);
80+
81+
fn equatable_pattern(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
82+
fn array_rec(cx: &LateContext<'_>, pats: &[Pat<'_>]) -> bool {
83+
pats.iter().all(|x| equatable_pattern(cx, x))
84+
}
85+
fn is_derived(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
86+
let ty = cx.typeck_results().pat_ty(pat);
87+
if let Some(def_id) = cx.tcx.lang_items().structural_peq_trait() {
88+
implements_trait(cx, ty, def_id, &[ty.into()])
89+
} else {
4990
false
91+
}
92+
}
93+
match &pat.kind {
94+
PatKind::Slice(a, None, []) => array_rec(cx, a),
95+
PatKind::Struct(_, a, etc) => !etc && is_derived(cx, pat) && a.iter().all(|x| equatable_pattern(cx, x.pat)),
96+
PatKind::Tuple(a, etc) => !etc.is_some() && array_rec(cx, a),
97+
PatKind::TupleStruct(_, a, etc) => !etc.is_some() && is_derived(cx, pat) && array_rec(cx, a),
98+
PatKind::Ref(x, _) | PatKind::Box(x) => equatable_pattern(cx, x),
99+
PatKind::Path(QPath::Resolved(_, b)) => match b.res {
100+
Res::Def(DefKind::Const, _) => true,
101+
_ => is_derived(cx, pat),
50102
},
51-
PatKind::Struct(_, a, etc) => !etc && a.iter().all(|x| unary_pattern(x.pat)),
52-
PatKind::Tuple(a, etc) | PatKind::TupleStruct(_, a, etc) => !etc.is_some() && array_rec(a),
53-
PatKind::Ref(x, _) | PatKind::Box(x) => unary_pattern(x),
54-
PatKind::Path(_) | PatKind::Lit(_) => true,
103+
PatKind::Path(_) => is_derived(cx, pat),
104+
PatKind::Lit(_) => true,
105+
PatKind::Slice(..) | PatKind::Range(..) | PatKind::Binding(..) | PatKind::Wild | PatKind::Or(_) => false,
55106
}
56107
}
57108

58-
fn is_structural_partial_eq(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> bool {
109+
fn is_partial_eq(cx: &LateContext<'tcx>, t1: Ty<'tcx>, t2: Ty<'tcx>) -> bool {
59110
if let Some(def_id) = cx.tcx.lang_items().eq_trait() {
60-
implements_trait(cx, ty, def_id, &[other.into()])
111+
implements_trait(cx, t1, def_id, &[t2.into()])
61112
} else {
62113
false
63114
}
64115
}
65116

117+
fn pat_to_string(cx: &LateContext<'tcx>, app: &mut Applicability, pat: &Pat<'_>, goal: Ty<'_>, ctxt: SyntaxContext) -> Option<String> {
118+
fn inner(cx: &LateContext<'tcx>, app: &mut Applicability, pat: &Pat<'_>, goal: Ty<'_>, r: &mut String, ctxt: SyntaxContext) -> bool {
119+
let ty = cx.typeck_results().pat_ty(pat);
120+
if ty == goal {
121+
match &pat.kind {
122+
PatKind::TupleStruct(q, ..) | PatKind::Struct(q, ..) => {
123+
let (adt_def, generic_args) = if let Adt(x, y) = ty.kind() {
124+
(x, y)
125+
} else {
126+
return false; // shouldn't happen
127+
};
128+
let path = if let QPath::Resolved(.., p) = q {
129+
p
130+
} else {
131+
return false; // give up
132+
};
133+
let var = adt_def.variant_of_res(path.res);
134+
match &pat.kind {
135+
PatKind::TupleStruct(_, params, _) => {
136+
*r += &*snippet_with_applicability(cx, path.span, "..", app);
137+
*r += "(";
138+
for (i, (p, f)) in params.iter().zip(var.fields.iter()).enumerate() {
139+
if i != 0 {
140+
*r += ", ";
141+
}
142+
inner(cx, app, p, f.ty(cx.tcx, generic_args), r, ctxt);
143+
}
144+
*r += ")";
145+
},
146+
PatKind::Struct(_, fields, _) => {
147+
*r += &*snippet_with_applicability(cx, path.span, "..", app);
148+
*r += " { ";
149+
for (i, p) in fields.iter().enumerate() {
150+
if i != 0 {
151+
*r += ", ";
152+
}
153+
*r += &*snippet_with_applicability(cx, p.ident.span, "..", app);
154+
*r += ": ";
155+
if let Some(x) = var.fields.iter().find(|f| f.ident == p.ident) {
156+
inner(cx, app, p.pat, x.ty(cx.tcx, generic_args), r, ctxt);
157+
} else {
158+
return false; // won't happen
159+
}
160+
}
161+
*r += " }";
162+
},
163+
_ => return false, // won't happen
164+
}
165+
},
166+
_ => {
167+
*r += &*snippet_with_context(cx, pat.span, ctxt, "..", app).0;
168+
},
169+
}
170+
return true;
171+
}
172+
if goal.is_ref() {
173+
if let Some(tam) = goal.builtin_deref(true) {
174+
*r += "&";
175+
return inner(cx, app, pat, tam.ty, r, ctxt);
176+
}
177+
}
178+
false
179+
}
180+
let mut r = "".to_string();
181+
if let PatKind::Struct(..) = pat.kind {
182+
r += "(";
183+
}
184+
let success = inner(cx, app, pat, goal, &mut r, ctxt);
185+
if let PatKind::Struct(..) = pat.kind {
186+
r += ")";
187+
}
188+
if !success {
189+
return None;
190+
}
191+
Some(r)
192+
}
193+
194+
fn level_contains(level: EquatablePatternLevel, pat: &Pat<'_>) -> bool {
195+
match level {
196+
EquatablePatternLevel::Primitive => matches!(pat.kind, PatKind::Lit(_)),
197+
EquatablePatternLevel::Simple => matches!(pat.kind, PatKind::Lit(_) | PatKind::Path(_)),
198+
EquatablePatternLevel::All => true,
199+
}
200+
}
201+
202+
fn emit_lint(
203+
cx: &LateContext<'tcx>,
204+
pat: &Pat<'_>,
205+
exp: &Expr<'_>,
206+
ctxt: SyntaxContext,
207+
span: Span,
208+
lint: &'static Lint,
209+
level: EquatablePatternLevel,
210+
) {
211+
if_chain! {
212+
if equatable_pattern(cx, pat);
213+
if level_contains(level, pat);
214+
let exp_ty = cx.typeck_results().expr_ty(exp);
215+
if is_partial_eq(cx, exp_ty, exp_ty);
216+
let mut app = Applicability::MachineApplicable;
217+
if let Some(pat_str) = pat_to_string(cx, &mut app, pat, exp_ty, ctxt);
218+
then {
219+
let exp_str = snippet_with_context(cx, exp.span, ctxt, "..", &mut app).0;
220+
span_lint_and_sugg(
221+
cx,
222+
lint,
223+
span,
224+
"this pattern matching can be expressed using equality",
225+
"try",
226+
format!(
227+
"{} == {}",
228+
exp_str,
229+
pat_str,
230+
),
231+
app,
232+
);
233+
}
234+
}
235+
}
236+
66237
impl<'tcx> LateLintPass<'tcx> for PatternEquality {
67238
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
68-
if_chain! {
69-
if let ExprKind::Let(pat, exp, _) = expr.kind;
70-
if unary_pattern(pat);
71-
let exp_ty = cx.typeck_results().expr_ty(exp);
72-
let pat_ty = cx.typeck_results().pat_ty(pat);
73-
if is_structural_partial_eq(cx, exp_ty, pat_ty);
74-
then {
75-
76-
let mut applicability = Applicability::MachineApplicable;
77-
let pat_str = match pat.kind {
78-
PatKind::Struct(..) => format!(
79-
"({})",
80-
snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0,
81-
),
82-
_ => snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(),
83-
};
84-
span_lint_and_sugg(
85-
cx,
86-
EQUATABLE_IF_LET,
87-
expr.span,
88-
"this pattern matching can be expressed using equality",
89-
"try",
90-
format!(
91-
"{} == {}",
92-
snippet_with_context(cx, exp.span, expr.span.ctxt(), "..", &mut applicability).0,
93-
pat_str,
94-
),
95-
applicability,
96-
);
97-
}
239+
if let ExprKind::Let(pat, exp, _) = expr.kind {
240+
emit_lint(cx, pat, exp, expr.span.ctxt(), expr.span, EQUATABLE_IF_LET, self.level);
241+
}
242+
if let Some(MatchesExpn {
243+
call_site,
244+
arm: Arm { pat, guard: None, .. },
245+
exp,
246+
}) = MatchesExpn::parse(expr)
247+
{
248+
emit_lint(cx, pat, exp, expr.span.ctxt(), call_site, EQUATABLE_MATCHES, self.level);
98249
}
99250
}
100251
}

‎clippy_lints/src/lib.register_all.rs

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
5151
LintId::of(enum_variants::MODULE_INCEPTION),
5252
LintId::of(eq_op::EQ_OP),
5353
LintId::of(eq_op::OP_REF),
54+
LintId::of(equatable_if_let::EQUATABLE_IF_LET),
5455
LintId::of(erasing_op::ERASING_OP),
5556
LintId::of(escape::BOXED_LOCAL),
5657
LintId::of(eta_reduction::REDUNDANT_CLOSURE),

‎clippy_lints/src/lib.register_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ store.register_lints(&[
118118
eq_op::EQ_OP,
119119
eq_op::OP_REF,
120120
equatable_if_let::EQUATABLE_IF_LET,
121+
equatable_if_let::EQUATABLE_MATCHES,
121122
erasing_op::ERASING_OP,
122123
escape::BOXED_LOCAL,
123124
eta_reduction::REDUNDANT_CLOSURE,

‎clippy_lints/src/lib.register_nursery.rs

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
88
LintId::of(copies::BRANCHES_SHARING_CODE),
99
LintId::of(disallowed_method::DISALLOWED_METHOD),
1010
LintId::of(disallowed_type::DISALLOWED_TYPE),
11-
LintId::of(equatable_if_let::EQUATABLE_IF_LET),
1211
LintId::of(fallible_impl_from::FALLIBLE_IMPL_FROM),
1312
LintId::of(floating_point_arithmetic::IMPRECISE_FLOPS),
1413
LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS),

‎clippy_lints/src/lib.register_pedantic.rs

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
2828
LintId::of(doc::MISSING_PANICS_DOC),
2929
LintId::of(empty_enum::EMPTY_ENUM),
3030
LintId::of(enum_variants::MODULE_NAME_REPETITIONS),
31+
LintId::of(equatable_if_let::EQUATABLE_MATCHES),
3132
LintId::of(eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS),
3233
LintId::of(excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
3334
LintId::of(excessive_bools::STRUCT_EXCESSIVE_BOOLS),

‎clippy_lints/src/lib.register_style.rs

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
2020
LintId::of(enum_variants::ENUM_VARIANT_NAMES),
2121
LintId::of(enum_variants::MODULE_INCEPTION),
2222
LintId::of(eq_op::OP_REF),
23+
LintId::of(equatable_if_let::EQUATABLE_IF_LET),
2324
LintId::of(eta_reduction::REDUNDANT_CLOSURE),
2425
LintId::of(float_literal::EXCESSIVE_PRECISION),
2526
LintId::of(from_over_into::FROM_OVER_INTO),

‎clippy_lints/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
720720
store.register_late_pass(|| Box::new(option_if_let_else::OptionIfLetElse));
721721
store.register_late_pass(|| Box::new(future_not_send::FutureNotSend));
722722
store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));
723-
store.register_late_pass(|| Box::new(equatable_if_let::PatternEquality));
723+
let equatable_pattern = conf.equatable_pattern;
724+
store.register_late_pass(move || Box::new(equatable_if_let::PatternEquality::new(equatable_pattern)));
724725
store.register_late_pass(|| Box::new(mut_mutex_lock::MutMutexLock));
725726
store.register_late_pass(|| Box::new(match_on_vec_items::MatchOnVecItems));
726727
store.register_late_pass(|| Box::new(manual_async_fn::ManualAsyncFn));

‎clippy_lints/src/manual_async_fn.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ fn captures_all_lifetimes(inputs: &[Ty<'_>], output_lifetimes: &[LifetimeName])
165165
// - There's only one output lifetime bound using `+ '_`
166166
// - All input lifetimes are explicitly bound to the output
167167
input_lifetimes.is_empty()
168-
|| (output_lifetimes.len() == 1 && matches!(output_lifetimes[0], LifetimeName::Underscore))
168+
|| (output_lifetimes.len() == 1 && output_lifetimes[0] == LifetimeName::Underscore)
169169
|| input_lifetimes
170170
.iter()
171171
.all(|in_lt| output_lifetimes.iter().any(|out_lt| in_lt == out_lt))

‎clippy_lints/src/manual_map.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ impl LateLintPass<'_> for ManualMap {
170170
}
171171

172172
// `ref` and `ref mut` annotations were handled earlier.
173-
let annotation = if matches!(annotation, BindingAnnotation::Mutable) {
173+
let annotation = if annotation == BindingAnnotation::Mutable {
174174
"mut "
175175
} else {
176176
""

0 commit comments

Comments
 (0)