From b0edbca0e6e4ce771bc5ecf54b9b35a824af07aa Mon Sep 17 00:00:00 2001 From: Paolo Borelli Date: Mon, 4 Apr 2022 18:38:38 +0200 Subject: [PATCH] new lint cast_abs_to_unsigned Add a lint to detect cast to unsigned for abs() and suggest unsigned_abs() to avoid panic when called on MIN. --- CHANGELOG.md | 1 + .../src/casts/cast_abs_to_unsigned.rs | 42 +++++++++++++++++++ clippy_lints/src/casts/mod.rs | 27 +++++++++++- clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_suspicious.rs | 1 + clippy_lints/src/utils/conf.rs | 2 +- clippy_utils/src/msrvs.rs | 2 +- tests/ui/cast.rs | 2 +- tests/ui/cast_abs_to_unsigned.fixed | 8 ++++ tests/ui/cast_abs_to_unsigned.rs | 8 ++++ tests/ui/cast_abs_to_unsigned.stderr | 10 +++++ tests/ui/min_rust_version_attr.rs | 6 +++ tests/ui/min_rust_version_attr.stderr | 8 ++-- 14 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 clippy_lints/src/casts/cast_abs_to_unsigned.rs create mode 100644 tests/ui/cast_abs_to_unsigned.fixed create mode 100644 tests/ui/cast_abs_to_unsigned.rs create mode 100644 tests/ui/cast_abs_to_unsigned.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 430154ac34d..fd0e58d5116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3201,6 +3201,7 @@ Released 2018-09-13 [`bytes_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_nth [`cargo_common_metadata`]: https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata [`case_sensitive_file_extension_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons +[`cast_abs_to_unsigned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_abs_to_unsigned [`cast_enum_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_enum_constructor [`cast_enum_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_enum_truncation [`cast_lossless`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless diff --git a/clippy_lints/src/casts/cast_abs_to_unsigned.rs b/clippy_lints/src/casts/cast_abs_to_unsigned.rs new file mode 100644 index 00000000000..e9b0f1f672d --- /dev/null +++ b/clippy_lints/src/casts/cast_abs_to_unsigned.rs @@ -0,0 +1,42 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::sugg::Sugg; +use clippy_utils::{meets_msrv, msrvs}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; +use rustc_semver::RustcVersion; + +use super::CAST_ABS_TO_UNSIGNED; + +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + cast_expr: &Expr<'_>, + cast_from: Ty<'_>, + cast_to: Ty<'_>, + msrv: &Option, +) { + if_chain! { + if meets_msrv(msrv.as_ref(), &msrvs::UNSIGNED_ABS); + if cast_from.is_integral(); + if cast_to.is_integral(); + if cast_from.is_signed(); + if !cast_to.is_signed(); + if let ExprKind::MethodCall(method_path, args, _) = cast_expr.kind; + if let method_name = method_path.ident.name.as_str(); + if method_name == "abs"; + then { + span_lint_and_sugg( + cx, + CAST_ABS_TO_UNSIGNED, + expr.span, + &format!("casting the result of `{}::{}()` to {}", cast_from, method_name, cast_to), + "replace with", + format!("{}.unsigned_abs()", Sugg::hir(cx, &args[0], "..")), + Applicability::MachineApplicable, + ); + } + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index be59145afa0..55c1f085657 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -1,3 +1,4 @@ +mod cast_abs_to_unsigned; mod cast_enum_constructor; mod cast_lossless; mod cast_possible_truncation; @@ -473,6 +474,28 @@ declare_clippy_lint! { "casts from an enum tuple constructor to an integer" } +declare_clippy_lint! { + /// ### What it does + /// Checks for uses of the `abs()` method that cast the result to unsigned. + /// + /// ### Why is this bad? + /// The `unsigned_abs()` method avoids panic when called on the MIN value. + /// + /// ### Example + /// ```rust + /// let x: i32 = -42; + /// let y: u32 = x.abs() as u32; + /// ``` + /// Use instead: + /// let x: i32 = -42; + /// let y: u32 = x.unsigned_abs(); + /// ``` + #[clippy::version = "1.61.0"] + pub CAST_ABS_TO_UNSIGNED, + suspicious, + "casting the result of `abs()` to an unsigned integer can panic" +} + pub struct Casts { msrv: Option, } @@ -500,7 +523,8 @@ impl_lint_pass!(Casts => [ CHAR_LIT_AS_U8, PTR_AS_PTR, CAST_ENUM_TRUNCATION, - CAST_ENUM_CONSTRUCTOR + CAST_ENUM_CONSTRUCTOR, + CAST_ABS_TO_UNSIGNED ]); impl<'tcx> LateLintPass<'tcx> for Casts { @@ -536,6 +560,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts { cast_possible_wrap::check(cx, expr, cast_from, cast_to); cast_precision_loss::check(cx, expr, cast_from, cast_to); cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); + cast_abs_to_unsigned::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv); } cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv); cast_enum_constructor::check(cx, expr, cast_expr, cast_from); diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index b2f674e9c88..3d092ac87a4 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -23,6 +23,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), LintId::of(booleans::LOGIC_BUG), LintId::of(booleans::NONMINIMAL_BOOL), + LintId::of(casts::CAST_ABS_TO_UNSIGNED), LintId::of(casts::CAST_ENUM_CONSTRUCTOR), LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(casts::CAST_REF_TO_MUT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index fb6612f5c32..1aa98a6ec98 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -70,6 +70,7 @@ store.register_lints(&[ cargo::REDUNDANT_FEATURE_NAMES, cargo::WILDCARD_DEPENDENCIES, case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS, + casts::CAST_ABS_TO_UNSIGNED, casts::CAST_ENUM_CONSTRUCTOR, casts::CAST_ENUM_TRUNCATION, casts::CAST_LOSSLESS, diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 4c3fbb6c4a8..82f45b5fd58 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -7,6 +7,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK), LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF), + LintId::of(casts::CAST_ABS_TO_UNSIGNED), LintId::of(casts::CAST_ENUM_CONSTRUCTOR), LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF), diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 0bb818e2079..e149a201b7a 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -156,7 +156,7 @@ define_Conf! { /// /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), - /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, EXPECT_ERR. + /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, EXPECT_ERR, CAST_ABS_TO_UNSIGNED. /// /// The minimum rust version that the project supports (msrv: Option = None), diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 12191109b8c..0424e067202 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -14,7 +14,7 @@ macro_rules! msrv_aliases { msrv_aliases! { 1,53,0 { OR_PATTERNS, MANUAL_BITS } 1,52,0 { STR_SPLIT_ONCE } - 1,51,0 { BORROW_AS_PTR } + 1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS } 1,50,0 { BOOL_THEN } 1,47,0 { TAU } 1,46,0 { CONST_IF_MATCH } diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 2e31ad3172e..cf85a5ca931 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -7,7 +7,7 @@ clippy::cast_sign_loss, clippy::cast_possible_wrap )] -#[allow(clippy::no_effect, clippy::unnecessary_operation)] +#[allow(clippy::cast_abs_to_unsigned, clippy::no_effect, clippy::unnecessary_operation)] fn main() { // Test clippy::cast_precision_loss let x0 = 1i32; diff --git a/tests/ui/cast_abs_to_unsigned.fixed b/tests/ui/cast_abs_to_unsigned.fixed new file mode 100644 index 00000000000..4ec2465be06 --- /dev/null +++ b/tests/ui/cast_abs_to_unsigned.fixed @@ -0,0 +1,8 @@ +// run-rustfix +#![warn(clippy::cast_abs_to_unsigned)] + +fn main() { + let x: i32 = -42; + let y: u32 = x.unsigned_abs(); + println!("The absolute value of {} is {}", x, y); +} diff --git a/tests/ui/cast_abs_to_unsigned.rs b/tests/ui/cast_abs_to_unsigned.rs new file mode 100644 index 00000000000..59b9c8c3678 --- /dev/null +++ b/tests/ui/cast_abs_to_unsigned.rs @@ -0,0 +1,8 @@ +// run-rustfix +#![warn(clippy::cast_abs_to_unsigned)] + +fn main() { + let x: i32 = -42; + let y: u32 = x.abs() as u32; + println!("The absolute value of {} is {}", x, y); +} diff --git a/tests/ui/cast_abs_to_unsigned.stderr b/tests/ui/cast_abs_to_unsigned.stderr new file mode 100644 index 00000000000..eb12857357a --- /dev/null +++ b/tests/ui/cast_abs_to_unsigned.stderr @@ -0,0 +1,10 @@ +error: casting the result of `i32::abs()` to u32 + --> $DIR/cast_abs_to_unsigned.rs:6:18 + | +LL | let y: u32 = x.abs() as u32; + | ^^^^^^^^^^^^^^ help: replace with: `x.unsigned_abs()` + | + = note: `-D clippy::cast-abs-to-unsigned` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/min_rust_version_attr.rs b/tests/ui/min_rust_version_attr.rs index 7666d01ffe1..f83c3e0e281 100644 --- a/tests/ui/min_rust_version_attr.rs +++ b/tests/ui/min_rust_version_attr.rs @@ -150,6 +150,11 @@ fn err_expect() { x.err().expect("Testing expect_err"); } +fn cast_abs_to_unsigned() { + let x: i32 = 10; + assert_eq!(10u32, x.abs() as u32); +} + fn main() { filter_map_next(); checked_conversion(); @@ -168,6 +173,7 @@ fn main() { unnest_or_patterns(); int_from_bool(); err_expect(); + cast_abs_to_unsigned(); } mod just_under_msrv { diff --git a/tests/ui/min_rust_version_attr.stderr b/tests/ui/min_rust_version_attr.stderr index 9ed6308f115..de225eb740d 100644 --- a/tests/ui/min_rust_version_attr.stderr +++ b/tests/ui/min_rust_version_attr.stderr @@ -1,12 +1,12 @@ error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:192:24 + --> $DIR/min_rust_version_attr.rs:198:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-strip` implied by `-D warnings` note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:191:9 + --> $DIR/min_rust_version_attr.rs:197:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,13 +17,13 @@ LL ~ assert_eq!(.to_uppercase(), "WORLD!"); | error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:204:24 + --> $DIR/min_rust_version_attr.rs:210:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:203:9 + --> $DIR/min_rust_version_attr.rs:209:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^