Auto merge of #9549 - royrustdev:manual_saturating_add, r=llogiq

new `implicit_saturating_add` lint

This fixes #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
This commit is contained in:
bors 2022-10-03 11:32:06 +00:00
commit 09e6c239f3
11 changed files with 598 additions and 0 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -191,6 +191,7 @@ docs! {
"implicit_clone",
"implicit_hasher",
"implicit_return",
"implicit_saturating_add",
"implicit_saturating_sub",
"imprecise_flops",
"inconsistent_digit_grouping",

View file

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

View file

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

View file

@ -0,0 +1,154 @@
// 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;
}
if u_8 != u8::MAX {
u_8 += 1;
}
if u_8 < u8::MAX {
u_8 += 1;
}
if u_8 < 15 {
u_8 += 1;
}
if u_16 != u16::MAX {
u_16 += 1;
}
if u_16 < u16::MAX {
u_16 += 1;
}
if u16::MAX > u_16 {
u_16 += 1;
}
if u_32 != u32::MAX {
u_32 += 1;
}
if u_32 < u32::MAX {
u_32 += 1;
}
if u32::MAX > u_32 {
u_32 += 1;
}
if u_64 != u64::MAX {
u_64 += 1;
}
if u_64 < u64::MAX {
u_64 += 1;
}
if u64::MAX > u_64 {
u_64 += 1;
}
if i_8 != i8::MAX {
i_8 += 1;
}
if i_8 < i8::MAX {
i_8 += 1;
}
if i8::MAX > i_8 {
i_8 += 1;
}
if i_16 != i16::MAX {
i_16 += 1;
}
if i_16 < i16::MAX {
i_16 += 1;
}
if i16::MAX > i_16 {
i_16 += 1;
}
if i_32 != i32::MAX {
i_32 += 1;
}
if i_32 < i32::MAX {
i_32 += 1;
}
if i32::MAX > i_32 {
i_32 += 1;
}
if i_64 != i64::MAX {
i_64 += 1;
}
if i_64 < i64::MAX {
i_64 += 1;
}
if i64::MAX > i_64 {
i_64 += 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 if u_32 < u32::MAX {
u_32 += 1;
}
}

View file

@ -0,0 +1,197 @@
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:23:5
|
LL | / if u_8 != u8::MAX {
LL | | u_8 += 1;
LL | | }
| |_____^ help: use instead: `u_8 = u_8.saturating_add(1);`
|
= note: `-D clippy::implicit-saturating-add` implied by `-D warnings`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:27:5
|
LL | / if u_8 < u8::MAX {
LL | | u_8 += 1;
LL | | }
| |_____^ help: use instead: `u_8 = u_8.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:35:5
|
LL | / if u_16 != u16::MAX {
LL | | u_16 += 1;
LL | | }
| |_____^ help: use instead: `u_16 = u_16.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:39:5
|
LL | / if u_16 < u16::MAX {
LL | | u_16 += 1;
LL | | }
| |_____^ help: use instead: `u_16 = u_16.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:43:5
|
LL | / if u16::MAX > u_16 {
LL | | u_16 += 1;
LL | | }
| |_____^ help: use instead: `u_16 = u_16.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:47:5
|
LL | / if u_32 != u32::MAX {
LL | | u_32 += 1;
LL | | }
| |_____^ help: use instead: `u_32 = u_32.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:51:5
|
LL | / if u_32 < u32::MAX {
LL | | u_32 += 1;
LL | | }
| |_____^ help: use instead: `u_32 = u_32.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:55:5
|
LL | / if u32::MAX > u_32 {
LL | | u_32 += 1;
LL | | }
| |_____^ help: use instead: `u_32 = u_32.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:59:5
|
LL | / if u_64 != u64::MAX {
LL | | u_64 += 1;
LL | | }
| |_____^ help: use instead: `u_64 = u_64.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:63:5
|
LL | / if u_64 < u64::MAX {
LL | | u_64 += 1;
LL | | }
| |_____^ help: use instead: `u_64 = u_64.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:67:5
|
LL | / if u64::MAX > u_64 {
LL | | u_64 += 1;
LL | | }
| |_____^ help: use instead: `u_64 = u_64.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:71:5
|
LL | / if i_8 != i8::MAX {
LL | | i_8 += 1;
LL | | }
| |_____^ help: use instead: `i_8 = i_8.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:75:5
|
LL | / if i_8 < i8::MAX {
LL | | i_8 += 1;
LL | | }
| |_____^ help: use instead: `i_8 = i_8.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:79:5
|
LL | / if i8::MAX > i_8 {
LL | | i_8 += 1;
LL | | }
| |_____^ help: use instead: `i_8 = i_8.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:83:5
|
LL | / if i_16 != i16::MAX {
LL | | i_16 += 1;
LL | | }
| |_____^ help: use instead: `i_16 = i_16.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:87:5
|
LL | / if i_16 < i16::MAX {
LL | | i_16 += 1;
LL | | }
| |_____^ help: use instead: `i_16 = i_16.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:91:5
|
LL | / if i16::MAX > i_16 {
LL | | i_16 += 1;
LL | | }
| |_____^ help: use instead: `i_16 = i_16.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:95:5
|
LL | / if i_32 != i32::MAX {
LL | | i_32 += 1;
LL | | }
| |_____^ help: use instead: `i_32 = i_32.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:99:5
|
LL | / if i_32 < i32::MAX {
LL | | i_32 += 1;
LL | | }
| |_____^ help: use instead: `i_32 = i_32.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:103:5
|
LL | / if i32::MAX > i_32 {
LL | | i_32 += 1;
LL | | }
| |_____^ help: use instead: `i_32 = i_32.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:107:5
|
LL | / if i_64 != i64::MAX {
LL | | i_64 += 1;
LL | | }
| |_____^ help: use instead: `i_64 = i_64.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:111:5
|
LL | / if i_64 < i64::MAX {
LL | | i_64 += 1;
LL | | }
| |_____^ help: use instead: `i_64 = i_64.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:115:5
|
LL | / if i64::MAX > i_64 {
LL | | i_64 += 1;
LL | | }
| |_____^ help: use instead: `i_64 = i_64.saturating_add(1);`
error: manual saturating add detected
--> $DIR/implicit_saturating_add.rs:151:12
|
LL | } else if u_32 < u32::MAX {
| ____________^
LL | | u_32 += 1;
LL | | }
| |_____^ help: use instead: `{u_32 = u_32.saturating_add(1); }`
error: aborting due to 24 previous errors