diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b62c9a59aa5..27b5854cc223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5847,6 +5847,7 @@ Released 2018-09-13 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits [`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals +[`manual_checked_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_checked_sub [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp [`manual_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains [`manual_dangling_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_dangling_ptr diff --git a/clippy_lints/.DS_Store b/clippy_lints/.DS_Store new file mode 100644 index 000000000000..4b3523353e03 Binary files /dev/null and b/clippy_lints/.DS_Store differ diff --git a/clippy_lints/src/.DS_Store b/clippy_lints/src/.DS_Store new file mode 100644 index 000000000000..88c8a6f0cc7f Binary files /dev/null and b/clippy_lints/src/.DS_Store differ diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2cccd6ba2702..c2aa079ea090 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -289,6 +289,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::manual_assert::MANUAL_ASSERT_INFO, crate::manual_async_fn::MANUAL_ASYNC_FN_INFO, crate::manual_bits::MANUAL_BITS_INFO, + crate::manual_checked_sub::MANUAL_CHECKED_SUB_INFO, crate::manual_clamp::MANUAL_CLAMP_INFO, crate::manual_div_ceil::MANUAL_DIV_CEIL_INFO, crate::manual_float_methods::MANUAL_IS_FINITE_INFO, diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 7da5a530eaa3..507c2da7a83d 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -433,12 +433,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { (2, deref_msg) }; - if deref_count >= required_refs { + if let Some(result) = deref_count.checked_sub(required_refs) { self.state = Some(( State::DerefedBorrow(DerefedBorrow { // One of the required refs is for the current borrow expression, the remaining ones // can't be removed without breaking the code. See earlier comment. - count: deref_count - required_refs, + count: result, msg, stability, for_field_access: if let ExprUseNode::FieldAccess(name) = use_node diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5fa8f6f4bf3d..c856f0187412 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -208,6 +208,7 @@ mod manual_abs_diff; mod manual_assert; mod manual_async_fn; mod manual_bits; +mod manual_checked_sub; mod manual_clamp; mod manual_div_ceil; mod manual_float_methods; @@ -942,6 +943,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf))); store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf))); store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap)); + store.register_late_pass(move |_| Box::new(manual_checked_sub::ManualCheckedSub::new(conf))); store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix)); + // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_checked_sub.rs b/clippy_lints/src/manual_checked_sub.rs new file mode 100644 index 000000000000..a862b01a9225 --- /dev/null +++ b/clippy_lints/src/manual_checked_sub.rs @@ -0,0 +1,193 @@ +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint; +use clippy_utils::msrvs::{self, Msrv}; + +use clippy_utils::{is_mutable, path_to_local}; +use rustc_ast::{BinOpKind, LitIntType, LitKind}; +use rustc_data_structures::packed::Pu128; +use rustc_hir::intravisit::{Visitor, walk_expr}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self}; +use rustc_session::impl_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// Checks for manual re-implementations of checked subtraction. + /// + /// ### Why is this bad? + /// Manually re-implementing checked subtraction can be error-prone and less readable. + /// Using the standard library method `.checked_sub()` is clearer and less likely to contain bugs. + /// + /// ### Example + /// ```rust + /// // Bad: Manual implementation of checked subtraction + /// fn get_remaining_items(total: u32, used: u32) -> Option { + /// if total >= used { + /// Some(total - used) + /// } else { + /// None + /// } + /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// // Good: Using the standard library's checked_sub + /// fn get_remaining_items(total: u32, used: u32) -> Option { + /// total.checked_sub(used) + /// } + /// ``` + #[clippy::version = "1.86.0"] + pub MANUAL_CHECKED_SUB, + complexity, + "Checks for manual re-implementations of checked subtraction." +} + +pub struct ManualCheckedSub { + msrv: Msrv, +} + +impl ManualCheckedSub { + #[must_use] + pub fn new(conf: &'static Conf) -> Self { + Self { msrv: conf.msrv } + } +} + +impl_lint_pass!(ManualCheckedSub => [MANUAL_CHECKED_SUB]); + +impl<'tcx> LateLintPass<'tcx> for ManualCheckedSub { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if !self.msrv.meets(cx, msrvs::CHECKED_SUB) { + return; + } + + if let Some(if_expr) = clippy_utils::higher::If::hir(expr) + && let ExprKind::Binary(op, lhs, rhs) = if_expr.cond.kind + && !(matches!(lhs.kind, ExprKind::Lit(_)) && matches!(rhs.kind, ExprKind::Lit(_))) + && is_unsigned_int(cx, lhs) + && is_unsigned_int(cx, rhs) + { + // Skip if either non-literal operand is mutable + if (!matches!(lhs.kind, ExprKind::Lit(_)) && is_mutable(cx, lhs)) + || (!matches!(rhs.kind, ExprKind::Lit(_)) && is_mutable(cx, rhs)) + { + return; + } + + // Skip if either lhs or rhs is a macro call + if lhs.span.from_expansion() || rhs.span.from_expansion() { + return; + } + + if let BinOpKind::Ge | BinOpKind::Gt | BinOpKind::Le | BinOpKind::Lt = op.node { + SubExprVisitor { + cx, + if_expr: expr, + + condition_lhs: lhs, + condition_rhs: rhs, + condition_op: op.node, + } + .visit_expr(if_expr.then); + } + } + } +} + +struct SubExprVisitor<'cx, 'tcx> { + cx: &'cx LateContext<'tcx>, + if_expr: &'tcx Expr<'tcx>, + + condition_lhs: &'tcx Expr<'tcx>, + condition_rhs: &'tcx Expr<'tcx>, + condition_op: BinOpKind, +} + +impl<'tcx> Visitor<'tcx> for SubExprVisitor<'_, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if let ExprKind::Binary(op, sub_lhs, sub_rhs) = expr.kind + && let BinOpKind::Sub = op.node + { + // // Skip if either sub_lhs or sub_rhs is a macro call + if sub_lhs.span.from_expansion() || sub_rhs.span.from_expansion() { + return; + } + + if let ExprKind::Lit(lit) = self.condition_lhs.kind + && self.condition_op == BinOpKind::Lt + && let LitKind::Int(Pu128(0), _) = lit.node + && (is_referencing_same_variable(sub_lhs, self.condition_rhs)) + { + self.emit_lint(); + } + + if let ExprKind::Lit(lit) = self.condition_rhs.kind + && self.condition_op == BinOpKind::Gt + && let LitKind::Int(Pu128(0), _) = lit.node + && (is_referencing_same_variable(sub_lhs, self.condition_lhs)) + { + self.emit_lint(); + } + + if self.condition_op == BinOpKind::Ge + && (is_referencing_same_variable(sub_lhs, self.condition_lhs) + || are_literals_equal(sub_lhs, self.condition_lhs)) + && (is_referencing_same_variable(sub_rhs, self.condition_rhs) + || are_literals_equal(sub_rhs, self.condition_rhs)) + { + self.emit_lint(); + } + + if self.condition_op == BinOpKind::Le + && (is_referencing_same_variable(sub_lhs, self.condition_rhs) + || are_literals_equal(sub_lhs, self.condition_rhs)) + && (is_referencing_same_variable(sub_rhs, self.condition_lhs) + || are_literals_equal(sub_rhs, self.condition_lhs)) + { + self.emit_lint(); + } + } + + walk_expr(self, expr); + } +} + +impl SubExprVisitor<'_, '_> { + fn emit_lint(&mut self) { + span_lint( + self.cx, + MANUAL_CHECKED_SUB, + self.if_expr.span, + "manual re-implementation of checked subtraction - consider using `.checked_sub()`", + ); + } +} + +fn is_unsigned_int<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool { + let expr_type = cx.typeck_results().expr_ty(expr).peel_refs(); + if matches!(expr_type.kind(), ty::Uint(_)) { + return true; + } + + false +} + +fn are_literals_equal<'tcx>(expr1: &Expr<'tcx>, expr2: &Expr<'tcx>) -> bool { + if let (ExprKind::Lit(lit1), ExprKind::Lit(lit2)) = (&expr1.kind, &expr2.kind) + && let (LitKind::Int(val1, suffix1), LitKind::Int(val2, suffix2)) = (&lit1.node, &lit2.node) + { + return val1 == val2 && suffix1 == suffix2 && *suffix1 != LitIntType::Unsuffixed; + } + + false +} + +fn is_referencing_same_variable<'tcx>(expr1: &'tcx Expr<'_>, expr2: &'tcx Expr<'_>) -> bool { + if let (Some(id1), Some(id2)) = (path_to_local(expr1), path_to_local(expr2)) { + return id1 == id2; + } + + false +} diff --git a/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs b/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs index a2a522a60687..13715b823aa6 100644 --- a/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs +++ b/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs @@ -30,10 +30,10 @@ fn extract_count_with_applicability( && let LitKind::Int(Pu128(upper_bound), _) = lit.node { // Here we can explicitly calculate the number of iterations - let count = if upper_bound >= lower_bound { + let count = if let Some(result) = upper_bound.checked_sub(lower_bound) { match range.limits { - RangeLimits::HalfOpen => upper_bound - lower_bound, - RangeLimits::Closed => (upper_bound - lower_bound).checked_add(1)?, + RangeLimits::HalfOpen => result, + RangeLimits::Closed => (result).checked_add(1)?, } } else { 0 diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 19061b574ff8..b444e2185838 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -51,7 +51,7 @@ msrv_aliases! { 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } 1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS } 1,50,0 { BOOL_THEN, CLAMP, SLICE_FILL } - 1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN, SATURATING_SUB_CONST } + 1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN, SATURATING_SUB_CONST, CHECKED_SUB } 1,46,0 { CONST_IF_MATCH } 1,45,0 { STR_STRIP_PREFIX } 1,43,0 { LOG2_10, LOG10_2, NUMERIC_ASSOCIATED_CONSTANTS } diff --git a/tests/ui/implicit_saturating_sub.fixed b/tests/ui/implicit_saturating_sub.fixed index 1aab6c54407e..aa62b7ffa4c0 100644 --- a/tests/ui/implicit_saturating_sub.fixed +++ b/tests/ui/implicit_saturating_sub.fixed @@ -1,4 +1,9 @@ -#![allow(unused_assignments, unused_mut, clippy::assign_op_pattern)] +#![allow( + unused_assignments, + unused_mut, + clippy::assign_op_pattern, + clippy::manual_checked_sub +)] #![warn(clippy::implicit_saturating_sub)] use std::cmp::PartialEq; diff --git a/tests/ui/implicit_saturating_sub.rs b/tests/ui/implicit_saturating_sub.rs index 7ca57a2902db..5d774127cb62 100644 --- a/tests/ui/implicit_saturating_sub.rs +++ b/tests/ui/implicit_saturating_sub.rs @@ -1,4 +1,9 @@ -#![allow(unused_assignments, unused_mut, clippy::assign_op_pattern)] +#![allow( + unused_assignments, + unused_mut, + clippy::assign_op_pattern, + clippy::manual_checked_sub +)] #![warn(clippy::implicit_saturating_sub)] use std::cmp::PartialEq; diff --git a/tests/ui/implicit_saturating_sub.stderr b/tests/ui/implicit_saturating_sub.stderr index 0c225856fd07..a8d433799efc 100644 --- a/tests/ui/implicit_saturating_sub.stderr +++ b/tests/ui/implicit_saturating_sub.stderr @@ -1,5 +1,5 @@ error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:27:5 + --> tests/ui/implicit_saturating_sub.rs:32:5 | LL | / if u_8 > 0 { LL | | @@ -11,7 +11,7 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::implicit_saturating_sub)]` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:35:13 + --> tests/ui/implicit_saturating_sub.rs:40:13 | LL | / if u_8 > 0 { LL | | @@ -20,7 +20,7 @@ LL | | } | |_____________^ help: try: `u_8 = u_8.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:50:5 + --> tests/ui/implicit_saturating_sub.rs:55:5 | LL | / if u_16 > 0 { LL | | @@ -29,7 +29,7 @@ LL | | } | |_____^ help: try: `u_16 = u_16.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:61:5 + --> tests/ui/implicit_saturating_sub.rs:66:5 | LL | / if u_32 != 0 { LL | | @@ -38,7 +38,7 @@ LL | | } | |_____^ help: try: `u_32 = u_32.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:83:5 + --> tests/ui/implicit_saturating_sub.rs:88:5 | LL | / if u_64 > 0 { LL | | @@ -47,7 +47,7 @@ LL | | } | |_____^ help: try: `u_64 = u_64.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:89:5 + --> tests/ui/implicit_saturating_sub.rs:94:5 | LL | / if 0 < u_64 { LL | | @@ -56,7 +56,7 @@ LL | | } | |_____^ help: try: `u_64 = u_64.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:95:5 + --> tests/ui/implicit_saturating_sub.rs:100:5 | LL | / if 0 != u_64 { LL | | @@ -65,7 +65,7 @@ LL | | } | |_____^ help: try: `u_64 = u_64.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:117:5 + --> tests/ui/implicit_saturating_sub.rs:122:5 | LL | / if u_usize > 0 { LL | | @@ -74,7 +74,7 @@ LL | | } | |_____^ help: try: `u_usize = u_usize.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:130:5 + --> tests/ui/implicit_saturating_sub.rs:135:5 | LL | / if i_8 > i8::MIN { LL | | @@ -83,7 +83,7 @@ LL | | } | |_____^ help: try: `i_8 = i_8.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:136:5 + --> tests/ui/implicit_saturating_sub.rs:141:5 | LL | / if i_8 > i8::MIN { LL | | @@ -92,7 +92,7 @@ LL | | } | |_____^ help: try: `i_8 = i_8.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:142:5 + --> tests/ui/implicit_saturating_sub.rs:147:5 | LL | / if i_8 != i8::MIN { LL | | @@ -101,7 +101,7 @@ LL | | } | |_____^ help: try: `i_8 = i_8.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:148:5 + --> tests/ui/implicit_saturating_sub.rs:153:5 | LL | / if i_8 != i8::MIN { LL | | @@ -110,7 +110,7 @@ LL | | } | |_____^ help: try: `i_8 = i_8.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:159:5 + --> tests/ui/implicit_saturating_sub.rs:164:5 | LL | / if i_16 > i16::MIN { LL | | @@ -119,7 +119,7 @@ LL | | } | |_____^ help: try: `i_16 = i_16.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:165:5 + --> tests/ui/implicit_saturating_sub.rs:170:5 | LL | / if i_16 > i16::MIN { LL | | @@ -128,7 +128,7 @@ LL | | } | |_____^ help: try: `i_16 = i_16.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:171:5 + --> tests/ui/implicit_saturating_sub.rs:176:5 | LL | / if i_16 != i16::MIN { LL | | @@ -137,7 +137,7 @@ LL | | } | |_____^ help: try: `i_16 = i_16.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:177:5 + --> tests/ui/implicit_saturating_sub.rs:182:5 | LL | / if i_16 != i16::MIN { LL | | @@ -146,7 +146,7 @@ LL | | } | |_____^ help: try: `i_16 = i_16.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:188:5 + --> tests/ui/implicit_saturating_sub.rs:193:5 | LL | / if i_32 > i32::MIN { LL | | @@ -155,7 +155,7 @@ LL | | } | |_____^ help: try: `i_32 = i_32.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:194:5 + --> tests/ui/implicit_saturating_sub.rs:199:5 | LL | / if i_32 > i32::MIN { LL | | @@ -164,7 +164,7 @@ LL | | } | |_____^ help: try: `i_32 = i_32.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:200:5 + --> tests/ui/implicit_saturating_sub.rs:205:5 | LL | / if i_32 != i32::MIN { LL | | @@ -173,7 +173,7 @@ LL | | } | |_____^ help: try: `i_32 = i_32.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:206:5 + --> tests/ui/implicit_saturating_sub.rs:211:5 | LL | / if i_32 != i32::MIN { LL | | @@ -182,7 +182,7 @@ LL | | } | |_____^ help: try: `i_32 = i_32.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:217:5 + --> tests/ui/implicit_saturating_sub.rs:222:5 | LL | / if i64::MIN < i_64 { LL | | @@ -191,7 +191,7 @@ LL | | } | |_____^ help: try: `i_64 = i_64.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:223:5 + --> tests/ui/implicit_saturating_sub.rs:228:5 | LL | / if i64::MIN != i_64 { LL | | @@ -200,7 +200,7 @@ LL | | } | |_____^ help: try: `i_64 = i_64.saturating_sub(1);` error: implicitly performing saturating subtraction - --> tests/ui/implicit_saturating_sub.rs:229:5 + --> tests/ui/implicit_saturating_sub.rs:234:5 | LL | / if i64::MIN < i_64 { LL | | @@ -209,7 +209,7 @@ LL | | } | |_____^ help: try: `i_64 = i_64.saturating_sub(1);` error: manual arithmetic check found - --> tests/ui/implicit_saturating_sub.rs:298:12 + --> tests/ui/implicit_saturating_sub.rs:303:12 | LL | } else if a >= b { | ____________^ @@ -221,19 +221,19 @@ LL | | } | |_____^ help: replace it with: `{ b.saturating_sub(a) }` error: manual arithmetic check found - --> tests/ui/implicit_saturating_sub.rs:314:13 + --> tests/ui/implicit_saturating_sub.rs:319:13 | LL | let _ = if a * 2 > b { a * 2 - b } else { 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `(a * 2).saturating_sub(b)` error: manual arithmetic check found - --> tests/ui/implicit_saturating_sub.rs:317:13 + --> tests/ui/implicit_saturating_sub.rs:322:13 | LL | let _ = if a > b * 2 { a - b * 2 } else { 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b * 2)` error: manual arithmetic check found - --> tests/ui/implicit_saturating_sub.rs:320:13 + --> tests/ui/implicit_saturating_sub.rs:325:13 | LL | let _ = if a < b * 2 { 0 } else { a - b * 2 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b * 2)` diff --git a/tests/ui/manual_abs_diff.fixed b/tests/ui/manual_abs_diff.fixed index f1b1278ea6d2..57eb303f0977 100644 --- a/tests/ui/manual_abs_diff.fixed +++ b/tests/ui/manual_abs_diff.fixed @@ -1,5 +1,5 @@ +#![allow(clippy::manual_checked_sub)] #![warn(clippy::manual_abs_diff)] - use std::time::Duration; fn main() { diff --git a/tests/ui/manual_abs_diff.rs b/tests/ui/manual_abs_diff.rs index 60ef819c12d3..ee9c8298ab98 100644 --- a/tests/ui/manual_abs_diff.rs +++ b/tests/ui/manual_abs_diff.rs @@ -1,5 +1,5 @@ +#![allow(clippy::manual_checked_sub)] #![warn(clippy::manual_abs_diff)] - use std::time::Duration; fn main() { diff --git a/tests/ui/manual_checked_sub.rs b/tests/ui/manual_checked_sub.rs new file mode 100644 index 000000000000..167a8655f7ee --- /dev/null +++ b/tests/ui/manual_checked_sub.rs @@ -0,0 +1,150 @@ +#![allow(clippy::collapsible_if)] +// #![allow(clippy::manual_abs_diff)] +#![warn(clippy::manual_checked_sub)] + +fn positive_tests() { + let a: u32 = 10; + let b: u32 = 5; + let c: u32 = 3; + + if a >= b { + //~^ manual_checked_sub + let difference = "some test"; + let d = a - b; + } + + if b <= a { + //~^ manual_checked_sub + let d = a - b; + } + + if a >= b { + //~^ manual_checked_sub + let d = a - b; + } else { + let d = a + b; + } + + if a >= b { + //~^ manual_checked_sub + let d = a - b; + } else if a < b { + let d = a + b; + } + + if a > 0 { + //~^ manual_checked_sub + let _ = a - 1; + } + + if 0 < a { + //~^ manual_checked_sub + let _ = a - 1; + } + + let mut difference = "variable initialized outside the if scope"; + if a >= b { + //~^ manual_checked_sub + let some_reference = difference; + let d = a - b; + } + + if a > 0 { + //~^ manual_checked_sub + let decremented = "variable initialized inside the if scope"; + let d = a - b; + } + + if a >= b { + //~^ manual_checked_sub + some_function(a - b); + } + + struct Example { + value: u32, + } + + if a >= b { + //~^ manual_checked_sub + let _ = Example { value: a - b }; + } + + if a >= b { + //~^ manual_checked_sub + if c > 0 { + let _ = a - b; + } + } + + if a >= b { + if c > 0 { + //~^ manual_checked_sub + let _ = c - 1; + } + } +} + +fn some_function(a: u32) {} + +fn negative_tests() { + let a: u32 = 10; + let b: u32 = 5; + + let x: i32 = 10; + let y: i32 = 5; + + let mut a_mut = 10; + + if a_mut >= b { + a_mut *= 2; + let c = a_mut - b; + } + + if a_mut > 0 { + a_mut *= 2; + let c = a_mut - 1; + } + + if x >= y { + let _ = x - y; + } + + if a == b { + let _ = a - b; + } + + if a > b { + let _ = a - b; + } + + if 10 >= 5 { + let _ = 10 - 5; + } + + macro_rules! id { + ($e:ident) => { + $e + }; + } + if id!(a) >= id!(b) { + let d = id!(a) - id!(b); + } + + if a >= b { + let d = id!(a) - id!(b); + } + + if a >= b { + macro_rules! subtract { + () => { + a - b + }; + } + let _ = subtract!(); + } +} + +fn main() { + positive_tests(); + negative_tests(); +} diff --git a/tests/ui/manual_checked_sub.stderr b/tests/ui/manual_checked_sub.stderr new file mode 100644 index 000000000000..a487ec7dbd9c --- /dev/null +++ b/tests/ui/manual_checked_sub.stderr @@ -0,0 +1,122 @@ +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:10:5 + | +LL | / if a >= b { +LL | | +LL | | let difference = "some test"; +LL | | let d = a - b; +LL | | } + | |_____^ + | + = note: `-D clippy::manual-checked-sub` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_checked_sub)]` + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:16:5 + | +LL | / if b <= a { +LL | | +LL | | let d = a - b; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:21:5 + | +LL | / if a >= b { +LL | | +LL | | let d = a - b; +LL | | } else { +LL | | let d = a + b; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:28:5 + | +LL | / if a >= b { +LL | | +LL | | let d = a - b; +LL | | } else if a < b { +LL | | let d = a + b; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:35:5 + | +LL | / if a > 0 { +LL | | +LL | | let _ = a - 1; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:40:5 + | +LL | / if 0 < a { +LL | | +LL | | let _ = a - 1; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:46:5 + | +LL | / if a >= b { +LL | | +LL | | let some_reference = difference; +LL | | let d = a - b; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:52:5 + | +LL | / if a > 0 { +LL | | +LL | | let decremented = "variable initialized inside the if scope"; +LL | | let d = a - b; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:58:5 + | +LL | / if a >= b { +LL | | +LL | | some_function(a - b); +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:67:5 + | +LL | / if a >= b { +LL | | +LL | | let _ = Example { value: a - b }; +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:72:5 + | +LL | / if a >= b { +LL | | +LL | | if c > 0 { +LL | | let _ = a - b; +LL | | } +LL | | } + | |_____^ + +error: manual re-implementation of checked subtraction - consider using `.checked_sub()` + --> tests/ui/manual_checked_sub.rs:80:9 + | +LL | / if c > 0 { +LL | | +LL | | let _ = c - 1; +LL | | } + | |_________^ + +error: aborting due to 12 previous errors +