Add new lint manual_isolate_lowest_one

CHANGELOG.md

@@ -6906,6 +6906,7 @@ Released 2018-09-13
 [`manual_is_multiple_of`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_multiple_of
 [`manual_is_power_of_two`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_power_of_two
 [`manual_is_variant_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_variant_and
+[`manual_isolate_lowest_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_isolate_lowest_one
 [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
 [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str
 [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map

clippy_lints/src/declared_lints.rs

@@ -311,6 +311,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::manual_ilog2::MANUAL_ILOG2_INFO,
     crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
     crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO,
+    crate::manual_isolate_lowest_one::MANUAL_ISOLATE_LOWEST_ONE_INFO,
     crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,

clippy_lints/src/lib.rs

@@ -208,6 +208,7 @@ mod manual_ignore_case_cmp;
 mod manual_ilog2;
 mod manual_is_ascii_check;
 mod manual_is_power_of_two;
+mod manual_isolate_lowest_one;
 mod manual_let_else;
 mod manual_main_separator_str;
 mod manual_non_exhaustive;
@@ -863,6 +864,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
         Box::new(move |tcx| Box::new(disallowed_fields::DisallowedFields::new(tcx, conf))),
         Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
+        Box::new(move |_| Box::new(manual_isolate_lowest_one::ManualIsolateLowestOne::new(conf))),
         Box::new(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)),
         Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))),
         Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))),

clippy_lints/src/manual_isolate_lowest_one.rs

@@ -0,0 +1,113 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{SpanlessEq, is_from_proc_macro, sym};
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+use rustc_span::SyntaxContext;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for expressions like `x & -x` or `x & x.wrapping_neg()`, which are manual
+    /// reimplementations of `x.isolate_lowest_one()`.
+    ///
+    /// ### Why is this bad?
+    /// `x.isolate_lowest_one()` is clearer than the bitwise trick. It also avoids the
+    /// overflow that occurs when `x == T::MIN` for signed types using the `-` operator,
+    /// and preserves non-zero type information for `NonZero<T>`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let x: u32 = 5;
+    /// let lsb = x & x.wrapping_neg();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let x: u32 = 5;
+    /// let lsb = x.isolate_lowest_one();
+    /// ```
+    #[clippy::version = "1.97.0"]
+    pub MANUAL_ISOLATE_LOWEST_ONE,
+    complexity,
+    "manually reimplementing `isolate_lowest_one`"
+}
+
+impl_lint_pass!(ManualIsolateLowestOne => [MANUAL_ISOLATE_LOWEST_ONE]);
+
+pub struct ManualIsolateLowestOne {
+    msrv: Msrv,
+}
+
+impl ManualIsolateLowestOne {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self { msrv: conf.msrv }
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for ManualIsolateLowestOne {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if expr.span.from_expansion() {
+            return;
+        }
+
+        let ExprKind::Binary(op, lhs, rhs) = expr.kind else {
+            return;
+        };
+        if op.node != BinOpKind::BitAnd || lhs.span.from_expansion() || rhs.span.from_expansion() {
+            return;
+        }
+
+        let ctxt = expr.span.ctxt();
+        let recv = is_negation_pair(cx, ctxt, lhs, rhs).or_else(|| is_negation_pair(cx, ctxt, rhs, lhs));
+        let Some(recv) = recv else { return };
+
+        if !cx.typeck_results().expr_ty_adjusted(recv).is_integral() {
+            return;
+        }
+
+        if !self.msrv.meets(cx, msrvs::ISOLATE_LOWEST_ONE) {
+            return;
+        }
+
+        if is_from_proc_macro(cx, expr) {
+            return;
+        }
+
+        let mut applicability = Applicability::MachineApplicable;
+        let snippet = Sugg::hir_with_context(cx, recv, ctxt, "_", &mut applicability);
+
+        span_lint_and_sugg(
+            cx,
+            MANUAL_ISOLATE_LOWEST_ONE,
+            expr.span,
+            "manually reimplementing `isolate_lowest_one`",
+            "consider using `.isolate_lowest_one()`",
+            format!("{}.isolate_lowest_one()", snippet.maybe_paren()),
+            applicability,
+        );
+    }
+}
+
+/// Returns `Some(base)` if `negated` is `-base` or `base.wrapping_neg()` (where `base` is
+/// structurally equal to `expected_base`).
+fn is_negation_pair<'tcx>(
+    cx: &LateContext<'tcx>,
+    ctxt: SyntaxContext,
+    expected_base: &'tcx Expr<'tcx>,
+    negated: &'tcx Expr<'tcx>,
+) -> Option<&'tcx Expr<'tcx>> {
+    match negated.kind {
+        ExprKind::Unary(UnOp::Neg, inner) if SpanlessEq::new(cx).eq_expr(ctxt, expected_base, inner) => {
+            Some(expected_base)
+        },
+        ExprKind::MethodCall(method, inner, [], _)
+            if method.ident.name == sym::wrapping_neg && SpanlessEq::new(cx).eq_expr(ctxt, expected_base, inner) =>
+        {
+            Some(expected_base)
+        },
+        _ => None,
+    }
+}

clippy_utils/src/msrvs.rs

@@ -23,6 +23,7 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
+    1,97,0 { ISOLATE_LOWEST_ONE }
     1,93,0 { VEC_DEQUE_POP_BACK_IF, VEC_DEQUE_POP_FRONT_IF }
     1,91,0 { DURATION_FROM_MINUTES_HOURS }
     1,88,0 { LET_CHAINS }

clippy_utils/src/sym.rs

@@ -649,6 +649,7 @@ generate! {
     warnings,
     wildcard_imports,
     with_capacity,
+    wrapping_neg,
     wrapping_offset,
     write,
     write_unaligned,

tests/ui/manual_isolate_lowest_one.fixed

@@ -0,0 +1,34 @@
+//@aux-build:proc_macros.rs
+#![warn(clippy::manual_isolate_lowest_one)]
+#![allow(clippy::unnecessary_operation, clippy::no_effect, dead_code)]
+
+use proc_macros::{external, with_span};
+
+fn unsigned(a: u32, b: u64) {
+    let _ = a.isolate_lowest_one(); //~ manual_isolate_lowest_one
+    let _ = a.isolate_lowest_one(); //~ manual_isolate_lowest_one
+    let _ = b.isolate_lowest_one(); //~ manual_isolate_lowest_one
+}
+
+fn signed(a: i32) {
+    let _ = a.isolate_lowest_one(); //~ manual_isolate_lowest_one
+    let _ = a.isolate_lowest_one(); //~ manual_isolate_lowest_one
+    let _ = a.isolate_lowest_one(); //~ manual_isolate_lowest_one
+}
+
+fn no_lint_different_operand(a: i32, c: i32) {
+    // Different operands — must not lint.
+    let _ = a & c.wrapping_neg();
+    let _ = a & -c;
+}
+
+fn no_lint_macros(a: u32) {
+    macro_rules! same {
+        ($x:expr) => {
+            $x & $x.wrapping_neg()
+        };
+    }
+    same!(a);
+    external!($a & $a.wrapping_neg());
+    with_span!(span; a & a.wrapping_neg());
+}

tests/ui/manual_isolate_lowest_one.rs

@@ -0,0 +1,34 @@
+//@aux-build:proc_macros.rs
+#![warn(clippy::manual_isolate_lowest_one)]
+#![allow(clippy::unnecessary_operation, clippy::no_effect, dead_code)]
+
+use proc_macros::{external, with_span};
+
+fn unsigned(a: u32, b: u64) {
+    let _ = a & a.wrapping_neg(); //~ manual_isolate_lowest_one
+    let _ = a.wrapping_neg() & a; //~ manual_isolate_lowest_one
+    let _ = b & b.wrapping_neg(); //~ manual_isolate_lowest_one
+}
+
+fn signed(a: i32) {
+    let _ = a & -a; //~ manual_isolate_lowest_one
+    let _ = -a & a; //~ manual_isolate_lowest_one
+    let _ = a & a.wrapping_neg(); //~ manual_isolate_lowest_one
+}
+
+fn no_lint_different_operand(a: i32, c: i32) {
+    // Different operands — must not lint.
+    let _ = a & c.wrapping_neg();
+    let _ = a & -c;
+}
+
+fn no_lint_macros(a: u32) {
+    macro_rules! same {
+        ($x:expr) => {
+            $x & $x.wrapping_neg()
+        };
+    }
+    same!(a);
+    external!($a & $a.wrapping_neg());
+    with_span!(span; a & a.wrapping_neg());
+}

tests/ui/manual_isolate_lowest_one.stderr

@@ -0,0 +1,41 @@
+error: manually reimplementing `isolate_lowest_one`
+  --> tests/ui/manual_isolate_lowest_one.rs:8:13
+   |
+LL |     let _ = a & a.wrapping_neg();
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider using `.isolate_lowest_one()`: `a.isolate_lowest_one()`
+   |
+   = note: `-D clippy::manual-isolate-lowest-one` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_isolate_lowest_one)]`
+
+error: manually reimplementing `isolate_lowest_one`
+  --> tests/ui/manual_isolate_lowest_one.rs:9:13
+   |
+LL |     let _ = a.wrapping_neg() & a;
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider using `.isolate_lowest_one()`: `a.isolate_lowest_one()`
+
+error: manually reimplementing `isolate_lowest_one`
+  --> tests/ui/manual_isolate_lowest_one.rs:10:13
+   |
+LL |     let _ = b & b.wrapping_neg();
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider using `.isolate_lowest_one()`: `b.isolate_lowest_one()`
+
+error: manually reimplementing `isolate_lowest_one`
+  --> tests/ui/manual_isolate_lowest_one.rs:14:13
+   |
+LL |     let _ = a & -a;
+   |             ^^^^^^ help: consider using `.isolate_lowest_one()`: `a.isolate_lowest_one()`
+
+error: manually reimplementing `isolate_lowest_one`
+  --> tests/ui/manual_isolate_lowest_one.rs:15:13
+   |
+LL |     let _ = -a & a;
+   |             ^^^^^^ help: consider using `.isolate_lowest_one()`: `a.isolate_lowest_one()`
+
+error: manually reimplementing `isolate_lowest_one`
+  --> tests/ui/manual_isolate_lowest_one.rs:16:13
+   |
+LL |     let _ = a & a.wrapping_neg();
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider using `.isolate_lowest_one()`: `a.isolate_lowest_one()`
+
+error: aborting due to 6 previous errors
+