diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f12a673596..bfeb009727b 100644 --- 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 new file mode 100644 index 00000000000..bf1351829c6 --- /dev/null +++ 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 { + 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 { + 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 index 9ad2a88eb26..642d7070fb6 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -85,6 +85,7 @@ 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 index 307ec40f40b..08eb6c2b39b 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -184,6 +184,7 @@ 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 index ab7c4034b0e..6d5483bc701 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -29,6 +29,7 @@ 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 index 3b78e492baa..e5d26ac22a7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -238,6 +238,7 @@ macro_rules! declare_clippy_lint { 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 index 39540e4b048..1b727e1a100 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -191,6 +191,7 @@ pub fn explain(lint: &str) { "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 new file mode 100644 index 00000000000..5883a5363e2 --- /dev/null +++ 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); +``` \ No newline at end of file diff --git a/tests/ui/implicit_saturating_add.fixed b/tests/ui/implicit_saturating_add.fixed new file mode 100644 index 00000000000..7d363d59a6f --- /dev/null +++ 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); } +} diff --git a/tests/ui/implicit_saturating_add.rs b/tests/ui/implicit_saturating_add.rs new file mode 100644 index 00000000000..31a5916277f --- /dev/null +++ b/tests/ui/implicit_saturating_add.rs @@ -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; + } +} diff --git a/tests/ui/implicit_saturating_add.stderr b/tests/ui/implicit_saturating_add.stderr new file mode 100644 index 00000000000..42ae1b48885 --- /dev/null +++ b/tests/ui/implicit_saturating_add.stderr @@ -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 +