Skip to content

Commit

Permalink
Implement equatable if let lint
Browse files Browse the repository at this point in the history
  • Loading branch information
HKalbasi committed Oct 4, 2021
1 parent 3311b36 commit 388a3d0
Show file tree
Hide file tree
Showing 72 changed files with 572 additions and 253 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -2695,6 +2695,7 @@ Released 2018-09-13
[`enum_glob_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_glob_use
[`enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names
[`eq_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#eq_op
[`equatable_if_let`]: https://rust-lang.github.io/rust-clippy/master/index.html#equatable_if_let
[`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op
[`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
[`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/attrs.rs
Expand Up @@ -563,7 +563,7 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute) {
skip_item.path.segments.last().expect("empty path in attribute").ident.name == sym::skip;
// Only lint outer attributes, because custom inner attributes are unstable
// Tracking issue: https://github.com/rust-lang/rust/issues/54726
if let AttrStyle::Outer = attr.style;
if attr.style == AttrStyle::Outer;
then {
span_lint_and_sugg(
cx,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_precision_loss.rs
Expand Up @@ -12,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, ca
}

let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
let to_nbits = if cast_to.kind() == &ty::Float(FloatTy::F32) {
32
} else {
64
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/derive.rs
Expand Up @@ -393,7 +393,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {

if_chain! {
if let Some(header) = kind.header();
if let Unsafety::Unsafe = header.unsafety;
if header.unsafety == Unsafety::Unsafe;
then {
self.has_unsafe = true;
}
Expand All @@ -408,7 +408,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {
}

if let ExprKind::Block(block, _) = expr.kind {
if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules {
if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) {
self.has_unsafe = true;
}
}
Expand Down
100 changes: 100 additions & 0 deletions clippy_lints/src/equatable_if_let.rs
@@ -0,0 +1,100 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
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};
use rustc_middle::ty::Ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// ### What it does
/// Checks for pattern matchings that can be expressed using equality.
///
/// ### Why is this bad?
///
/// * It reads better and has less cognitive load because equality won't cause binding.
/// * It is a [Yoda condition](https://en.wikipedia.org/wiki/Yoda_conditions). Yoda conditions are widely
/// criticized for increasing the cognitive load of reading the code.
/// * Equality is a simple bool expression and can be merged with `&&` and `||` and
/// reuse if blocks
///
/// ### Example
/// ```rust,ignore
/// if let Some(2) = x {
/// do_thing();
/// }
/// ```
/// Should be written
/// ```rust,ignore
/// if x == Some(2) {
/// do_thing();
/// }
/// ```
pub EQUATABLE_IF_LET,
nursery,
"using pattern matching instead of equality"
}

declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);

/// detects if pattern matches just one thing
fn unary_pattern(pat: &Pat<'_>) -> bool {
fn array_rec(pats: &[Pat<'_>]) -> bool {
pats.iter().all(unary_pattern)
}
match &pat.kind {
PatKind::Slice(_, _, _) | PatKind::Range(_, _, _) | PatKind::Binding(..) | PatKind::Wild | PatKind::Or(_) => {
false
},
PatKind::Struct(_, a, etc) => !etc && a.iter().all(|x| unary_pattern(x.pat)),
PatKind::Tuple(a, etc) | PatKind::TupleStruct(_, a, etc) => !etc.is_some() && array_rec(a),
PatKind::Ref(x, _) | PatKind::Box(x) => unary_pattern(x),
PatKind::Path(_) | PatKind::Lit(_) => true,
}
}

fn is_structural_partial_eq(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
}
}

impl<'tcx> LateLintPass<'tcx> for PatternEquality {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if_chain! {
if let ExprKind::Let(pat, exp, _) = expr.kind;
if unary_pattern(pat);
let exp_ty = cx.typeck_results().expr_ty(exp);
let pat_ty = cx.typeck_results().pat_ty(pat);
if is_structural_partial_eq(cx, exp_ty, pat_ty);
then {

let mut applicability = Applicability::MachineApplicable;
let pat_str = match pat.kind {
PatKind::Struct(..) => format!(
"({})",
snippet_with_applicability(cx, pat.span, "..", &mut applicability),
),
_ => snippet_with_applicability(cx, pat.span, "..", &mut applicability).to_string(),
};
span_lint_and_sugg(
cx,
EQUATABLE_IF_LET,
expr.span,
"this pattern matching can be expressed using equality",
"try",
format!(
"{} == {}",
snippet_with_applicability(cx, exp.span, "..", &mut applicability),
pat_str,
),
applicability,
);
}
}
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/erasing_op.rs
Expand Up @@ -47,7 +47,7 @@ impl<'tcx> LateLintPass<'tcx> for ErasingOp {
}

fn check(cx: &LateContext<'_>, e: &Expr<'_>, span: Span) {
if let Some(Constant::Int(0)) = constant_simple(cx, cx.typeck_results(), e) {
if constant_simple(cx, cx.typeck_results(), e) == Some(Constant::Int(0)) {
span_lint(
cx,
ERASING_OP,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/escape.rs
Expand Up @@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal {
for trait_item in items {
if trait_item.id.hir_id() == hir_id {
// be sure we have `self` parameter in this function
if let AssocItemKind::Fn { has_self: true } = trait_item.kind {
if trait_item.kind == (AssocItemKind::Fn { has_self: true }) {
trait_self_ty = Some(
TraitRef::identity(cx.tcx, trait_item.id.def_id.to_def_id())
.self_ty()
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/eta_reduction.rs
Expand Up @@ -116,7 +116,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
if let Some(mut snippet) = snippet_opt(cx, callee.span) {
if_chain! {
if let ty::Closure(_, substs) = callee_ty.peel_refs().kind();
if let ClosureKind::FnMut = substs.as_closure().kind();
if substs.as_closure().kind() == ClosureKind::FnMut;
if get_enclosing_loop_or_closure(cx.tcx, expr).is_some()
|| UsedAfterExprVisitor::is_found(cx, callee);

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/eval_order_dependence.rs
Expand Up @@ -141,7 +141,7 @@ impl<'a, 'tcx> Visitor<'tcx> for DivergenceVisitor<'a, 'tcx> {
match typ.kind() {
ty::FnDef(..) | ty::FnPtr(_) => {
let sig = typ.fn_sig(self.cx.tcx);
if let ty::Never = self.cx.tcx.erase_late_bound_regions(sig).output().kind() {
if self.cx.tcx.erase_late_bound_regions(sig).output().kind() == &ty::Never {
self.report_diverging_sub_expr(e);
}
},
Expand Down
14 changes: 3 additions & 11 deletions clippy_lints/src/identity_op.rs
@@ -1,5 +1,4 @@
use clippy_utils::source::snippet;
use if_chain::if_chain;
use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
Expand Down Expand Up @@ -62,16 +61,9 @@ impl<'tcx> LateLintPass<'tcx> for IdentityOp {

fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool {
// `1 << 0` is a common pattern in bit manipulation code
if_chain! {
if let BinOpKind::Shl = cmp.node;
if let Some(Constant::Int(0)) = constant_simple(cx, cx.typeck_results(), right);
if let Some(Constant::Int(1)) = constant_simple(cx, cx.typeck_results(), left);
then {
return true;
}
}

false
cmp.node == BinOpKind::Shl
&& constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0))
&& constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1))
}

#[allow(clippy::cast_possible_wrap)]
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/integer_division.rs
Expand Up @@ -48,7 +48,7 @@ impl<'tcx> LateLintPass<'tcx> for IntegerDivision {
fn is_integer_division<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) -> bool {
if_chain! {
if let hir::ExprKind::Binary(binop, left, right) = &expr.kind;
if let hir::BinOpKind::Div = &binop.node;
if binop.node == hir::BinOpKind::Div;
then {
let (left_ty, right_ty) = (cx.typeck_results().expr_ty(left), cx.typeck_results().expr_ty(right));
return left_ty.is_integral() && right_ty.is_integral();
Expand Down
12 changes: 4 additions & 8 deletions clippy_lints/src/len_zero.rs
Expand Up @@ -455,14 +455,10 @@ fn is_empty_array(expr: &Expr<'_>) -> bool {
fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
/// Gets an `AssocItem` and return true if it matches `is_empty(self)`.
fn is_is_empty(cx: &LateContext<'_>, item: &ty::AssocItem) -> bool {
if let ty::AssocKind::Fn = item.kind {
if item.ident.name.as_str() == "is_empty" {
let sig = cx.tcx.fn_sig(item.def_id);
let ty = sig.skip_binder();
ty.inputs().len() == 1
} else {
false
}
if item.kind == ty::AssocKind::Fn && item.ident.name.as_str() == "is_empty" {
let sig = cx.tcx.fn_sig(item.def_id);
let ty = sig.skip_binder();
ty.inputs().len() == 1
} else {
false
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.mods.rs
Expand Up @@ -49,6 +49,7 @@ mod entry;
mod enum_clike;
mod enum_variants;
mod eq_op;
mod equatable_if_let;
mod erasing_op;
mod escape;
mod eta_reduction;
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Expand Up @@ -116,6 +116,7 @@ store.register_lints(&[
enum_variants::MODULE_NAME_REPETITIONS,
eq_op::EQ_OP,
eq_op::OP_REF,
equatable_if_let::EQUATABLE_IF_LET,
erasing_op::ERASING_OP,
escape::BOXED_LOCAL,
eta_reduction::REDUNDANT_CLOSURE,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_nursery.rs
Expand Up @@ -8,6 +8,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(copies::BRANCHES_SHARING_CODE),
LintId::of(disallowed_method::DISALLOWED_METHOD),
LintId::of(disallowed_type::DISALLOWED_TYPE),
LintId::of(equatable_if_let::EQUATABLE_IF_LET),
LintId::of(fallible_impl_from::FALLIBLE_IMPL_FROM),
LintId::of(floating_point_arithmetic::IMPRECISE_FLOPS),
LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -485,6 +485,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(option_if_let_else::OptionIfLetElse));
store.register_late_pass(|| Box::new(future_not_send::FutureNotSend));
store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));
store.register_late_pass(|| Box::new(equatable_if_let::PatternEquality));
store.register_late_pass(|| Box::new(mut_mutex_lock::MutMutexLock));
store.register_late_pass(|| Box::new(match_on_vec_items::MatchOnVecItems));
store.register_late_pass(|| Box::new(manual_async_fn::ManualAsyncFn));
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/mut_range_bound.rs
Expand Up @@ -92,7 +92,7 @@ impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> {
fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}

fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) {
if let ty::BorrowKind::MutBorrow = bk {
if bk == ty::BorrowKind::MutBorrow {
if let PlaceBase::Local(id) = cmt.place.base {
if Some(id) == self.hir_id_low && !BreakAfterExprVisitor::is_found(self.cx, diag_expr_id) {
self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id));
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/needless_range_loop.rs
Expand Up @@ -95,7 +95,7 @@ pub(super) fn check<'tcx>(
let mut take_expr = end;

if let ExprKind::Binary(ref op, left, right) = end.kind {
if let BinOpKind::Add = op.node {
if op.node == BinOpKind::Add {
let start_equal_left = SpanlessEq::new(cx).eq_expr(start, left);
let start_equal_right = SpanlessEq::new(cx).eq_expr(start, right);

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/never_loop.rs
Expand Up @@ -14,7 +14,7 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
NeverLoopResult::AlwaysBreak => {
span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| {
if_chain! {
if let LoopSource::ForLoop = source;
if source == LoopSource::ForLoop;
if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1);
if let Some(ForLoop { arg: iterator, pat, span: for_span, .. }) = ForLoop::hir(parent_match);
then {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/manual_async_fn.rs
Expand Up @@ -49,7 +49,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAsyncFn {
) {
if_chain! {
if let Some(header) = kind.header();
if let IsAsync::NotAsync = header.asyncness;
if header.asyncness == IsAsync::NotAsync;
// Check that this function returns `impl Future`
if let FnRetTy::Return(ret_ty) = decl.output;
if let Some((trait_ref, output_lifetimes)) = future_trait_ref(cx, ret_ty);
Expand Down Expand Up @@ -178,7 +178,7 @@ fn desugared_async_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>)
if args.len() == 1;
if let Expr{kind: ExprKind::Closure(_, _, body_id, ..), ..} = args[0];
let closure_body = cx.tcx.hir().body(body_id);
if let Some(GeneratorKind::Async(AsyncGeneratorKind::Block)) = closure_body.generator_kind;
if closure_body.generator_kind == Some(GeneratorKind::Async(AsyncGeneratorKind::Block));
then {
return Some(closure_body);
}
Expand Down
14 changes: 7 additions & 7 deletions clippy_lints/src/methods/or_fun_call.rs
Expand Up @@ -178,15 +178,15 @@ pub(super) fn check<'tcx>(
hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
},
hir::ExprKind::Block(block, _) => {
if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules {
if let Some(block_expr) = block.expr {
if let hir::ExprKind::MethodCall(..) = block_expr.kind {
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
}
hir::ExprKind::Block(block, _)
if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) =>
{
if let Some(block_expr) = block.expr {
if let hir::ExprKind::MethodCall(..) = block_expr.kind {
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
}
}
},
}
_ => (),
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/modulo_arithmetic.rs
Expand Up @@ -128,7 +128,7 @@ impl<'tcx> LateLintPass<'tcx> for ModuloArithmetic {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
match &expr.kind {
ExprKind::Binary(op, lhs, rhs) | ExprKind::AssignOp(op, lhs, rhs) => {
if let BinOpKind::Rem = op.node {
if op.node == BinOpKind::Rem {
let lhs_operand = analyze_operand(lhs, cx, expr);
let rhs_operand = analyze_operand(rhs, cx, expr);
if_chain! {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/needless_bool.rs
Expand Up @@ -248,7 +248,7 @@ fn check_comparison<'a, 'tcx>(
if l_ty.is_bool() && r_ty.is_bool() {
let mut applicability = Applicability::MachineApplicable;

if let BinOpKind::Eq = op.node {
if op.node == BinOpKind::Eq {
let expression_info = one_side_is_unary_not(left_side, right_side);
if expression_info.one_side_is_unary_not {
span_lint_and_sugg(
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/neg_multiply.rs
Expand Up @@ -46,7 +46,7 @@ impl<'tcx> LateLintPass<'tcx> for NegMultiply {
fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) {
if_chain! {
if let ExprKind::Lit(ref l) = lit.kind;
if let Constant::Int(1) = consts::lit_to_constant(&l.node, cx.typeck_results().expr_ty_opt(lit));
if consts::lit_to_constant(&l.node, cx.typeck_results().expr_ty_opt(lit)) == Constant::Int(1);
if cx.typeck_results().expr_ty(exp).is_integral();
then {
span_lint(cx, NEG_MULTIPLY, span, "negation by multiplying with `-1`");
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/new_without_default.rs
Expand Up @@ -69,7 +69,7 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
}) = item.kind
{
for assoc_item in items {
if let hir::AssocItemKind::Fn { has_self: false } = assoc_item.kind {
if assoc_item.kind == (hir::AssocItemKind::Fn { has_self: false }) {
let impl_item = cx.tcx.hir().impl_item(assoc_item.id);
if in_external_macro(cx.sess(), impl_item.span) {
return;
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/non_send_fields_in_send_ty.rs
Expand Up @@ -81,7 +81,7 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
if let Some(trait_ref) = &hir_impl.of_trait;
if let Some(trait_id) = trait_ref.trait_def_id();
if send_trait == trait_id;
if let ImplPolarity::Positive = hir_impl.polarity;
if hir_impl.polarity == ImplPolarity::Positive;
if let Some(ty_trait_ref) = cx.tcx.impl_trait_ref(item.def_id);
if let self_ty = ty_trait_ref.self_ty();
if let ty::Adt(adt_def, impl_trait_substs) = self_ty.kind();
Expand Down

0 comments on commit 388a3d0

Please sign in to comment.