Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint equatable_if_let #7762

Merged
merged 1 commit into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading