Move IdentityOp into Operators lint pass

Author: Jarcho

clippy_lints/src/lib.register_all.rs

@@ -82,7 +82,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(functions::RESULT_UNIT_ERR),
     LintId::of(functions::TOO_MANY_ARGUMENTS),
     LintId::of(get_first::GET_FIRST),
-    LintId::of(identity_op::IDENTITY_OP),
     LintId::of(if_let_mutex::IF_LET_MUTEX),
     LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
     LintId::of(infinite_iter::INFINITE_ITER),
@@ -250,6 +249,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(operators::EQ_OP),
     LintId::of(operators::ERASING_OP),
     LintId::of(operators::FLOAT_EQUALITY_WITHOUT_ABS),
+    LintId::of(operators::IDENTITY_OP),
     LintId::of(operators::INEFFECTIVE_BIT_MASK),
     LintId::of(operators::MISREFACTORED_ASSIGN_OP),
     LintId::of(operators::OP_REF),

clippy_lints/src/lib.register_complexity.rs

@@ -13,7 +13,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(explicit_write::EXPLICIT_WRITE),
     LintId::of(format::USELESS_FORMAT),
     LintId::of(functions::TOO_MANY_ARGUMENTS),
-    LintId::of(identity_op::IDENTITY_OP),
     LintId::of(int_plus_one::INT_PLUS_ONE),
     LintId::of(lifetimes::EXTRA_UNUSED_LIFETIMES),
     LintId::of(lifetimes::NEEDLESS_LIFETIMES),
@@ -68,6 +67,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(no_effect::UNNECESSARY_OPERATION),
     LintId::of(operators::DOUBLE_COMPARISONS),
     LintId::of(operators::DURATION_SUBSEC),
+    LintId::of(operators::IDENTITY_OP),
     LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
     LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
     LintId::of(precedence::PRECEDENCE),

clippy_lints/src/lib.register_lints.rs

@@ -172,7 +172,6 @@ store.register_lints(&[
     functions::TOO_MANY_LINES,
     future_not_send::FUTURE_NOT_SEND,
     get_first::GET_FIRST,
-    identity_op::IDENTITY_OP,
     if_let_mutex::IF_LET_MUTEX,
     if_not_else::IF_NOT_ELSE,
     if_then_some_else_none::IF_THEN_SOME_ELSE_NONE,
@@ -420,6 +419,7 @@ store.register_lints(&[
     operators::ERASING_OP,
     operators::FLOAT_ARITHMETIC,
     operators::FLOAT_EQUALITY_WITHOUT_ABS,
+    operators::IDENTITY_OP,
     operators::INEFFECTIVE_BIT_MASK,
     operators::INTEGER_ARITHMETIC,
     operators::MISREFACTORED_ASSIGN_OP,

clippy_lints/src/lib.rs

@@ -236,7 +236,6 @@ mod from_str_radix_10;
 mod functions;
 mod future_not_send;
 mod get_first;
-mod identity_op;
 mod if_let_mutex;
 mod if_not_else;
 mod if_then_some_else_none;
@@ -548,7 +547,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(needless_for_each::NeedlessForEach));
     store.register_late_pass(|| Box::new(misc::MiscLints));
     store.register_late_pass(|| Box::new(eta_reduction::EtaReduction));
-    store.register_late_pass(|| Box::new(identity_op::IdentityOp));
     store.register_late_pass(|| Box::new(mut_mut::MutMut));
     store.register_late_pass(|| Box::new(mut_reference::UnnecessaryMutPassed));
     store.register_late_pass(|| Box::new(len_zero::LenZero));

clippy_lints/src/identity_op.rs renamed to clippy_lints/src/operators/identity_op.rs

@@ -3,61 +3,40 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{clip, unsext};
 use rustc_errors::Applicability;
-use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, Node};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_hir::{BinOpKind, Expr, ExprKind, Node};
+use rustc_lint::LateContext;
 use rustc_middle::ty;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for identity operations, e.g., `x + 0`.
-    ///
-    /// ### Why is this bad?
-    /// This code can be removed without changing the
-    /// meaning. So it just obscures what's going on. Delete it mercilessly.
-    ///
-    /// ### Example
-    /// ```rust
-    /// # let x = 1;
-    /// x / 1 + 0 * 1 - 0 | 0;
-    /// ```
-    #[clippy::version = "pre 1.29.0"]
-    pub IDENTITY_OP,
-    complexity,
-    "using identity operations, e.g., `x + 0` or `y / 1`"
-}
-
-declare_lint_pass!(IdentityOp => [IDENTITY_OP]);
+use super::IDENTITY_OP;
 
-impl<'tcx> LateLintPass<'tcx> for IdentityOp {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if expr.span.from_expansion() {
-            return;
-        }
-        if let ExprKind::Binary(cmp, left, right) = &expr.kind {
-            if !is_allowed(cx, *cmp, left, right) {
-                match cmp.node {
-                    BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
-                        check(cx, left, 0, expr.span, right.span, needs_parenthesis(cx, expr, right));
-                        check(cx, right, 0, expr.span, left.span, Parens::Unneeded);
-                    },
-                    BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
-                        check(cx, right, 0, expr.span, left.span, Parens::Unneeded);
-                    },
-                    BinOpKind::Mul => {
-                        check(cx, left, 1, expr.span, right.span, needs_parenthesis(cx, expr, right));
-                        check(cx, right, 1, expr.span, left.span, Parens::Unneeded);
-                    },
-                    BinOpKind::Div => check(cx, right, 1, expr.span, left.span, Parens::Unneeded),
-                    BinOpKind::BitAnd => {
-                        check(cx, left, -1, expr.span, right.span, needs_parenthesis(cx, expr, right));
-                        check(cx, right, -1, expr.span, left.span, Parens::Unneeded);
-                    },
-                    BinOpKind::Rem => check_remainder(cx, left, right, expr.span, left.span),
-                    _ => (),
-                }
-            }
+pub(crate) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    op: BinOpKind,
+    left: &'tcx Expr<'_>,
+    right: &'tcx Expr<'_>,
+) {
+    if !is_allowed(cx, op, left, right) {
+        match op {
+            BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
+                check_op(cx, left, 0, expr.span, right.span, needs_parenthesis(cx, expr, right));
+                check_op(cx, right, 0, expr.span, left.span, Parens::Unneeded);
+            },
+            BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
+                check_op(cx, right, 0, expr.span, left.span, Parens::Unneeded);
+            },
+            BinOpKind::Mul => {
+                check_op(cx, left, 1, expr.span, right.span, needs_parenthesis(cx, expr, right));
+                check_op(cx, right, 1, expr.span, left.span, Parens::Unneeded);
+            },
+            BinOpKind::Div => check_op(cx, right, 1, expr.span, left.span, Parens::Unneeded),
+            BinOpKind::BitAnd => {
+                check_op(cx, left, -1, expr.span, right.span, needs_parenthesis(cx, expr, right));
+                check_op(cx, right, -1, expr.span, left.span, Parens::Unneeded);
+            },
+            BinOpKind::Rem => check_remainder(cx, left, right, expr.span, left.span),
+            _ => (),
         }
     }
 }
@@ -108,12 +87,12 @@ fn needs_parenthesis(cx: &LateContext<'_>, binary: &Expr<'_>, right: &Expr<'_>)
     Parens::Needed
 }
 
-fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool {
+fn is_allowed(cx: &LateContext<'_>, cmp: BinOpKind, left: &Expr<'_>, right: &Expr<'_>) -> bool {
     // This lint applies to integers
     !cx.typeck_results().expr_ty(left).peel_refs().is_integral()
         || !cx.typeck_results().expr_ty(right).peel_refs().is_integral()
         // `1 << 0` is a common pattern in bit manipulation code
-        || (cmp.node == BinOpKind::Shl
+        || (cmp == BinOpKind::Shl
             && constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0))
             && constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1)))
 }
@@ -130,7 +109,7 @@ fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span
     }
 }
 
-fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens) {
+fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens) {
     if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) {
         let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() {
             ty::Int(ity) => unsext(cx.tcx, -1_i128, ity),

clippy_lints/src/operators/mod.rs

@@ -11,6 +11,7 @@ mod duration_subsec;
 mod eq_op;
 mod erasing_op;
 mod float_equality_without_abs;
+mod identity_op;
 mod misrefactored_assign_op;
 mod op_ref;
 mod verbose_bit_mask;
@@ -411,6 +412,25 @@ declare_clippy_lint! {
     "float equality check without `.abs()`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for identity operations, e.g., `x + 0`.
+    ///
+    /// ### Why is this bad?
+    /// This code can be removed without changing the
+    /// meaning. So it just obscures what's going on. Delete it mercilessly.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # let x = 1;
+    /// x / 1 + 0 * 1 - 0 | 0;
+    /// ```
+    #[clippy::version = "pre 1.29.0"]
+    pub IDENTITY_OP,
+    complexity,
+    "using identity operations, e.g., `x + 0` or `y / 1`"
+}
+
 pub struct Operators {
     arithmetic_context: arithmetic::Context,
     verbose_bit_mask_threshold: u64,
@@ -430,6 +450,7 @@ impl_lint_pass!(Operators => [
     OP_REF,
     ERASING_OP,
     FLOAT_EQUALITY_WITHOUT_ABS,
+    IDENTITY_OP,
 ]);
 impl Operators {
     pub fn new(verbose_bit_mask_threshold: u64) -> Self {
@@ -451,6 +472,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                         op_ref::check(cx, e, op.node, lhs, rhs);
                     }
                     erasing_op::check(cx, e, op.node, lhs, rhs);
+                    identity_op::check(cx, e, op.node, lhs, rhs);
                 }
                 self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
                 bit_mask::check(cx, e, op.node, lhs, rhs);