Skip to content

Commit 09e6c23

Browse files
borsbors
committed
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
2 parents 18e10ca + f1c831a commit 09e6c23

11 files changed

+598
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -3917,6 +3917,7 @@ Released 2018-09-13
39173917
[`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone
39183918
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
39193919
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
3920+
[`implicit_saturating_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_add
39203921
[`implicit_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub
39213922
[`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops
39223923
[`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping

‎clippy_lints/src/implicit_saturating_add.rs

+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
use clippy_utils::consts::{constant, Constant};
2+
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::get_parent_expr;
4+
use clippy_utils::source::snippet_with_applicability;
5+
use if_chain::if_chain;
6+
use rustc_ast::ast::{LitIntType, LitKind};
7+
use rustc_errors::Applicability;
8+
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Stmt, StmtKind};
9+
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_middle::ty::{Int, IntTy, Ty, Uint, UintTy};
11+
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Checks for implicit saturating addition.
16+
///
17+
/// ### Why is this bad?
18+
/// The built-in function is more readable and may be faster.
19+
///
20+
/// ### Example
21+
/// ```rust
22+
///let mut u:u32 = 7000;
23+
///
24+
/// if u != u32::MAX {
25+
/// u += 1;
26+
/// }
27+
/// ```
28+
/// Use instead:
29+
/// ```rust
30+
///let mut u:u32 = 7000;
31+
///
32+
/// u = u.saturating_add(1);
33+
/// ```
34+
#[clippy::version = "1.65.0"]
35+
pub IMPLICIT_SATURATING_ADD,
36+
style,
37+
"Perform saturating addition instead of implicitly checking max bound of data type"
38+
}
39+
declare_lint_pass!(ImplicitSaturatingAdd => [IMPLICIT_SATURATING_ADD]);
40+
41+
impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingAdd {
42+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
43+
if_chain! {
44+
if let ExprKind::If(cond, then, None) = expr.kind;
45+
if let ExprKind::DropTemps(expr1) = cond.kind;
46+
if let Some((c, op_node, l)) = get_const(cx, expr1);
47+
if let BinOpKind::Ne | BinOpKind::Lt = op_node;
48+
if let ExprKind::Block(block, None) = then.kind;
49+
if let Block {
50+
stmts:
51+
[Stmt
52+
{ kind: StmtKind::Expr(ex) | StmtKind::Semi(ex), .. }],
53+
expr: None, ..} |
54+
Block { stmts: [], expr: Some(ex), ..} = block;
55+
if let ExprKind::AssignOp(op1, target, value) = ex.kind;
56+
let ty = cx.typeck_results().expr_ty(target);
57+
if Some(c) == get_int_max(ty);
58+
if clippy_utils::SpanlessEq::new(cx).eq_expr(l, target);
59+
if BinOpKind::Add == op1.node;
60+
if let ExprKind::Lit(ref lit) = value.kind;
61+
if let LitKind::Int(1, LitIntType::Unsuffixed) = lit.node;
62+
if block.expr.is_none();
63+
then {
64+
let mut app = Applicability::MachineApplicable;
65+
let code = snippet_with_applicability(cx, target.span, "_", &mut app);
66+
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);")};
67+
span_lint_and_sugg(cx, IMPLICIT_SATURATING_ADD, expr.span, "manual saturating add detected", "use instead", sugg, app);
68+
}
69+
}
70+
}
71+
}
72+
73+
fn get_int_max(ty: Ty<'_>) -> Option<u128> {
74+
match ty.peel_refs().kind() {
75+
Int(IntTy::I8) => i8::max_value().try_into().ok(),
76+
Int(IntTy::I16) => i16::max_value().try_into().ok(),
77+
Int(IntTy::I32) => i32::max_value().try_into().ok(),
78+
Int(IntTy::I64) => i64::max_value().try_into().ok(),
79+
Int(IntTy::I128) => i128::max_value().try_into().ok(),
80+
Int(IntTy::Isize) => isize::max_value().try_into().ok(),
81+
Uint(UintTy::U8) => u8::max_value().try_into().ok(),
82+
Uint(UintTy::U16) => u16::max_value().try_into().ok(),
83+
Uint(UintTy::U32) => u32::max_value().try_into().ok(),
84+
Uint(UintTy::U64) => u64::max_value().try_into().ok(),
85+
Uint(UintTy::U128) => Some(u128::max_value()),
86+
Uint(UintTy::Usize) => usize::max_value().try_into().ok(),
87+
_ => None,
88+
}
89+
}
90+
91+
fn get_const<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<(u128, BinOpKind, &'tcx Expr<'tcx>)> {
92+
if let ExprKind::Binary(op, l, r) = expr.kind {
93+
let tr = cx.typeck_results();
94+
if let Some((Constant::Int(c), _)) = constant(cx, tr, r) {
95+
return Some((c, op.node, l));
96+
};
97+
if let Some((Constant::Int(c), _)) = constant(cx, tr, l) {
98+
return Some((c, invert_op(op.node)?, r));
99+
}
100+
}
101+
None
102+
}
103+
104+
fn invert_op(op: BinOpKind) -> Option<BinOpKind> {
105+
use rustc_hir::BinOpKind::{Ge, Gt, Le, Lt, Ne};
106+
match op {
107+
Lt => Some(Gt),
108+
Le => Some(Ge),
109+
Ne => Some(Ne),
110+
Ge => Some(Le),
111+
Gt => Some(Lt),
112+
_ => None,
113+
}
114+
}

‎clippy_lints/src/lib.register_all.rs

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
8585
LintId::of(functions::RESULT_UNIT_ERR),
8686
LintId::of(functions::TOO_MANY_ARGUMENTS),
8787
LintId::of(if_let_mutex::IF_LET_MUTEX),
88+
LintId::of(implicit_saturating_add::IMPLICIT_SATURATING_ADD),
8889
LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
8990
LintId::of(infinite_iter::INFINITE_ITER),
9091
LintId::of(inherent_to_string::INHERENT_TO_STRING),

‎clippy_lints/src/lib.register_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ store.register_lints(&[
184184
if_then_some_else_none::IF_THEN_SOME_ELSE_NONE,
185185
implicit_hasher::IMPLICIT_HASHER,
186186
implicit_return::IMPLICIT_RETURN,
187+
implicit_saturating_add::IMPLICIT_SATURATING_ADD,
187188
implicit_saturating_sub::IMPLICIT_SATURATING_SUB,
188189
inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR,
189190
index_refutable_slice::INDEX_REFUTABLE_SLICE,

‎clippy_lints/src/lib.register_style.rs

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
2929
LintId::of(functions::DOUBLE_MUST_USE),
3030
LintId::of(functions::MUST_USE_UNIT),
3131
LintId::of(functions::RESULT_UNIT_ERR),
32+
LintId::of(implicit_saturating_add::IMPLICIT_SATURATING_ADD),
3233
LintId::of(inherent_to_string::INHERENT_TO_STRING),
3334
LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS),
3435
LintId::of(len_zero::COMPARISON_TO_EMPTY),

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ mod if_not_else;
238238
mod if_then_some_else_none;
239239
mod implicit_hasher;
240240
mod implicit_return;
241+
mod implicit_saturating_add;
241242
mod implicit_saturating_sub;
242243
mod inconsistent_struct_constructor;
243244
mod index_refutable_slice;
@@ -904,6 +905,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
904905
store.register_early_pass(|| Box::new(multi_assignments::MultiAssignments));
905906
store.register_late_pass(|_| Box::new(bool_to_int_with_if::BoolToIntWithIf));
906907
store.register_late_pass(|_| Box::new(box_default::BoxDefault));
908+
store.register_late_pass(|_| Box::new(implicit_saturating_add::ImplicitSaturatingAdd));
907909
// add lints here, do not remove this comment, it's used in `new_lint`
908910
}
909911

‎src/docs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ docs! {
191191
"implicit_clone",
192192
"implicit_hasher",
193193
"implicit_return",
194+
"implicit_saturating_add",
194195
"implicit_saturating_sub",
195196
"imprecise_flops",
196197
"inconsistent_digit_grouping",

‎src/docs/implicit_saturating_add.txt

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
### What it does
2+
Checks for implicit saturating addition.
3+
4+
### Why is this bad?
5+
The built-in function is more readable and may be faster.
6+
7+
### Example
8+
```
9+
let mut u:u32 = 7000;
10+
11+
if u != u32::MAX {
12+
u += 1;
13+
}
14+
```
15+
Use instead:
16+
```
17+
let mut u:u32 = 7000;
18+
19+
u = u.saturating_add(1);
20+
```

‎tests/ui/implicit_saturating_add.fixed

+106
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// run-rustfix
2+
3+
#![allow(unused)]
4+
#![warn(clippy::implicit_saturating_add)]
5+
6+
fn main() {
7+
let mut u_8: u8 = 255;
8+
let mut u_16: u16 = 500;
9+
let mut u_32: u32 = 7000;
10+
let mut u_64: u64 = 7000;
11+
let mut i_8: i8 = 30;
12+
let mut i_16: i16 = 500;
13+
let mut i_32: i32 = 7000;
14+
let mut i_64: i64 = 7000;
15+
16+
if i_8 < 42 {
17+
i_8 += 1;
18+
}
19+
if i_8 != 42 {
20+
i_8 += 1;
21+
}
22+
23+
u_8 = u_8.saturating_add(1);
24+
25+
u_8 = u_8.saturating_add(1);
26+
27+
if u_8 < 15 {
28+
u_8 += 1;
29+
}
30+
31+
u_16 = u_16.saturating_add(1);
32+
33+
u_16 = u_16.saturating_add(1);
34+
35+
u_16 = u_16.saturating_add(1);
36+
37+
u_32 = u_32.saturating_add(1);
38+
39+
u_32 = u_32.saturating_add(1);
40+
41+
u_32 = u_32.saturating_add(1);
42+
43+
u_64 = u_64.saturating_add(1);
44+
45+
u_64 = u_64.saturating_add(1);
46+
47+
u_64 = u_64.saturating_add(1);
48+
49+
i_8 = i_8.saturating_add(1);
50+
51+
i_8 = i_8.saturating_add(1);
52+
53+
i_8 = i_8.saturating_add(1);
54+
55+
i_16 = i_16.saturating_add(1);
56+
57+
i_16 = i_16.saturating_add(1);
58+
59+
i_16 = i_16.saturating_add(1);
60+
61+
i_32 = i_32.saturating_add(1);
62+
63+
i_32 = i_32.saturating_add(1);
64+
65+
i_32 = i_32.saturating_add(1);
66+
67+
i_64 = i_64.saturating_add(1);
68+
69+
i_64 = i_64.saturating_add(1);
70+
71+
i_64 = i_64.saturating_add(1);
72+
73+
if i_64 < 42 {
74+
i_64 += 1;
75+
}
76+
77+
if 42 > i_64 {
78+
i_64 += 1;
79+
}
80+
81+
let a = 12;
82+
let mut b = 8;
83+
84+
if a < u8::MAX {
85+
b += 1;
86+
}
87+
88+
if u8::MAX > a {
89+
b += 1;
90+
}
91+
92+
if u_32 < u32::MAX {
93+
u_32 += 1;
94+
} else {
95+
println!("don't lint this");
96+
}
97+
98+
if u_32 < u32::MAX {
99+
println!("don't lint this");
100+
u_32 += 1;
101+
}
102+
103+
if u_32 < 42 {
104+
println!("brace yourself!");
105+
} else {u_32 = u_32.saturating_add(1); }
106+
}

0 commit comments

Comments
 (0)