Auto merge of #12987 - belyakov-am:lint/manual_div_ceil, r=blyxyas

[`manual_div_ceil`]: init

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

Partially fixes #12894

changelog: new lint: [`manual_div_ceil`]

CHANGELOG.md

@@ -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

clippy_config/src/conf.rs

@@ -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| {

clippy_config/src/msrvs.rs

@@ -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 }

clippy_lints/src/declared_lints.rs

@@ -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,

clippy_lints/src/lib.rs

@@ -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;
@@ -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`
 }

clippy_lints/src/manual_div_ceil.rs

@@ -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,
+    );
+}

tests/ui/manual_div_ceil.fixed

@@ -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;
+}

tests/ui/manual_div_ceil.rs

@@ -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;
+}

tests/ui/manual_div_ceil.stderr

@@ -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
+

tests/ui/manual_div_ceil_with_feature.fixed

@@ -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;
+}

tests/ui/manual_div_ceil_with_feature.rs

@@ -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;
+}