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.
This commit is contained in:
Paolo Borelli 2022-04-04 18:38:38 +02:00
parent 0d66404941
commit b0edbca0e6
14 changed files with 111 additions and 8 deletions

View File

@ -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

View File

@ -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<RustcVersion>,
) {
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,
);
}
}
}

View File

@ -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<RustcVersion>,
}
@ -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);

View File

@ -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),

View File

@ -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,

View File

@ -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),

View File

@ -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<String> = None),

View File

@ -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 }

View File

@ -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;

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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

View File

@ -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 {

View File

@ -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!(<stripped>.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, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^