Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Check MSRV before suggesting fix in const context
  • Loading branch information
samueltardieu committed Apr 14, 2025
commit 4d343d56e1ffb45b14d00f87736d405e5670970c
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
* [`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_is_ascii_check`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check)
* [`manual_is_power_of_two`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_power_of_two)
* [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else)
* [`manual_midpoint`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_midpoint)
* [`manual_non_exhaustive`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive)
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,7 @@ define_Conf! {
manual_flatten,
manual_hash_one,
manual_is_ascii_check,
manual_is_power_of_two,
manual_let_else,
manual_midpoint,
manual_non_exhaustive,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
store.register_late_pass(move |_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo::new(conf)));
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
store.register_late_pass(|_| Box::new(literal_string_with_formatting_args::LiteralStringWithFormattingArg));
store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
Expand Down
60 changes: 38 additions & 22 deletions clippy_lints/src/manual_is_power_of_two.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
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_integer_literal};
use clippy_utils::{SpanlessEq, is_in_const_context, is_integer_literal};
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;

declare_clippy_lint! {
/// ### What it does
Expand All @@ -31,7 +33,36 @@ declare_clippy_lint! {
"manually reimplementing `is_power_of_two`"
}

declare_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
pub struct ManualIsPowerOfTwo {
msrv: Msrv,
}

impl_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);

impl ManualIsPowerOfTwo {
pub fn new(conf: &'static Conf) -> Self {
Self { msrv: conf.msrv }
}

fn build_sugg(&self, cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>) {
if is_in_const_context(cx) && !self.msrv.meets(cx, msrvs::CONST_IS_POWER_OF_TWO) {
return;
}

let mut applicability = Applicability::MachineApplicable;
let snippet = Sugg::hir_with_applicability(cx, receiver, "_", &mut applicability);

span_lint_and_sugg(
cx,
MANUAL_IS_POWER_OF_TWO,
expr.span,
"manually reimplementing `is_power_of_two`",
"consider using `.is_power_of_two()`",
format!("{}.is_power_of_two()", snippet.maybe_paren()),
applicability,
);
}
}

impl<'tcx> LateLintPass<'tcx> for ManualIsPowerOfTwo {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
Expand All @@ -41,39 +72,24 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsPowerOfTwo {
if let Some(a) = count_ones_receiver(cx, lhs)
&& is_integer_literal(rhs, 1)
{
build_sugg(cx, expr, a);
self.build_sugg(cx, expr, a);
} else if let Some(a) = count_ones_receiver(cx, rhs)
&& is_integer_literal(lhs, 1)
{
build_sugg(cx, expr, a);
self.build_sugg(cx, expr, a);
} else if is_integer_literal(rhs, 0)
&& let Some(a) = is_and_minus_one(cx, lhs)
{
build_sugg(cx, expr, a);
self.build_sugg(cx, expr, a);
} else if is_integer_literal(lhs, 0)
&& let Some(a) = is_and_minus_one(cx, rhs)
{
build_sugg(cx, expr, a);
self.build_sugg(cx, expr, a);
}
}
}
}

fn build_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>) {
let mut applicability = Applicability::MachineApplicable;
let snippet = Sugg::hir_with_applicability(cx, receiver, "_", &mut applicability);

span_lint_and_sugg(
cx,
MANUAL_IS_POWER_OF_TWO,
expr.span,
"manually reimplementing `is_power_of_two`",
"consider using `.is_power_of_two()`",
format!("{}.is_power_of_two()", snippet.maybe_paren()),
applicability,
);
}

/// Return the unsigned integer receiver of `.count_ones()`
fn count_ones_receiver<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if let ExprKind::MethodCall(method_name, receiver, [], _) = expr.kind
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also check for <int>::count_ones(x), it's a normal ExprKind::Call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Done, and tested, in a new commit on top of the others.

Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ msrv_aliases! {
1,35,0 { OPTION_COPIED, RANGE_CONTAINS }
1,34,0 { TRY_FROM }
1,33,0 { UNDERSCORE_IMPORTS }
1,32,0 { CONST_IS_POWER_OF_TWO }
1,31,0 { OPTION_REPLACE }
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
1,29,0 { ITER_FLATTEN }
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/manual_is_power_of_two.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,14 @@ fn main() {
let _ = binop!(a, and, a - 1) == 0;
let _ = a & binop!(a, minus, 1) == 0;
}

#[clippy::msrv = "1.31"]
const fn low_msrv(a: u32) -> bool {
a & (a - 1) == 0
}

#[clippy::msrv = "1.32"]
const fn high_msrv(a: u32) -> bool {
a.is_power_of_two()
//~^ manual_is_power_of_two
}
11 changes: 11 additions & 0 deletions tests/ui/manual_is_power_of_two.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,14 @@ fn main() {
let _ = binop!(a, and, a - 1) == 0;
let _ = a & binop!(a, minus, 1) == 0;
}

#[clippy::msrv = "1.31"]
const fn low_msrv(a: u32) -> bool {
a & (a - 1) == 0
}

#[clippy::msrv = "1.32"]
const fn high_msrv(a: u32) -> bool {
a & (a - 1) == 0
//~^ manual_is_power_of_two
}
8 changes: 7 additions & 1 deletion tests/ui/manual_is_power_of_two.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,11 @@ error: manually reimplementing `is_power_of_two`
LL | let _ = i as u32 & (i as u32 - 1) == 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `(i as u32).is_power_of_two()`

error: aborting due to 7 previous errors
error: manually reimplementing `is_power_of_two`
--> tests/ui/manual_is_power_of_two.rs:56:5
|
LL | a & (a - 1) == 0
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`

error: aborting due to 8 previous errors