Add manual_div_ceil lint

Author: belyakov-am

CHANGELOG.md

@@ -5515,6 +5515,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_lints/src/declared_lints.rs

@@ -303,6 +303,7 @@ pub(crate) 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

@@ -202,6 +202,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;
@@ -1171,6 +1172,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     });
     store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv())));
     store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
+    store.register_late_pass(|_| Box::new(manual_div_ceil::ManualDivCeil));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/manual_div_ceil.rs

@@ -0,0 +1,132 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::SpanlessEq;
+use rustc_ast::{BinOpKind, LitKind};
+use rustc_data_structures::packed::Pu128;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, QPath};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{self};
+use rustc_session::declare_lint_pass;
+
+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 and more readable.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let x: i32 = 7;
+    /// ley y: i32 = 4;
+    /// let div = (x + (y - 1)) / y;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// 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`"
+}
+
+declare_lint_pass!(ManualDivCeil => [MANUAL_DIV_CEIL]);
+
+impl LateLintPass<'_> for ManualDivCeil {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        let mut applicability = Applicability::MachineApplicable;
+
+        if let ExprKind::Binary(div_op, div_lhs, div_rhs) = expr.kind
+            && check_int_ty(cx, div_lhs)
+            && check_int_ty(cx, div_rhs)
+            && div_op.node == BinOpKind::Div
+        {
+            // (x + (y - 1)) / y
+            if let ExprKind::Binary(add_op, add_lhs, add_rhs) = div_lhs.kind
+                && add_op.node == BinOpKind::Add
+                && let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = add_rhs.kind
+                && sub_op.node == BinOpKind::Sub
+                && check_literal(sub_rhs)
+                && check_eq_path_segment(cx, sub_lhs, div_rhs)
+            {
+                build_suggestion(cx, expr, add_lhs, div_rhs, &mut applicability);
+                return;
+            }
+
+            // ((y - 1) + x) / y
+            if let ExprKind::Binary(add_op, add_lhs, add_rhs) = div_lhs.kind
+                && add_op.node == BinOpKind::Add
+                && let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = add_lhs.kind
+                && sub_op.node == BinOpKind::Sub
+                && check_literal(sub_rhs)
+                && check_eq_path_segment(cx, sub_lhs, div_rhs)
+            {
+                build_suggestion(cx, expr, add_rhs, div_rhs, &mut applicability);
+                return;
+            }
+
+            // (x + y - 1) / y
+            if let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = div_lhs.kind
+                && sub_op.node == BinOpKind::Sub
+                && let ExprKind::Binary(add_op, add_lhs, add_rhs) = sub_lhs.kind
+                && add_op.node == BinOpKind::Add
+                && check_literal(sub_rhs)
+                && check_eq_path_segment(cx, add_rhs, div_rhs)
+            {
+                build_suggestion(cx, expr, add_lhs, div_rhs, &mut applicability);
+            }
+        }
+    }
+}
+
+fn check_int_ty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let expr_ty = cx.typeck_results().expr_ty(expr);
+    matches!(expr_ty.peel_refs().kind(), ty::Int(_) | ty::Uint(_))
+}
+
+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_path_segment(cx: &LateContext<'_>, lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool {
+    if let ExprKind::Path(QPath::Resolved(_, lhs_path)) = lhs.kind
+        && let ExprKind::Path(QPath::Resolved(_, rhs_path)) = rhs.kind
+        && SpanlessEq::new(cx).eq_path_segment(&lhs_path.segments[0], &rhs_path.segments[0])
+    {
+        return true;
+    }
+    false
+}
+
+fn build_suggestion(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    lhs: &Expr<'_>,
+    rhs: &Expr<'_>,
+    applicability: &mut Applicability,
+) {
+    let dividend_snippet = snippet_with_applicability(cx, lhs.span.source_callsite(), "..", applicability);
+    let divisor_snippet = snippet_with_applicability(cx, rhs.span.source_callsite(), "..", applicability);
+
+    let sugg = format!("{dividend_snippet}.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,21 @@
+#![warn(clippy::manual_div_ceil)]
+#![feature(int_roundings)]
+
+fn main() {
+    let x: i32 = 7;
+    let y: i32 = 4;
+
+    // Lint.
+    let _ = x.div_ceil(y);
+    let _ = x.div_ceil(y);
+    let _ = x.div_ceil(y);
+
+    let _ = (7 + (4 - 1)) / 4;
+
+    // No lint.
+    let _ = (x + (y - 2)) / y;
+    let _ = (x + (y + 1)) / y;
+
+    let z: i32 = 3;
+    let _ = (x + (y - 1)) / z;
+}

tests/ui/manual_div_ceil.rs

@@ -0,0 +1,21 @@
+#![warn(clippy::manual_div_ceil)]
+#![feature(int_roundings)]
+
+fn main() {
+    let x: i32 = 7;
+    let y: i32 = 4;
+
+    // Lint.
+    let _ = (x + (y - 1)) / y;
+    let _ = ((y - 1) + x) / y;
+    let _ = (x + y - 1) / y;
+
+    let _ = (7 + (4 - 1)) / 4;
+
+    // No lint.
+    let _ = (x + (y - 2)) / y;
+    let _ = (x + (y + 1)) / y;
+
+    let z: i32 = 3;
+    let _ = (x + (y - 1)) / z;
+}

tests/ui/manual_div_ceil.stderr

@@ -0,0 +1,23 @@
+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: aborting due to 3 previous errors
+