diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 375d179681da..9d2532f8e47f 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -298,7 +298,6 @@ 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, @@ -592,6 +591,7 @@ crate::operators::IMPOSSIBLE_COMPARISONS_INFO, crate::operators::INEFFECTIVE_BIT_MASK_INFO, crate::operators::INTEGER_DIVISION_INFO, + crate::operators::MANUAL_DIV_CEIL_INFO, crate::operators::MANUAL_IS_MULTIPLE_OF_INFO, crate::operators::MANUAL_MIDPOINT_INFO, crate::operators::MISREFACTORED_ASSIGN_OP_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a4ad9424b3eb..96077d0bd00f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -203,7 +203,6 @@ mod manual_async_fn; mod manual_bits; mod manual_clamp; -mod manual_div_ceil; mod manual_float_methods; mod manual_hash_one; mod manual_ignore_case_cmp; @@ -807,7 +806,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co 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))); 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)); diff --git a/clippy_lints/src/manual_div_ceil.rs b/clippy_lints/src/operators/manual_div_ceil.rs similarity index 50% rename from clippy_lints/src/manual_div_ceil.rs rename to clippy_lints/src/operators/manual_div_ceil.rs index ee531741a515..98aa47421537 100644 --- a/clippy_lints/src/manual_div_ceil.rs +++ b/clippy_lints/src/operators/manual_div_ceil.rs @@ -1,4 +1,3 @@ -use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::sugg::{Sugg, has_enclosing_paren}; @@ -7,111 +6,69 @@ use rustc_data_structures::packed::Pu128; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::LateContext; use rustc_middle::ty::{self}; -use rustc_session::impl_lint_pass; use rustc_span::source_map::Spanned; -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.83.0"] - pub MANUAL_DIV_CEIL, - complexity, - "manually reimplementing `div_ceil`" -} +use super::MANUAL_DIV_CEIL; -pub struct ManualDivCeil { - msrv: Msrv, -} +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, op: BinOpKind, lhs: &Expr<'_>, rhs: &Expr<'_>, msrv: Msrv) { + let mut applicability = Applicability::MachineApplicable; -impl ManualDivCeil { - #[must_use] - pub fn new(conf: &'static Conf) -> Self { - Self { msrv: conf.msrv } - } -} - -impl_lint_pass!(ManualDivCeil => [MANUAL_DIV_CEIL]); - -impl<'tcx> LateLintPass<'tcx> for ManualDivCeil { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) { - 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 - && self.msrv.meets(cx, msrvs::DIV_CEIL) + if op == BinOpKind::Div + && check_int_ty_and_feature(cx, lhs) + && check_int_ty_and_feature(cx, rhs) + && let ExprKind::Binary(inner_op, inner_lhs, inner_rhs) = lhs.kind + && msrv.meets(cx, msrvs::DIV_CEIL) + { + // (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, rhs) { - // (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; - } + build_suggestion(cx, expr, inner_lhs, 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; - } + // ((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, rhs) + { + build_suggestion(cx, expr, inner_rhs, 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); - } + // (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, rhs) + { + build_suggestion(cx, expr, add_lhs, rhs, &mut applicability); + } - // (x + (Y - 1)) / Y - if inner_op.node == BinOpKind::Add && differ_by_one(inner_rhs, div_rhs) { - build_suggestion(cx, expr, inner_lhs, div_rhs, &mut applicability); - } + // (x + (Y - 1)) / Y + if inner_op.node == BinOpKind::Add && differ_by_one(inner_rhs, rhs) { + build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability); + } - // ((Y - 1) + x) / Y - if inner_op.node == BinOpKind::Add && differ_by_one(inner_lhs, div_rhs) { - build_suggestion(cx, expr, inner_rhs, div_rhs, &mut applicability); - } + // ((Y - 1) + x) / Y + if inner_op.node == BinOpKind::Add && differ_by_one(inner_lhs, rhs) { + build_suggestion(cx, expr, inner_rhs, rhs, &mut applicability); + } - // (x - (-Y - 1)) / Y - if inner_op.node == BinOpKind::Sub - && let ExprKind::Unary(UnOp::Neg, abs_div_rhs) = div_rhs.kind - && differ_by_one(abs_div_rhs, inner_rhs) - { - build_suggestion(cx, expr, inner_lhs, div_rhs, &mut applicability); - } + // (x - (-Y - 1)) / Y + if inner_op.node == BinOpKind::Sub + && let ExprKind::Unary(UnOp::Neg, abs_div_rhs) = rhs.kind + && differ_by_one(abs_div_rhs, inner_rhs) + { + build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability); } } } diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index aaea4ff11fc3..f2aae8ca458d 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -11,6 +11,7 @@ mod float_equality_without_abs; mod identity_op; mod integer_division; +mod manual_div_ceil; mod manual_is_multiple_of; mod manual_midpoint; mod misrefactored_assign_op; @@ -860,6 +861,33 @@ "manual implementation of `.is_multiple_of()`" } +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.83.0"] + pub MANUAL_DIV_CEIL, + complexity, + "manually reimplementing `div_ceil`" +} + pub struct Operators { arithmetic_context: numeric_arithmetic::Context, verbose_bit_mask_threshold: u64, @@ -906,6 +934,7 @@ pub fn new(conf: &'static Conf) -> Self { SELF_ASSIGNMENT, MANUAL_MIDPOINT, MANUAL_IS_MULTIPLE_OF, + MANUAL_DIV_CEIL, ]); impl<'tcx> LateLintPass<'tcx> for Operators { @@ -944,6 +973,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { rhs, self.modulo_arithmetic_allow_comparison_to_zero, ); + manual_div_ceil::check(cx, e, op.node, lhs, rhs, self.msrv); }, ExprKind::AssignOp(op, lhs, rhs) => { let bin_op = op.node.into();