diff --git a/CHANGELOG.md b/CHANGELOG.md index b2e9f6d1dd3e..dbf7f3b15032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6456,6 +6456,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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 375d179681da..127f95c54dc9 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -303,6 +303,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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 149785c59447..b394426beb04 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -212,6 +212,7 @@ mod manual_div_ceil; 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; @@ -834,5 +835,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)); store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites)); store.register_late_pass(|_| Box::new(replace_box::ReplaceBox)); + store.register_late_pass(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_ilog2.rs b/clippy_lints/src/manual_ilog2.rs new file mode 100644 index 000000000000..335e135590d8 --- /dev/null +++ b/clippy_lints/src/manual_ilog2.rs @@ -0,0 +1,115 @@ +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::res::{MaybeDef, MaybeTypeckRes}; +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()` or `x.ilog(2)` 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(); + /// let log = x.ilog(2); + /// ``` + /// Use instead: + /// ```no_run + /// let x: u32 = 5; + /// let log = x.ilog2(); + /// let log = x.ilog2(); + /// ``` + #[clippy::version = "1.92.0"] + pub MANUAL_ILOG2, + complexity, + "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 + // Whether `leading_zeros` is an inherent method, i.e. doesn't come from some unrelated trait + && cx.ty_based_def(right).opt_parent(cx).is_impl(cx) + && let ty = cx.typeck_results().expr_ty(recv) + && let Some(bit_width) = match ty.kind() { + ty::Int(int_ty) => int_ty.bit_width(), + ty::Uint(uint_ty) => uint_ty.bit_width(), + _ => return, + } + && val == u128::from(bit_width) - 1 + && self.msrv.meets(cx, msrvs::ILOG2) + && !is_from_proc_macro(cx, expr) => + { + emit(cx, recv, expr); + }, + + // `n.ilog(2)` + ExprKind::MethodCall(ilog, recv, [two], _) + if expr.span.eq_ctxt(two.span) + && ilog.ident.name == sym::ilog + // Whether `ilog` is an inherent method, i.e. doesn't come from some unrelated trait + && cx.ty_based_def(expr).opt_parent(cx).is_impl(cx) + && let ExprKind::Lit(lit) = two.kind + && let LitKind::Int(Pu128(2), _) = lit.node + && cx.typeck_results().expr_ty(recv).is_integral() + && self.msrv.meets(cx, msrvs::ILOG2) + && !is_from_proc_macro(cx, expr) => + { + emit(cx, recv, expr); + }, + + _ => {}, + } + } +} + +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, + ); +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 62041fc631c0..4a8ef48cb006 100644 --- a/clippy_utils/src/msrvs.rs +++ b/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 } diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index 2b22f344e8c0..8277bfb2b697 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -175,6 +175,7 @@ generate! { has_significant_drop, hidden_glob_reexports, hygiene, + ilog, insert, insert_str, inspect, @@ -202,6 +203,7 @@ generate! { join, kw, lazy_static, + leading_zeros, lint_vec, ln, lock, diff --git a/tests/ui/manual_ilog2.fixed b/tests/ui/manual_ilog2.fixed new file mode 100644 index 000000000000..a0f6d9392c30 --- /dev/null +++ b/tests/ui/manual_ilog2.fixed @@ -0,0 +1,32 @@ +//@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 + 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! two { + () => { + 2 + }; + }; + + macro_rules! thirty_one { + () => { + 31 + }; + }; + + a.ilog(two!()); + thirty_one!() - a.leading_zeros(); + + external!($a.ilog(2)); + with_span!(span; a.ilog(2)); +} diff --git a/tests/ui/manual_ilog2.rs b/tests/ui/manual_ilog2.rs new file mode 100644 index 000000000000..bd4b5d9d3c0d --- /dev/null +++ b/tests/ui/manual_ilog2.rs @@ -0,0 +1,32 @@ +//@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 + a.ilog(2); //~ 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! two { + () => { + 2 + }; + }; + + macro_rules! thirty_one { + () => { + 31 + }; + }; + + a.ilog(two!()); + thirty_one!() - a.leading_zeros(); + + external!($a.ilog(2)); + with_span!(span; a.ilog(2)); +} diff --git a/tests/ui/manual_ilog2.stderr b/tests/ui/manual_ilog2.stderr new file mode 100644 index 000000000000..7c9694f35330 --- /dev/null +++ b/tests/ui/manual_ilog2.stderr @@ -0,0 +1,23 @@ +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 | a.ilog(2); + | ^^^^^^^^^ help: try: `a.ilog2()` + +error: manually reimplementing `ilog2` + --> tests/ui/manual_ilog2.rs:11:5 + | +LL | 63 - b.leading_zeros(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `b.ilog2()` + +error: aborting due to 3 previous errors +