From a342f52f9169af747f81b27505e5759511db4667 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Mon, 23 May 2022 12:19:25 +0000 Subject: [PATCH] cast_abs_to_unsigned: do not remove cast if it's required --- .../src/casts/cast_abs_to_unsigned.rs | 46 +++++----- tests/ui/cast_abs_to_unsigned.fixed | 21 +++++ tests/ui/cast_abs_to_unsigned.rs | 21 +++++ tests/ui/cast_abs_to_unsigned.stderr | 92 ++++++++++++++++++- 4 files changed, 157 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/casts/cast_abs_to_unsigned.rs b/clippy_lints/src/casts/cast_abs_to_unsigned.rs index 6bac6bf83..64ea326b7 100644 --- a/clippy_lints/src/casts/cast_abs_to_unsigned.rs +++ b/clippy_lints/src/casts/cast_abs_to_unsigned.rs @@ -1,11 +1,10 @@ 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_middle::ty::{self, Ty}; use rustc_semver::RustcVersion; use super::CAST_ABS_TO_UNSIGNED; @@ -18,25 +17,28 @@ pub(super) fn check( cast_to: Ty<'_>, msrv: Option, ) { - if_chain! { - if meets_msrv(msrv, 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, - ); - } + if meets_msrv(msrv, msrvs::UNSIGNED_ABS) + && let ty::Int(from) = cast_from.kind() + && let ty::Uint(to) = cast_to.kind() + && let ExprKind::MethodCall(method_path, args, _) = cast_expr.kind + && method_path.ident.name.as_str() == "abs" + { + let span = if from.bit_width() == to.bit_width() { + expr.span + } else { + // if the result of `.unsigned_abs` would be a different type, keep the cast + // e.g. `i64 -> usize`, `i16 -> u8` + cast_expr.span + }; + + span_lint_and_sugg( + cx, + CAST_ABS_TO_UNSIGNED, + span, + &format!("casting the result of `{cast_from}::abs()` to {cast_to}"), + "replace with", + format!("{}.unsigned_abs()", Sugg::hir(cx, &args[0], "..")), + Applicability::MachineApplicable, + ); } } diff --git a/tests/ui/cast_abs_to_unsigned.fixed b/tests/ui/cast_abs_to_unsigned.fixed index 4ec2465be..a68b32b09 100644 --- a/tests/ui/cast_abs_to_unsigned.fixed +++ b/tests/ui/cast_abs_to_unsigned.fixed @@ -5,4 +5,25 @@ fn main() { let x: i32 = -42; let y: u32 = x.unsigned_abs(); println!("The absolute value of {} is {}", x, y); + + let a: i32 = -3; + let _: usize = a.unsigned_abs() as usize; + let _: usize = a.unsigned_abs() as _; + let _ = a.unsigned_abs() as usize; + + let a: i64 = -3; + let _ = a.unsigned_abs() as usize; + let _ = a.unsigned_abs() as u8; + let _ = a.unsigned_abs() as u16; + let _ = a.unsigned_abs() as u32; + let _ = a.unsigned_abs(); + let _ = a.unsigned_abs() as u128; + + let a: isize = -3; + let _ = a.unsigned_abs(); + let _ = a.unsigned_abs() as u8; + let _ = a.unsigned_abs() as u16; + let _ = a.unsigned_abs() as u32; + let _ = a.unsigned_abs() as u64; + let _ = a.unsigned_abs() as u128; } diff --git a/tests/ui/cast_abs_to_unsigned.rs b/tests/ui/cast_abs_to_unsigned.rs index 59b9c8c36..110fbc6c2 100644 --- a/tests/ui/cast_abs_to_unsigned.rs +++ b/tests/ui/cast_abs_to_unsigned.rs @@ -5,4 +5,25 @@ fn main() { let x: i32 = -42; let y: u32 = x.abs() as u32; println!("The absolute value of {} is {}", x, y); + + let a: i32 = -3; + let _: usize = a.abs() as usize; + let _: usize = a.abs() as _; + let _ = a.abs() as usize; + + let a: i64 = -3; + let _ = a.abs() as usize; + let _ = a.abs() as u8; + let _ = a.abs() as u16; + let _ = a.abs() as u32; + let _ = a.abs() as u64; + let _ = a.abs() as u128; + + let a: isize = -3; + let _ = a.abs() as usize; + let _ = a.abs() as u8; + let _ = a.abs() as u16; + let _ = a.abs() as u32; + let _ = a.abs() as u64; + let _ = a.abs() as u128; } diff --git a/tests/ui/cast_abs_to_unsigned.stderr b/tests/ui/cast_abs_to_unsigned.stderr index eb1285735..02c24e106 100644 --- a/tests/ui/cast_abs_to_unsigned.stderr +++ b/tests/ui/cast_abs_to_unsigned.stderr @@ -6,5 +6,95 @@ LL | let y: u32 = x.abs() as u32; | = note: `-D clippy::cast-abs-to-unsigned` implied by `-D warnings` -error: aborting due to previous error +error: casting the result of `i32::abs()` to usize + --> $DIR/cast_abs_to_unsigned.rs:10:20 + | +LL | let _: usize = a.abs() as usize; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i32::abs()` to usize + --> $DIR/cast_abs_to_unsigned.rs:11:20 + | +LL | let _: usize = a.abs() as _; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i32::abs()` to usize + --> $DIR/cast_abs_to_unsigned.rs:12:13 + | +LL | let _ = a.abs() as usize; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i64::abs()` to usize + --> $DIR/cast_abs_to_unsigned.rs:15:13 + | +LL | let _ = a.abs() as usize; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i64::abs()` to u8 + --> $DIR/cast_abs_to_unsigned.rs:16:13 + | +LL | let _ = a.abs() as u8; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i64::abs()` to u16 + --> $DIR/cast_abs_to_unsigned.rs:17:13 + | +LL | let _ = a.abs() as u16; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i64::abs()` to u32 + --> $DIR/cast_abs_to_unsigned.rs:18:13 + | +LL | let _ = a.abs() as u32; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i64::abs()` to u64 + --> $DIR/cast_abs_to_unsigned.rs:19:13 + | +LL | let _ = a.abs() as u64; + | ^^^^^^^^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `i64::abs()` to u128 + --> $DIR/cast_abs_to_unsigned.rs:20:13 + | +LL | let _ = a.abs() as u128; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `isize::abs()` to usize + --> $DIR/cast_abs_to_unsigned.rs:23:13 + | +LL | let _ = a.abs() as usize; + | ^^^^^^^^^^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `isize::abs()` to u8 + --> $DIR/cast_abs_to_unsigned.rs:24:13 + | +LL | let _ = a.abs() as u8; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `isize::abs()` to u16 + --> $DIR/cast_abs_to_unsigned.rs:25:13 + | +LL | let _ = a.abs() as u16; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `isize::abs()` to u32 + --> $DIR/cast_abs_to_unsigned.rs:26:13 + | +LL | let _ = a.abs() as u32; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `isize::abs()` to u64 + --> $DIR/cast_abs_to_unsigned.rs:27:13 + | +LL | let _ = a.abs() as u64; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: casting the result of `isize::abs()` to u128 + --> $DIR/cast_abs_to_unsigned.rs:28:13 + | +LL | let _ = a.abs() as u128; + | ^^^^^^^ help: replace with: `a.unsigned_abs()` + +error: aborting due to 16 previous errors