Add new lint [] init

benacq · benacq · commit 98b16669bd46 · 2025-02-16T22:58:44.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5742,6 +5742,7 @@ Released 2018-09-13
 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
 [`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_checked_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_checked_sub
 [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
 [`manual_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains
 [`manual_div_ceil`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_div_ceil
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -312,6 +312,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::manual_assert::MANUAL_ASSERT_INFO,
     crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
     crate::manual_bits::MANUAL_BITS_INFO,
+    crate::manual_checked_sub::MANUAL_CHECKED_SUB_INFO,
     crate::manual_clamp::MANUAL_CLAMP_INFO,
     crate::manual_div_ceil::MANUAL_DIV_CEIL_INFO,
     crate::manual_float_methods::MANUAL_IS_FINITE_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -207,6 +207,7 @@ mod main_recursion;
 mod manual_assert;
 mod manual_async_fn;
 mod manual_bits;
+mod manual_checked_sub;
 mod manual_clamp;
 mod manual_div_ceil;
 mod manual_float_methods;
@@ -980,5 +981,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf)));
     store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
     store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
+    store.register_late_pass(move |_| Box::new(manual_checked_sub::ManualCheckedSub::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
diff --git a/clippy_lints/src/manual_checked_sub.rs b/clippy_lints/src/manual_checked_sub.rs
@@ -0,0 +1,219 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::snippet_with_applicability;
+use rustc_ast::{BinOpKind, LitIntType, LitKind};
+use rustc_data_structures::packed::Pu128;
+use rustc_errors::Applicability;
+use rustc_hir::def::Res;
+use rustc_hir::intravisit::{Visitor, walk_expr};
+use rustc_hir::{Expr, ExprKind, QPath};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{self};
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual re-implementations of checked subtraction.
+    ///
+    /// ### Why is this bad?
+    /// Manually re-implementing checked subtraction can be error-prone and less readable.
+    /// Using the standard library method `.checked_sub()` is clearer and less likely to contain bugs.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// if a >= b {
+    ///     a - b
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// a.checked_sub(b)
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub MANUAL_CHECKED_SUB,
+    complexity,
+    "default lint description"
+}
+
+pub struct ManualCheckedSub {
+    msrv: Msrv,
+}
+
+impl ManualCheckedSub {
+    #[must_use]
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            msrv: conf.msrv.clone(),
+        }
+    }
+}
+
+impl_lint_pass!(ManualCheckedSub => [MANUAL_CHECKED_SUB]);
+
+impl<'tcx> LateLintPass<'tcx> for ManualCheckedSub {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if !self.msrv.meets(msrvs::MANUAL_CHECKED_SUB) {
+            return;
+        }
+
+        let applicability = Applicability::MachineApplicable;
+
+        if let ExprKind::If(drop_temp_expr, body_block, _) = expr.kind
+            && let ExprKind::DropTemps(condition_expr) = drop_temp_expr.kind
+            && let ExprKind::Binary(op, lhs, rhs) = condition_expr.kind
+            && is_unsigned_int(cx, &lhs)
+            && is_unsigned_int(cx, &rhs)
+        {
+            if let BinOpKind::Ge = op.node {
+                SubExprVisitor {
+                    cx,
+                    applicability,
+                    condition_lhs: &lhs,
+                    condition_rhs: &rhs,
+                }
+                .visit_expr(body_block)
+            }
+
+            if let BinOpKind::Gt = op.node {
+                if let ExprKind::Lit(lit) = &rhs.kind
+                    && let LitKind::Int(Pu128(0), _) = lit.node
+                {
+                    SubExprVisitor {
+                        cx,
+                        applicability,
+                        condition_lhs: &lhs,
+                        condition_rhs: &rhs,
+                    }
+                    .visit_expr(body_block)
+                }
+            }
+        }
+    }
+}
+
+struct SubExprVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    applicability: Applicability,
+    condition_lhs: &'tcx Expr<'tcx>,
+    condition_rhs: &'tcx Expr<'tcx>,
+}
+
+impl<'tcx> Visitor<'tcx> for SubExprVisitor<'_, 'tcx> {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+        if let ExprKind::Binary(op, sub_lhs, sub_rhs) = expr.kind
+            && let BinOpKind::Sub = op.node
+        {
+            if let ExprKind::Lit(lit) = self.condition_rhs.kind
+                && let LitKind::Int(Pu128(0), _) = lit.node
+                && (is_referencing_same_variable(&sub_lhs, self.condition_lhs)
+                    || are_literals_equal(&sub_lhs, self.condition_lhs))
+            {
+                self.build_suggession(expr);
+            } else {
+                if (is_referencing_same_variable(&sub_lhs, self.condition_lhs)
+                    || are_literals_equal(&sub_lhs, self.condition_lhs))
+                    && (is_referencing_same_variable(&sub_rhs, self.condition_rhs)
+                        || are_literals_equal(&sub_rhs, self.condition_rhs))
+                {
+                    self.build_suggession(expr);
+                }
+            }
+        }
+
+        walk_expr(self, expr);
+    }
+}
+
+impl<'a, 'tcx> SubExprVisitor<'a, 'tcx> {
+    fn build_suggession(&mut self, expr: &Expr<'tcx>) {
+        let lhs_snippet = snippet_with_applicability(self.cx, self.condition_lhs.span, "..", &mut self.applicability);
+        let rhs_snippet = snippet_with_applicability(self.cx, self.condition_rhs.span, "..", &mut self.applicability);
+
+        let lhs_needs_parens = matches!(self.condition_lhs.kind, ExprKind::Cast(..));
+
+        let suggestion = if lhs_needs_parens {
+            format!("({}).checked_sub({})", lhs_snippet, rhs_snippet)
+        } else {
+            format!("{}.checked_sub({})", lhs_snippet, rhs_snippet)
+        };
+
+        span_lint_and_sugg(
+            self.cx,
+            MANUAL_CHECKED_SUB,
+            expr.span,
+            "manual re-implementation of checked subtraction",
+            "consider using `.checked_sub()`",
+            suggestion,
+            self.applicability,
+        );
+    }
+}
+
+fn is_unsigned_int<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> bool {
+    let expr_type = cx.typeck_results().expr_ty(expr).peel_refs();
+    if matches!(expr_type.kind(), ty::Uint(_)) {
+        return true;
+    }
+
+    if let ExprKind::Lit(lit) = &expr.kind {
+        if let LitKind::Int(_, suffix) = &lit.node {
+            if let LitIntType::Unsuffixed = suffix {
+                return true;
+            }
+        }
+    }
+    false
+}
+
+fn are_literals_equal<'tcx>(expr1: &Expr<'tcx>, expr2: &Expr<'tcx>) -> bool {
+    if let (ExprKind::Lit(lit1), ExprKind::Lit(lit2)) = (&expr1.kind, &expr2.kind) {
+        if let (LitKind::Int(val1, suffix1), LitKind::Int(val2, suffix2)) = (&lit1.node, &lit2.node) {
+            return val1 == val2
+                && suffix1 == suffix2
+                && *suffix1 != LitIntType::Unsuffixed
+                && *suffix2 != LitIntType::Unsuffixed;
+        }
+    }
+    false
+}
+fn is_referencing_same_variable<'tcx>(expr1: &'tcx Expr<'_>, expr2: &'tcx Expr<'_>) -> bool {
+    if let ExprKind::Cast(cast_expr1, _) = &expr1.kind {
+        if let Some(cast_expr1_path) = get_resolved_path_id(cast_expr1) {
+            if let ExprKind::Cast(cast_expr2, _) = &expr2.kind {
+                if let Some(cast_expr2_path) = get_resolved_path_id(cast_expr2) {
+                    return cast_expr1_path == cast_expr2_path;
+                }
+            } else {
+                if let Some(expr2_path) = get_resolved_path_id(expr2) {
+                    return cast_expr1_path == expr2_path;
+                };
+                return false;
+            }
+        }
+    } else if let ExprKind::Cast(cast_expr2, _) = &expr2.kind {
+        if let Some(cast_expr2_path) = get_resolved_path_id(cast_expr2) {
+            if let Some(expr1_path) = get_resolved_path_id(expr1) {
+                return expr1_path == cast_expr2_path;
+            };
+            return false;
+        }
+    }
+
+    let expr1_path = get_resolved_path_id(expr1);
+    let expr2_path = get_resolved_path_id(expr2);
+
+    if let (Some(expr1_path), Some(expr2_path)) = (expr1_path, expr2_path) {
+        expr1_path == expr2_path
+    } else {
+        false
+    }
+}
+
+fn get_resolved_path_id<'tcx>(expr: &'tcx Expr<'_>) -> Option<Res> {
+    if let ExprKind::Path(QPath::Resolved(_, path)) = &expr.kind {
+        path.segments.first().map(|segment| segment.res)
+    } else {
+        None
+    }
+}
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
@@ -45,7 +45,7 @@ msrv_aliases! {
     1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
     1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS }
     1,50,0 { BOOL_THEN, CLAMP, SLICE_FILL }
-    1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN, SATURATING_SUB_CONST }
+    1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN, SATURATING_SUB_CONST, MANUAL_CHECKED_SUB }
     1,46,0 { CONST_IF_MATCH }
     1,45,0 { STR_STRIP_PREFIX }
     1,43,0 { LOG2_10, LOG10_2, NUMERIC_ASSOCIATED_CONSTANTS }
diff --git a/tests/ui/manual_checked_sub.fixed b/tests/ui/manual_checked_sub.fixed
@@ -0,0 +1,102 @@
+#![allow(clippy::collapsible_if)]
+#![warn(clippy::manual_checked_sub)]
+
+//Positive test, lint
+fn positive_tests() {
+    let a: u32 = 10;
+    let b: u32 = 5;
+
+    // Case 1: a - b inside an if a >= b
+    if a >= b {
+        let _ = a.checked_sub(b);
+    }
+
+    // Case 2: a - 1 inside an if a > 0
+    if a > 0 {
+        let _ = a.checked_sub(0);
+    }
+
+    // Case 3: Both suffixed, should lint
+    if 10u32 >= 5u32 {
+        let _ = 10u32.checked_sub(5u32);
+    }
+
+    let x: i32 = 10;
+    let y: i32 = 5;
+
+    // Case 4: casted variables inside an if, should lint
+    if x as u32 >= y as u32 {
+        let _ = (x as u32).checked_sub(y as u32);
+    }
+
+    // Case 5: casted variable and literal inside an if, should lint
+    if x as u32 >= 5u32 {
+        let _ = (x as u32).checked_sub(5u32);
+    }
+}
+
+// Negative tests, no lint
+fn negative_tests() {
+    let a: u32 = 10;
+    let b: u32 = 5;
+
+    // Case 1: Uses checked_sub() correctly, no lint
+    let _ = a.checked_sub(b);
+
+    // Case 2: Works with signed integers (i32), no lint
+    let x: i32 = 10;
+    let y: i32 = 5;
+    if x >= y {
+        let _ = x - y;
+    }
+
+    // Case 3: If condition doesn't match, no lint
+    if a == b {
+        let _ = a - b;
+    }
+
+    // Case 4: a - b inside an if a > b
+    if a > b {
+        let _ = a - b;
+    }
+
+    // Case 5: Unsuffixed literals, no lint
+    if 10 >= 5 {
+        let _ = 10 - 5;
+    }
+
+    // Case 6: Suffixed lhs, unsuffixed rhs, no lint
+    if 10u32 >= 5 {
+        let _ = 10u32 - 5;
+    }
+
+    // Case 7: Unsuffixed lhs, suffixed rhs, no lint
+    if 10 >= 5u32 {
+        let _ = 10 - 5u32;
+    }
+}
+
+fn edge_cases() {
+    let a: u32 = 10;
+    let b: u32 = 5;
+    let c = 3;
+
+    // Edge Case 1: Subtraction inside a nested if
+    if a >= b {
+        if c > 0 {
+            let _ = a.checked_sub(b);
+        }
+    }
+
+    if a > b {
+        if a - b > c {
+            let _ = a - b; // This subtraction is safe because `a > b` is checked
+        }
+    }
+}
+
+fn main() {
+    positive_tests();
+    negative_tests();
+    edge_cases();
+}
diff --git a/tests/ui/manual_checked_sub.rs b/tests/ui/manual_checked_sub.rs
diff --git a/tests/ui/manual_checked_sub.stderr b/tests/ui/manual_checked_sub.stderr
