Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new implicit_saturating_add lint #9549

Merged
merged 1 commit into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
114 changes: 114 additions & 0 deletions clippy_lints/src/implicit_saturating_add.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
1 change: 1 addition & 0 deletions src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ docs! {
"implicit_clone",
"implicit_hasher",
"implicit_return",
"implicit_saturating_add",
"implicit_saturating_sub",
"imprecise_flops",
"inconsistent_digit_grouping",
Expand Down
20 changes: 20 additions & 0 deletions src/docs/implicit_saturating_add.txt
Original file line number Diff line number Diff line change
@@ -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);
```
106 changes: 106 additions & 0 deletions tests/ui/implicit_saturating_add.fixed
Original file line number Diff line number Diff line change
@@ -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); }
}
Loading