feat(manual_ilog2): new lint

Author: ada4a

CHANGELOG.md

@@ -6541,6 +6541,7 @@ Released 2018-09-13
 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
 [`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
 [`manual_ignore_case_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ignore_case_cmp
+[`manual_ilog2`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ilog2
 [`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
 [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check

clippy_lints/src/declared_lints.rs

@@ -300,6 +300,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
     crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
     crate::manual_ignore_case_cmp::MANUAL_IGNORE_CASE_CMP_INFO,
+    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_let_else::MANUAL_LET_ELSE_INFO,

clippy_lints/src/lib.rs

@@ -201,6 +201,7 @@ mod manual_clamp;
 mod manual_float_methods;
 mod manual_hash_one;
 mod manual_ignore_case_cmp;
+mod manual_ilog2;
 mod manual_is_ascii_check;
 mod manual_is_power_of_two;
 mod manual_let_else;
@@ -848,6 +849,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)),
         Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
         Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
+        Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
         // add late passes here, used by `cargo dev new_lint`
     ];
     store.late_passes.extend(late_lints);

clippy_lints/src/manual_ilog2.rs

@@ -0,0 +1,101 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{is_from_proc_macro, sym};
+use rustc_ast::LitKind;
+use rustc_data_structures::packed::Pu128;
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::ty;
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for expressions like `31 - x.leading_zeros()` which are manual
+    /// reimplementations of `x.ilog2()`
+    ///
+    /// ### Why is this bad?
+    /// Manual reimplementations of `ilog2` increase code complexity for little benefit.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let x: u32 = 5;
+    /// let log = 31 - x.leading_zeros();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let x: u32 = 5;
+    /// let log = x.ilog2();
+    /// ```
+    #[clippy::version = "1.93.0"]
+    pub MANUAL_ILOG2,
+    pedantic,
+    "manually reimplementing `ilog2`"
+}
+
+pub struct ManualIlog2 {
+    msrv: Msrv,
+}
+
+impl ManualIlog2 {
+    pub fn new(conf: &Conf) -> Self {
+        Self { msrv: conf.msrv }
+    }
+}
+
+impl_lint_pass!(ManualIlog2 => [MANUAL_ILOG2]);
+
+impl LateLintPass<'_> for ManualIlog2 {
+    fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if expr.span.in_external_macro(cx.sess().source_map()) {
+            return;
+        }
+
+        match expr.kind {
+            // `BIT_WIDTH - 1 - n.leading_zeros()`
+            ExprKind::Binary(op, left, right)
+                if left.span.eq_ctxt(right.span)
+                    && op.node == BinOpKind::Sub
+                    && let ExprKind::Lit(lit) = left.kind
+                    && let LitKind::Int(Pu128(val), _) = lit.node
+                    && let ExprKind::MethodCall(leading_zeros, recv, [], _) = right.kind
+                    && leading_zeros.ident.name == sym::leading_zeros
+                    && let ty = cx.typeck_results().expr_ty(recv)
+                    && let Some(bit_width) = match ty.kind() {
+                        ty::Uint(uint_ty) => uint_ty.bit_width(),
+                        ty::Int(_) => {
+                            // On non-positive integers, `ilog2` would panic, which might be a sign that the author does
+                            // in fact want to calculate something different, so stay on the safer side and don't
+                            // suggest anything.
+                            return;
+                        },
+                        _ => return,
+                    }
+                    && val == u128::from(bit_width) - 1
+                    && self.msrv.meets(cx, msrvs::ILOG2)
+                    && !is_from_proc_macro(cx, expr) =>
+            {
+                emit(cx, recv, expr);
+            },
+
+            // NOTE: we don't check for `n.ilog(2)`, as the compiler will specialize that to `ilog2` by itself
+            _ => {},
+        }
+    }
+}
+
+fn emit(cx: &LateContext<'_>, recv: &Expr<'_>, full_expr: &Expr<'_>) {
+    let mut app = Applicability::MachineApplicable;
+    let recv = snippet_with_applicability(cx, recv.span, "_", &mut app);
+    span_lint_and_sugg(
+        cx,
+        MANUAL_ILOG2,
+        full_expr.span,
+        "manually reimplementing `ilog2`",
+        "try",
+        format!("{recv}.ilog2()"),
+        app,
+    );
+}

clippy_utils/src/msrvs.rs

@@ -40,6 +40,7 @@ msrv_aliases! {
     1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
     1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
     1,68,0 { PATH_MAIN_SEPARATOR_STR }
+    1,67,0 { ILOG2 }
     1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
     1,63,0 { CLONE_INTO, CONST_SLICE_FROM_REF }
     1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CONST_EXTERN_C_FN }

clippy_utils/src/sym.rs

@@ -207,6 +207,7 @@ generate! {
     join,
     kw,
     lazy_static,
+    leading_zeros,
     lint_vec,
     ln,
     lock,

tests/ui/manual_ilog2.fixed

@@ -0,0 +1,22 @@
+//@aux-build:proc_macros.rs
+#![warn(clippy::manual_ilog2)]
+#![allow(clippy::unnecessary_operation)]
+
+use proc_macros::{external, with_span};
+
+fn foo(a: u32, b: u64) {
+    a.ilog2(); //~ manual_ilog2
+    b.ilog2(); //~ manual_ilog2
+    64 - b.leading_zeros(); // No lint because manual ilog2 is `BIT_WIDTH - 1 - x.leading_zeros()`
+
+    // don't lint when macros are involved
+    macro_rules! thirty_one {
+        () => {
+            31
+        };
+    };
+    thirty_one!() - a.leading_zeros();
+
+    external!(31 - $a.leading_zeros());
+    with_span!(span; 31 - a.leading_zeros());
+}

tests/ui/manual_ilog2.rs

@@ -0,0 +1,22 @@
+//@aux-build:proc_macros.rs
+#![warn(clippy::manual_ilog2)]
+#![allow(clippy::unnecessary_operation)]
+
+use proc_macros::{external, with_span};
+
+fn foo(a: u32, b: u64) {
+    31 - a.leading_zeros(); //~ manual_ilog2
+    63 - b.leading_zeros(); //~ manual_ilog2
+    64 - b.leading_zeros(); // No lint because manual ilog2 is `BIT_WIDTH - 1 - x.leading_zeros()`
+
+    // don't lint when macros are involved
+    macro_rules! thirty_one {
+        () => {
+            31
+        };
+    };
+    thirty_one!() - a.leading_zeros();
+
+    external!(31 - $a.leading_zeros());
+    with_span!(span; 31 - a.leading_zeros());
+}

tests/ui/manual_ilog2.stderr

@@ -0,0 +1,17 @@
+error: manually reimplementing `ilog2`
+  --> tests/ui/manual_ilog2.rs:8:5
+   |
+LL |     31 - a.leading_zeros();
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: try: `a.ilog2()`
+   |
+   = note: `-D clippy::manual-ilog2` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_ilog2)]`
+
+error: manually reimplementing `ilog2`
+  --> tests/ui/manual_ilog2.rs:9:5
+   |
+LL |     63 - b.leading_zeros();
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: try: `b.ilog2()`
+
+error: aborting due to 2 previous errors
+