Skip to content

Commit

Permalink
Auto merge of #12987 - belyakov-am:lint/manual_div_ceil, r=blyxyas
Browse files Browse the repository at this point in the history
[`manual_div_ceil`]: init

Suggest using `div_ceil()` instead of manual implementation for basic cases.

Partially fixes #12894

changelog: new lint: [`manual_div_ceil`]
  • Loading branch information
bors committed Sep 5, 2024
2 parents c41be9e + 9415e6e commit 9e9042a
Show file tree
Hide file tree
Showing 12 changed files with 356 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5614,6 +5614,7 @@ Released 2018-09-13
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
[`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
[`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
[`manual_div_ceil`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_div_ceil
[`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
[`manual_find`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ fn calculate_dimensions(fields: &[&str]) -> (usize, Vec<usize>) {
cmp::max(1, terminal_width / (SEPARATOR_WIDTH + max_field_width))
});

let rows = (fields.len() + (columns - 1)) / columns;
let rows = fields.len().div_ceil(columns);

let column_widths = (0..columns)
.map(|column| {
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ msrv_aliases! {
1,80,0 { BOX_INTO_ITER}
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
1,73,0 { MANUAL_DIV_CEIL }
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 }
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
crate::manual_bits::MANUAL_BITS_INFO,
crate::manual_clamp::MANUAL_CLAMP_INFO,
crate::manual_div_ceil::MANUAL_DIV_CEIL_INFO,
crate::manual_float_methods::MANUAL_IS_FINITE_INFO,
crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ mod manual_assert;
mod manual_async_fn;
mod manual_bits;
mod manual_clamp;
mod manual_div_ceil;
mod manual_float_methods;
mod manual_hash_one;
mod manual_is_ascii_check;
Expand Down Expand Up @@ -936,5 +937,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_early_pass(|| Box::new(cfg_not_test::CfgNotTest));
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)));
// add lints here, do not remove this comment, it's used in `new_lint`
}
158 changes: 158 additions & 0 deletions clippy_lints/src/manual_div_ceil.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::sugg::Sugg;
use clippy_utils::SpanlessEq;
use rustc_ast::{BinOpKind, LitKind};
use rustc_data_structures::packed::Pu128;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self};
use rustc_session::impl_lint_pass;
use rustc_span::symbol::Symbol;

use clippy_config::Conf;

declare_clippy_lint! {
/// ### What it does
/// Checks for an expression like `(x + (y - 1)) / y` which is a common manual reimplementation
/// of `x.div_ceil(y)`.
///
/// ### Why is this bad?
/// It's simpler, clearer and more readable.
///
/// ### Example
/// ```no_run
/// let x: i32 = 7;
/// let y: i32 = 4;
/// let div = (x + (y - 1)) / y;
/// ```
/// Use instead:
/// ```no_run
/// #![feature(int_roundings)]
/// let x: i32 = 7;
/// let y: i32 = 4;
/// let div = x.div_ceil(y);
/// ```
#[clippy::version = "1.81.0"]
pub MANUAL_DIV_CEIL,
complexity,
"manually reimplementing `div_ceil`"
}

pub struct ManualDivCeil {
msrv: Msrv,
}

impl ManualDivCeil {
#[must_use]
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}

impl_lint_pass!(ManualDivCeil => [MANUAL_DIV_CEIL]);

impl<'tcx> LateLintPass<'tcx> for ManualDivCeil {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
if !self.msrv.meets(msrvs::MANUAL_DIV_CEIL) {
return;
}

let mut applicability = Applicability::MachineApplicable;

if let ExprKind::Binary(div_op, div_lhs, div_rhs) = expr.kind
&& div_op.node == BinOpKind::Div
&& check_int_ty_and_feature(cx, div_lhs)
&& check_int_ty_and_feature(cx, div_rhs)
&& let ExprKind::Binary(inner_op, inner_lhs, inner_rhs) = div_lhs.kind
{
// (x + (y - 1)) / y
if let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = inner_rhs.kind
&& inner_op.node == BinOpKind::Add
&& sub_op.node == BinOpKind::Sub
&& check_literal(sub_rhs)
&& check_eq_expr(cx, sub_lhs, div_rhs)
{
build_suggestion(cx, expr, inner_lhs, div_rhs, &mut applicability);
return;
}

// ((y - 1) + x) / y
if let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = inner_lhs.kind
&& inner_op.node == BinOpKind::Add
&& sub_op.node == BinOpKind::Sub
&& check_literal(sub_rhs)
&& check_eq_expr(cx, sub_lhs, div_rhs)
{
build_suggestion(cx, expr, inner_rhs, div_rhs, &mut applicability);
return;
}

// (x + y - 1) / y
if let ExprKind::Binary(add_op, add_lhs, add_rhs) = inner_lhs.kind
&& inner_op.node == BinOpKind::Sub
&& add_op.node == BinOpKind::Add
&& check_literal(inner_rhs)
&& check_eq_expr(cx, add_rhs, div_rhs)
{
build_suggestion(cx, expr, add_lhs, div_rhs, &mut applicability);
}
}
}

extract_msrv_attr!(LateContext);
}

fn check_int_ty_and_feature(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let expr_ty = cx.typeck_results().expr_ty(expr);
match expr_ty.peel_refs().kind() {
ty::Uint(_) => true,
ty::Int(_) => cx
.tcx
.features()
.declared_features
.contains(&Symbol::intern("int_roundings")),

_ => false,
}
}

fn check_literal(expr: &Expr<'_>) -> bool {
if let ExprKind::Lit(lit) = expr.kind
&& let LitKind::Int(Pu128(1), _) = lit.node
{
return true;
}
false
}

fn check_eq_expr(cx: &LateContext<'_>, lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool {
SpanlessEq::new(cx).eq_expr(lhs, rhs)
}

fn build_suggestion(
cx: &LateContext<'_>,
expr: &Expr<'_>,
lhs: &Expr<'_>,
rhs: &Expr<'_>,
applicability: &mut Applicability,
) {
let dividend_sugg = Sugg::hir_with_applicability(cx, lhs, "..", applicability).maybe_par();
let divisor_snippet = snippet_with_applicability(cx, rhs.span.source_callsite(), "..", applicability);

let sugg = format!("{dividend_sugg}.div_ceil({divisor_snippet})");

span_lint_and_sugg(
cx,
MANUAL_DIV_CEIL,
expr.span,
"manually reimplementing `div_ceil`",
"consider using `.div_ceil()`",
sugg,
*applicability,
);
}
30 changes: 30 additions & 0 deletions tests/ui/manual_div_ceil.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![warn(clippy::manual_div_ceil)]

fn main() {
let x = 7_u32;
let y = 4_u32;
let z = 11_u32;

// Lint
let _ = x.div_ceil(y); //~ ERROR: manually reimplementing `div_ceil`
let _ = x.div_ceil(y); //~ ERROR: manually reimplementing `div_ceil`
let _ = x.div_ceil(y); //~ ERROR: manually reimplementing `div_ceil`

let _ = 7_u32.div_ceil(4); //~ ERROR: manually reimplementing `div_ceil`
let _ = (7_i32 as u32).div_ceil(4); //~ ERROR: manually reimplementing `div_ceil`

// No lint
let _ = (x + (y - 2)) / y;
let _ = (x + (y + 1)) / y;

let _ = (x + (y - 1)) / z;

let x_i = 7_i32;
let y_i = 4_i32;
let z_i = 11_i32;

// No lint because `int_roundings` feature is not enabled.
let _ = (z as i32 + (y_i - 1)) / y_i;
let _ = (7_u32 as i32 + (y_i - 1)) / y_i;
let _ = (7_u32 as i32 + (4 - 1)) / 4;
}
30 changes: 30 additions & 0 deletions tests/ui/manual_div_ceil.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![warn(clippy::manual_div_ceil)]

fn main() {
let x = 7_u32;
let y = 4_u32;
let z = 11_u32;

// Lint
let _ = (x + (y - 1)) / y; //~ ERROR: manually reimplementing `div_ceil`
let _ = ((y - 1) + x) / y; //~ ERROR: manually reimplementing `div_ceil`
let _ = (x + y - 1) / y; //~ ERROR: manually reimplementing `div_ceil`

let _ = (7_u32 + (4 - 1)) / 4; //~ ERROR: manually reimplementing `div_ceil`
let _ = (7_i32 as u32 + (4 - 1)) / 4; //~ ERROR: manually reimplementing `div_ceil`

// No lint
let _ = (x + (y - 2)) / y;
let _ = (x + (y + 1)) / y;

let _ = (x + (y - 1)) / z;

let x_i = 7_i32;
let y_i = 4_i32;
let z_i = 11_i32;

// No lint because `int_roundings` feature is not enabled.
let _ = (z as i32 + (y_i - 1)) / y_i;
let _ = (7_u32 as i32 + (y_i - 1)) / y_i;
let _ = (7_u32 as i32 + (4 - 1)) / 4;
}
35 changes: 35 additions & 0 deletions tests/ui/manual_div_ceil.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil.rs:9:13
|
LL | let _ = (x + (y - 1)) / y;
| ^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(y)`
|
= note: `-D clippy::manual-div-ceil` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_div_ceil)]`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil.rs:10:13
|
LL | let _ = ((y - 1) + x) / y;
| ^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(y)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil.rs:11:13
|
LL | let _ = (x + y - 1) / y;
| ^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(y)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil.rs:13:13
|
LL | let _ = (7_u32 + (4 - 1)) / 4;
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `7_u32.div_ceil(4)`

error: manually reimplementing `div_ceil`
--> tests/ui/manual_div_ceil.rs:14:13
|
LL | let _ = (7_i32 as u32 + (4 - 1)) / 4;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `(7_i32 as u32).div_ceil(4)`

error: aborting due to 5 previous errors

25 changes: 25 additions & 0 deletions tests/ui/manual_div_ceil_with_feature.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![warn(clippy::manual_div_ceil)]
#![feature(int_roundings)]

fn main() {
let x = 7_i32;
let y = 4_i32;
let z = 3_i32;
let z_u: u32 = 11;

// Lint.
let _ = x.div_ceil(y);
let _ = x.div_ceil(y);
let _ = x.div_ceil(y);

let _ = 7_i32.div_ceil(4);
let _ = (7_i32 as u32).div_ceil(4);
let _ = (7_u32 as i32).div_ceil(4);
let _ = z_u.div_ceil(4);

// No lint.
let _ = (x + (y - 2)) / y;
let _ = (x + (y + 1)) / y;

let _ = (x + (y - 1)) / z;
}
25 changes: 25 additions & 0 deletions tests/ui/manual_div_ceil_with_feature.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![warn(clippy::manual_div_ceil)]
#![feature(int_roundings)]

fn main() {
let x = 7_i32;
let y = 4_i32;
let z = 3_i32;
let z_u: u32 = 11;

// Lint.
let _ = (x + (y - 1)) / y;
let _ = ((y - 1) + x) / y;
let _ = (x + y - 1) / y;

let _ = (7_i32 + (4 - 1)) / 4;
let _ = (7_i32 as u32 + (4 - 1)) / 4;
let _ = (7_u32 as i32 + (4 - 1)) / 4;
let _ = (z_u + (4 - 1)) / 4;

// No lint.
let _ = (x + (y - 2)) / y;
let _ = (x + (y + 1)) / y;

let _ = (x + (y - 1)) / z;
}
Loading

0 comments on commit 9e9042a

Please sign in to comment.