Auto merge of rust-lang#9549 - royrustdev:manual_saturating_add, r=ll…

…ogiq new `implicit_saturating_add` lint This fixes rust-lang#9393 If you added a new lint, here's a checklist for things that will be checked during review or continuous integration. - \[x] Followed [lint naming conventions][lint_naming] - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - \[x] Added lint documentation - \[x] Run `cargo dev fmt` --- changelog: add [`manual_saturating_add`] lint
flip1995 · Oct 3, 2022 · 09e6c23 · 09e6c23
2 parents 18e10ca + f1c831a
commit 09e6c23
Show file tree

Hide file tree

Showing 11 changed files with 598 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3917,6 +3917,7 @@ Released 2018-09-13
 [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone
 [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
 [`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
+[`implicit_saturating_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_add
 [`implicit_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub
 [`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops
 [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping

diff --git a/clippy_lints/src/implicit_saturating_add.rs b/clippy_lints/src/implicit_saturating_add.rs
@@ -0,0 +1,114 @@
+use clippy_utils::consts::{constant, Constant};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::get_parent_expr;
+use clippy_utils::source::snippet_with_applicability;
+use if_chain::if_chain;
+use rustc_ast::ast::{LitIntType, LitKind};
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Stmt, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{Int, IntTy, Ty, Uint, UintTy};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for implicit saturating addition.
+    ///
+    /// ### Why is this bad?
+    /// The built-in function is more readable and may be faster.
+    ///
+    /// ### Example
+    /// ```rust
+    ///let mut u:u32 = 7000;
+    ///
+    /// if u != u32::MAX {
+    ///     u += 1;
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    ///let mut u:u32 = 7000;
+    ///
+    /// u = u.saturating_add(1);
+    /// ```
+    #[clippy::version = "1.65.0"]
+    pub IMPLICIT_SATURATING_ADD,
+    style,
+    "Perform saturating addition instead of implicitly checking max bound of data type"
+}
+declare_lint_pass!(ImplicitSaturatingAdd => [IMPLICIT_SATURATING_ADD]);
+
+impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingAdd {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if_chain! {
+            if let ExprKind::If(cond, then, None) = expr.kind;
+            if let ExprKind::DropTemps(expr1) = cond.kind;
+            if let Some((c, op_node, l)) = get_const(cx, expr1);
+            if let BinOpKind::Ne | BinOpKind::Lt = op_node;
+            if let ExprKind::Block(block, None) = then.kind;
+            if let Block {
+                stmts:
+                    [Stmt
+                        { kind: StmtKind::Expr(ex) | StmtKind::Semi(ex), .. }],
+                        expr: None, ..} |
+                        Block { stmts: [], expr: Some(ex), ..} = block;
+            if let ExprKind::AssignOp(op1, target, value) = ex.kind;
+            let ty = cx.typeck_results().expr_ty(target);
+            if Some(c) == get_int_max(ty);
+            if clippy_utils::SpanlessEq::new(cx).eq_expr(l, target);
+            if BinOpKind::Add == op1.node;
+            if let ExprKind::Lit(ref lit) = value.kind;
+            if let LitKind::Int(1, LitIntType::Unsuffixed) = lit.node;
+            if block.expr.is_none();
+            then {
+                let mut app = Applicability::MachineApplicable;
+                let code = snippet_with_applicability(cx, target.span, "_", &mut app);
+                let sugg = if let Some(parent) = get_parent_expr(cx, expr) && let ExprKind::If(_cond, _then, Some(else_)) = parent.kind && else_.hir_id == expr.hir_id {format!("{{{code} = {code}.saturating_add(1); }}")} else {format!("{code} = {code}.saturating_add(1);")};
+                span_lint_and_sugg(cx, IMPLICIT_SATURATING_ADD, expr.span, "manual saturating add detected", "use instead", sugg, app);
+            }
+        }
+    }
+}
+
+fn get_int_max(ty: Ty<'_>) -> Option<u128> {
+    match ty.peel_refs().kind() {
+        Int(IntTy::I8) => i8::max_value().try_into().ok(),
+        Int(IntTy::I16) => i16::max_value().try_into().ok(),
+        Int(IntTy::I32) => i32::max_value().try_into().ok(),
+        Int(IntTy::I64) => i64::max_value().try_into().ok(),
+        Int(IntTy::I128) => i128::max_value().try_into().ok(),
+        Int(IntTy::Isize) => isize::max_value().try_into().ok(),
+        Uint(UintTy::U8) => u8::max_value().try_into().ok(),
+        Uint(UintTy::U16) => u16::max_value().try_into().ok(),
+        Uint(UintTy::U32) => u32::max_value().try_into().ok(),
+        Uint(UintTy::U64) => u64::max_value().try_into().ok(),
+        Uint(UintTy::U128) => Some(u128::max_value()),
+        Uint(UintTy::Usize) => usize::max_value().try_into().ok(),
+        _ => None,
+    }
+}
+
+fn get_const<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<(u128, BinOpKind, &'tcx Expr<'tcx>)> {
+    if let ExprKind::Binary(op, l, r) = expr.kind {
+        let tr = cx.typeck_results();
+        if let Some((Constant::Int(c), _)) = constant(cx, tr, r) {
+            return Some((c, op.node, l));
+        };
+        if let Some((Constant::Int(c), _)) = constant(cx, tr, l) {
+            return Some((c, invert_op(op.node)?, r));
+        }
+    }
+    None
+}
+
+fn invert_op(op: BinOpKind) -> Option<BinOpKind> {
+    use rustc_hir::BinOpKind::{Ge, Gt, Le, Lt, Ne};
+    match op {
+        Lt => Some(Gt),
+        Le => Some(Ge),
+        Ne => Some(Ne),
+        Ge => Some(Le),
+        Gt => Some(Lt),
+        _ => None,
+    }
+}
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
@@ -85,6 +85,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(functions::RESULT_UNIT_ERR),
     LintId::of(functions::TOO_MANY_ARGUMENTS),
     LintId::of(if_let_mutex::IF_LET_MUTEX),
+    LintId::of(implicit_saturating_add::IMPLICIT_SATURATING_ADD),
     LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
     LintId::of(infinite_iter::INFINITE_ITER),
     LintId::of(inherent_to_string::INHERENT_TO_STRING),

diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
@@ -184,6 +184,7 @@ store.register_lints(&[
     if_then_some_else_none::IF_THEN_SOME_ELSE_NONE,
     implicit_hasher::IMPLICIT_HASHER,
     implicit_return::IMPLICIT_RETURN,
+    implicit_saturating_add::IMPLICIT_SATURATING_ADD,
     implicit_saturating_sub::IMPLICIT_SATURATING_SUB,
     inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR,
     index_refutable_slice::INDEX_REFUTABLE_SLICE,

diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs
@@ -29,6 +29,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(functions::DOUBLE_MUST_USE),
     LintId::of(functions::MUST_USE_UNIT),
     LintId::of(functions::RESULT_UNIT_ERR),
+    LintId::of(implicit_saturating_add::IMPLICIT_SATURATING_ADD),
     LintId::of(inherent_to_string::INHERENT_TO_STRING),
     LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS),
     LintId::of(len_zero::COMPARISON_TO_EMPTY),

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -238,6 +238,7 @@ mod if_not_else;
 mod if_then_some_else_none;
 mod implicit_hasher;
 mod implicit_return;
+mod implicit_saturating_add;
 mod implicit_saturating_sub;
 mod inconsistent_struct_constructor;
 mod index_refutable_slice;
@@ -904,6 +905,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| Box::new(multi_assignments::MultiAssignments));
     store.register_late_pass(|_| Box::new(bool_to_int_with_if::BoolToIntWithIf));
     store.register_late_pass(|_| Box::new(box_default::BoxDefault));
+    store.register_late_pass(|_| Box::new(implicit_saturating_add::ImplicitSaturatingAdd));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

diff --git a/src/docs.rs b/src/docs.rs
@@ -191,6 +191,7 @@ docs! {
     "implicit_clone",
     "implicit_hasher",
     "implicit_return",
+    "implicit_saturating_add",
     "implicit_saturating_sub",
     "imprecise_flops",
     "inconsistent_digit_grouping",

diff --git a/src/docs/implicit_saturating_add.txt b/src/docs/implicit_saturating_add.txt
@@ -0,0 +1,20 @@
+### What it does
+Checks for implicit saturating addition.
+
+### Why is this bad?
+The built-in function is more readable and may be faster.
+
+### Example
+```
+let mut u:u32 = 7000;
+
+if u != u32::MAX {
+    u += 1;
+}
+```
+Use instead:
+```
+let mut u:u32 = 7000;
+
+u = u.saturating_add(1);
+```
diff --git a/tests/ui/implicit_saturating_add.fixed b/tests/ui/implicit_saturating_add.fixed
@@ -0,0 +1,106 @@
+// run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::implicit_saturating_add)]
+
+fn main() {
+    let mut u_8: u8 = 255;
+    let mut u_16: u16 = 500;
+    let mut u_32: u32 = 7000;
+    let mut u_64: u64 = 7000;
+    let mut i_8: i8 = 30;
+    let mut i_16: i16 = 500;
+    let mut i_32: i32 = 7000;
+    let mut i_64: i64 = 7000;
+
+    if i_8 < 42 {
+        i_8 += 1;
+    }
+    if i_8 != 42 {
+        i_8 += 1;
+    }
+
+    u_8 = u_8.saturating_add(1);
+
+    u_8 = u_8.saturating_add(1);
+
+    if u_8 < 15 {
+        u_8 += 1;
+    }
+
+    u_16 = u_16.saturating_add(1);
+
+    u_16 = u_16.saturating_add(1);
+
+    u_16 = u_16.saturating_add(1);
+
+    u_32 = u_32.saturating_add(1);
+
+    u_32 = u_32.saturating_add(1);
+
+    u_32 = u_32.saturating_add(1);
+
+    u_64 = u_64.saturating_add(1);
+
+    u_64 = u_64.saturating_add(1);
+
+    u_64 = u_64.saturating_add(1);
+
+    i_8 = i_8.saturating_add(1);
+
+    i_8 = i_8.saturating_add(1);
+
+    i_8 = i_8.saturating_add(1);
+
+    i_16 = i_16.saturating_add(1);
+
+    i_16 = i_16.saturating_add(1);
+
+    i_16 = i_16.saturating_add(1);
+
+    i_32 = i_32.saturating_add(1);
+
+    i_32 = i_32.saturating_add(1);
+
+    i_32 = i_32.saturating_add(1);
+
+    i_64 = i_64.saturating_add(1);
+
+    i_64 = i_64.saturating_add(1);
+
+    i_64 = i_64.saturating_add(1);
+
+    if i_64 < 42 {
+        i_64 += 1;
+    }
+
+    if 42 > i_64 {
+        i_64 += 1;
+    }
+
+    let a = 12;
+    let mut b = 8;
+
+    if a < u8::MAX {
+        b += 1;
+    }
+
+    if u8::MAX > a {
+        b += 1;
+    }
+
+    if u_32 < u32::MAX {
+        u_32 += 1;
+    } else {
+        println!("don't lint this");
+    }
+
+    if u_32 < u32::MAX {
+        println!("don't lint this");
+        u_32 += 1;
+    }
+
+    if u_32 < 42 {
+        println!("brace yourself!");
+    } else {u_32 = u_32.saturating_add(1); }
+}