Skip to content

Commit 388a3d0

Browse files
committed
Implement equatable if let lint
1 parent 3311b36 commit 388a3d0

File tree

72 files changed

+572
-253
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

72 files changed

+572
-253
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2695,6 +2695,7 @@ Released 2018-09-13
26952695
[`enum_glob_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_glob_use
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
2698+
[`equatable_if_let`]: https://rust-lang.github.io/rust-clippy/master/index.html#equatable_if_let
26982699
[`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op
26992700
[`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
27002701
[`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision

clippy_lints/src/attrs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute) {
563563
skip_item.path.segments.last().expect("empty path in attribute").ident.name == sym::skip;
564564
// Only lint outer attributes, because custom inner attributes are unstable
565565
// Tracking issue: https://github.com/rust-lang/rust/issues/54726
566-
if let AttrStyle::Outer = attr.style;
566+
if attr.style == AttrStyle::Outer;
567567
then {
568568
span_lint_and_sugg(
569569
cx,

clippy_lints/src/casts/cast_precision_loss.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, ca
1212
}
1313

1414
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
15-
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
15+
let to_nbits = if cast_to.kind() == &ty::Float(FloatTy::F32) {
1616
32
1717
} else {
1818
64

clippy_lints/src/derive.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {
393393

394394
if_chain! {
395395
if let Some(header) = kind.header();
396-
if let Unsafety::Unsafe = header.unsafety;
396+
if header.unsafety == Unsafety::Unsafe;
397397
then {
398398
self.has_unsafe = true;
399399
}
@@ -408,7 +408,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {
408408
}
409409

410410
if let ExprKind::Block(block, _) = expr.kind {
411-
if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules {
411+
if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) {
412412
self.has_unsafe = true;
413413
}
414414
}

clippy_lints/src/equatable_if_let.rs

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_applicability;
3+
use clippy_utils::ty::implements_trait;
4+
use if_chain::if_chain;
5+
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};
10+
11+
declare_clippy_lint! {
12+
/// ### What it does
13+
/// Checks for pattern matchings that can be expressed using equality.
14+
///
15+
/// ### Why is this bad?
16+
///
17+
/// * It reads better and has less cognitive load because equality won't cause binding.
18+
/// * It is a [Yoda condition](https://en.wikipedia.org/wiki/Yoda_conditions). Yoda conditions are widely
19+
/// criticized for increasing the cognitive load of reading the code.
20+
/// * Equality is a simple bool expression and can be merged with `&&` and `||` and
21+
/// reuse if blocks
22+
///
23+
/// ### Example
24+
/// ```rust,ignore
25+
/// if let Some(2) = x {
26+
/// do_thing();
27+
/// }
28+
/// ```
29+
/// Should be written
30+
/// ```rust,ignore
31+
/// if x == Some(2) {
32+
/// do_thing();
33+
/// }
34+
/// ```
35+
pub EQUATABLE_IF_LET,
36+
nursery,
37+
"using pattern matching instead of equality"
38+
}
39+
40+
declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);
41+
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)
46+
}
47+
match &pat.kind {
48+
PatKind::Slice(_, _, _) | PatKind::Range(_, _, _) | PatKind::Binding(..) | PatKind::Wild | PatKind::Or(_) => {
49+
false
50+
},
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,
55+
}
56+
}
57+
58+
fn is_structural_partial_eq(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> bool {
59+
if let Some(def_id) = cx.tcx.lang_items().eq_trait() {
60+
implements_trait(cx, ty, def_id, &[other.into()])
61+
} else {
62+
false
63+
}
64+
}
65+
66+
impl<'tcx> LateLintPass<'tcx> for PatternEquality {
67+
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_applicability(cx, pat.span, "..", &mut applicability),
81+
),
82+
_ => snippet_with_applicability(cx, pat.span, "..", &mut applicability).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_applicability(cx, exp.span, "..", &mut applicability),
93+
pat_str,
94+
),
95+
applicability,
96+
);
97+
}
98+
}
99+
}
100+
}

clippy_lints/src/erasing_op.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl<'tcx> LateLintPass<'tcx> for ErasingOp {
4747
}
4848

4949
fn check(cx: &LateContext<'_>, e: &Expr<'_>, span: Span) {
50-
if let Some(Constant::Int(0)) = constant_simple(cx, cx.typeck_results(), e) {
50+
if constant_simple(cx, cx.typeck_results(), e) == Some(Constant::Int(0)) {
5151
span_lint(
5252
cx,
5353
ERASING_OP,

clippy_lints/src/escape.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal {
9090
for trait_item in items {
9191
if trait_item.id.hir_id() == hir_id {
9292
// be sure we have `self` parameter in this function
93-
if let AssocItemKind::Fn { has_self: true } = trait_item.kind {
93+
if trait_item.kind == (AssocItemKind::Fn { has_self: true }) {
9494
trait_self_ty = Some(
9595
TraitRef::identity(cx.tcx, trait_item.id.def_id.to_def_id())
9696
.self_ty()

clippy_lints/src/eta_reduction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
116116
if let Some(mut snippet) = snippet_opt(cx, callee.span) {
117117
if_chain! {
118118
if let ty::Closure(_, substs) = callee_ty.peel_refs().kind();
119-
if let ClosureKind::FnMut = substs.as_closure().kind();
119+
if substs.as_closure().kind() == ClosureKind::FnMut;
120120
if get_enclosing_loop_or_closure(cx.tcx, expr).is_some()
121121
|| UsedAfterExprVisitor::is_found(cx, callee);
122122

clippy_lints/src/eval_order_dependence.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ impl<'a, 'tcx> Visitor<'tcx> for DivergenceVisitor<'a, 'tcx> {
141141
match typ.kind() {
142142
ty::FnDef(..) | ty::FnPtr(_) => {
143143
let sig = typ.fn_sig(self.cx.tcx);
144-
if let ty::Never = self.cx.tcx.erase_late_bound_regions(sig).output().kind() {
144+
if self.cx.tcx.erase_late_bound_regions(sig).output().kind() == &ty::Never {
145145
self.report_diverging_sub_expr(e);
146146
}
147147
},

clippy_lints/src/identity_op.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use clippy_utils::source::snippet;
2-
use if_chain::if_chain;
32
use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind};
43
use rustc_lint::{LateContext, LateLintPass};
54
use rustc_middle::ty;
@@ -62,16 +61,9 @@ impl<'tcx> LateLintPass<'tcx> for IdentityOp {
6261

6362
fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool {
6463
// `1 << 0` is a common pattern in bit manipulation code
65-
if_chain! {
66-
if let BinOpKind::Shl = cmp.node;
67-
if let Some(Constant::Int(0)) = constant_simple(cx, cx.typeck_results(), right);
68-
if let Some(Constant::Int(1)) = constant_simple(cx, cx.typeck_results(), left);
69-
then {
70-
return true;
71-
}
72-
}
73-
74-
false
64+
cmp.node == BinOpKind::Shl
65+
&& constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0))
66+
&& constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1))
7567
}
7668

7769
#[allow(clippy::cast_possible_wrap)]

0 commit comments

Comments
 (0)