Skip to content

Commit

Permalink
Auto merge of #11287 - Centri3:#11285, r=llogiq
Browse files Browse the repository at this point in the history
[`cast_lossless`]: Suggest type alias instead of the aliased type

Fixes #11285

Still an issue with the "from" side, i.e., `I8::from(1) as I64` shows as `i8 to I64`, but this should be ok. Not possible to reliably fix currently anyway.

changelog: [`cast_lossless`]: Suggest type alias instead of the aliased type
  • Loading branch information
bors committed Mar 12, 2024
2 parents 3a14911 + 244d7da commit 92ca7c9
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 58 deletions.
34 changes: 26 additions & 8 deletions clippy_lints/src/casts/cast_lossless.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::in_constant;
use clippy_utils::source::snippet_opt;
use clippy_utils::source::{snippet_opt, snippet_with_applicability};
use clippy_utils::ty::is_isize_or_usize;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_hir::{Expr, ExprKind, QPath, TyKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, FloatTy, Ty};

Expand All @@ -16,6 +16,7 @@ pub(super) fn check(
cast_op: &Expr<'_>,
cast_from: Ty<'_>,
cast_to: Ty<'_>,
cast_to_hir: &rustc_hir::Ty<'_>,
msrv: &Msrv,
) {
if !should_lint(cx, expr, cast_from, cast_to, msrv) {
Expand All @@ -24,11 +25,11 @@ pub(super) fn check(

// The suggestion is to use a function call, so if the original expression
// has parens on the outside, they are no longer needed.
let mut applicability = Applicability::MachineApplicable;
let mut app = Applicability::MachineApplicable;
let opt = snippet_opt(cx, cast_op.span.source_callsite());
let sugg = opt.as_ref().map_or_else(
|| {
applicability = Applicability::HasPlaceholders;
app = Applicability::HasPlaceholders;
".."
},
|snip| {
Expand All @@ -40,10 +41,27 @@ pub(super) fn check(
},
);

// Display the type alias instead of the aliased type. Fixes #11285
//
// FIXME: Once `lazy_type_alias` is stabilized(?) we should use `rustc_middle` types instead,
// this will allow us to display the right type with `cast_from` as well.
let cast_to_fmt = if let TyKind::Path(QPath::Resolved(None, path)) = cast_to_hir.kind
// It's a bit annoying but the turbofish is optional for types. A type in an `as` cast
// shouldn't have these if they're primitives, which are the only things we deal with.
//
// This could be removed for performance if this check is determined to have a pretty major
// effect.
&& path.segments.iter().all(|segment| segment.args.is_none())
{
snippet_with_applicability(cx, cast_to_hir.span, "..", &mut app)
} else {
cast_to.to_string().into()
};

let message = if cast_from.is_bool() {
format!("casting `{cast_from:}` to `{cast_to:}` is more cleanly stated with `{cast_to:}::from(_)`")
format!("casting `{cast_from}` to `{cast_to_fmt}` is more cleanly stated with `{cast_to_fmt}::from(_)`")
} else {
format!("casting `{cast_from}` to `{cast_to}` may become silently lossy if you later change the type")
format!("casting `{cast_from}` to `{cast_to_fmt}` may become silently lossy if you later change the type")
};

span_lint_and_sugg(
Expand All @@ -52,8 +70,8 @@ pub(super) fn check(
expr.span,
&message,
"try",
format!("{cast_to}::from({sugg})"),
applicability,
format!("{cast_to_fmt}::from({sugg})"),
app,
);
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
cast_abs_to_unsigned::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
cast_nan_to_int::check(cx, expr, cast_expr, cast_from, cast_to);
}
cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, cast_to_hir, &self.msrv);
cast_enum_constructor::check(cx, expr, cast_expr, cast_from);
}

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/cast_lossless_bool.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(dead_code)]
#![warn(clippy::cast_lossless)]

type U8 = u8;

fn main() {
// Test clippy::cast_lossless with casts to integer types
let _ = u8::from(true);
Expand All @@ -19,6 +21,8 @@ fn main() {

// Test with an expression wrapped in parens
let _ = u16::from(true | false);

let _ = U8::from(true);
}

// The lint would suggest using `u32::from(input)` here but the `XX::from` function is not const,
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/cast_lossless_bool.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(dead_code)]
#![warn(clippy::cast_lossless)]

type U8 = u8;

fn main() {
// Test clippy::cast_lossless with casts to integer types
let _ = true as u8;
Expand All @@ -19,6 +21,8 @@ fn main() {

// Test with an expression wrapped in parens
let _ = (true | false) as u16;

let _ = true as U8;
}

// The lint would suggest using `u32::from(input)` here but the `XX::from` function is not const,
Expand Down
36 changes: 21 additions & 15 deletions tests/ui/cast_lossless_bool.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: casting `bool` to `u8` is more cleanly stated with `u8::from(_)`
--> tests/ui/cast_lossless_bool.rs:6:13
--> tests/ui/cast_lossless_bool.rs:8:13
|
LL | let _ = true as u8;
| ^^^^^^^^^^ help: try: `u8::from(true)`
Expand All @@ -8,82 +8,88 @@ LL | let _ = true as u8;
= help: to override `-D warnings` add `#[allow(clippy::cast_lossless)]`

error: casting `bool` to `u16` is more cleanly stated with `u16::from(_)`
--> tests/ui/cast_lossless_bool.rs:7:13
--> tests/ui/cast_lossless_bool.rs:9:13
|
LL | let _ = true as u16;
| ^^^^^^^^^^^ help: try: `u16::from(true)`

error: casting `bool` to `u32` is more cleanly stated with `u32::from(_)`
--> tests/ui/cast_lossless_bool.rs:8:13
--> tests/ui/cast_lossless_bool.rs:10:13
|
LL | let _ = true as u32;
| ^^^^^^^^^^^ help: try: `u32::from(true)`

error: casting `bool` to `u64` is more cleanly stated with `u64::from(_)`
--> tests/ui/cast_lossless_bool.rs:9:13
--> tests/ui/cast_lossless_bool.rs:11:13
|
LL | let _ = true as u64;
| ^^^^^^^^^^^ help: try: `u64::from(true)`

error: casting `bool` to `u128` is more cleanly stated with `u128::from(_)`
--> tests/ui/cast_lossless_bool.rs:10:13
--> tests/ui/cast_lossless_bool.rs:12:13
|
LL | let _ = true as u128;
| ^^^^^^^^^^^^ help: try: `u128::from(true)`

error: casting `bool` to `usize` is more cleanly stated with `usize::from(_)`
--> tests/ui/cast_lossless_bool.rs:11:13
--> tests/ui/cast_lossless_bool.rs:13:13
|
LL | let _ = true as usize;
| ^^^^^^^^^^^^^ help: try: `usize::from(true)`

error: casting `bool` to `i8` is more cleanly stated with `i8::from(_)`
--> tests/ui/cast_lossless_bool.rs:13:13
--> tests/ui/cast_lossless_bool.rs:15:13
|
LL | let _ = true as i8;
| ^^^^^^^^^^ help: try: `i8::from(true)`

error: casting `bool` to `i16` is more cleanly stated with `i16::from(_)`
--> tests/ui/cast_lossless_bool.rs:14:13
--> tests/ui/cast_lossless_bool.rs:16:13
|
LL | let _ = true as i16;
| ^^^^^^^^^^^ help: try: `i16::from(true)`

error: casting `bool` to `i32` is more cleanly stated with `i32::from(_)`
--> tests/ui/cast_lossless_bool.rs:15:13
--> tests/ui/cast_lossless_bool.rs:17:13
|
LL | let _ = true as i32;
| ^^^^^^^^^^^ help: try: `i32::from(true)`

error: casting `bool` to `i64` is more cleanly stated with `i64::from(_)`
--> tests/ui/cast_lossless_bool.rs:16:13
--> tests/ui/cast_lossless_bool.rs:18:13
|
LL | let _ = true as i64;
| ^^^^^^^^^^^ help: try: `i64::from(true)`

error: casting `bool` to `i128` is more cleanly stated with `i128::from(_)`
--> tests/ui/cast_lossless_bool.rs:17:13
--> tests/ui/cast_lossless_bool.rs:19:13
|
LL | let _ = true as i128;
| ^^^^^^^^^^^^ help: try: `i128::from(true)`

error: casting `bool` to `isize` is more cleanly stated with `isize::from(_)`
--> tests/ui/cast_lossless_bool.rs:18:13
--> tests/ui/cast_lossless_bool.rs:20:13
|
LL | let _ = true as isize;
| ^^^^^^^^^^^^^ help: try: `isize::from(true)`

error: casting `bool` to `u16` is more cleanly stated with `u16::from(_)`
--> tests/ui/cast_lossless_bool.rs:21:13
--> tests/ui/cast_lossless_bool.rs:23:13
|
LL | let _ = (true | false) as u16;
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::from(true | false)`

error: casting `bool` to `U8` is more cleanly stated with `U8::from(_)`
--> tests/ui/cast_lossless_bool.rs:25:13
|
LL | let _ = true as U8;
| ^^^^^^^^^^ help: try: `U8::from(true)`

error: casting `bool` to `u8` is more cleanly stated with `u8::from(_)`
--> tests/ui/cast_lossless_bool.rs:49:13
--> tests/ui/cast_lossless_bool.rs:53:13
|
LL | let _ = true as u8;
| ^^^^^^^^^^ help: try: `u8::from(true)`

error: aborting due to 14 previous errors
error: aborting due to 15 previous errors

5 changes: 5 additions & 0 deletions tests/ui/cast_lossless_float.fixed
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
#![allow(clippy::no_effect, clippy::unnecessary_operation, dead_code)]
#![warn(clippy::cast_lossless)]

type F32 = f32;
type F64 = f64;

fn main() {
// Test clippy::cast_lossless with casts to floating-point types
let x0 = 1i8;
let _ = f32::from(x0);
let _ = f64::from(x0);
let _ = F32::from(x0);
let _ = F64::from(x0);
let x1 = 1u8;
let _ = f32::from(x1);
let _ = f64::from(x1);
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/cast_lossless_float.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
#![allow(clippy::no_effect, clippy::unnecessary_operation, dead_code)]
#![warn(clippy::cast_lossless)]

type F32 = f32;
type F64 = f64;

fn main() {
// Test clippy::cast_lossless with casts to floating-point types
let x0 = 1i8;
let _ = x0 as f32;
let _ = x0 as f64;
let _ = x0 as F32;
let _ = x0 as F64;
let x1 = 1u8;
let _ = x1 as f32;
let _ = x1 as f64;
Expand Down
36 changes: 24 additions & 12 deletions tests/ui/cast_lossless_float.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: casting `i8` to `f32` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:7:13
--> tests/ui/cast_lossless_float.rs:10:13
|
LL | let _ = x0 as f32;
| ^^^^^^^^^ help: try: `f32::from(x0)`
Expand All @@ -8,64 +8,76 @@ LL | let _ = x0 as f32;
= help: to override `-D warnings` add `#[allow(clippy::cast_lossless)]`

error: casting `i8` to `f64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:8:13
--> tests/ui/cast_lossless_float.rs:11:13
|
LL | let _ = x0 as f64;
| ^^^^^^^^^ help: try: `f64::from(x0)`

error: casting `i8` to `F32` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:12:13
|
LL | let _ = x0 as F32;
| ^^^^^^^^^ help: try: `F32::from(x0)`

error: casting `i8` to `F64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:13:13
|
LL | let _ = x0 as F64;
| ^^^^^^^^^ help: try: `F64::from(x0)`

error: casting `u8` to `f32` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:10:13
--> tests/ui/cast_lossless_float.rs:15:13
|
LL | let _ = x1 as f32;
| ^^^^^^^^^ help: try: `f32::from(x1)`

error: casting `u8` to `f64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:11:13
--> tests/ui/cast_lossless_float.rs:16:13
|
LL | let _ = x1 as f64;
| ^^^^^^^^^ help: try: `f64::from(x1)`

error: casting `i16` to `f32` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:13:13
--> tests/ui/cast_lossless_float.rs:18:13
|
LL | let _ = x2 as f32;
| ^^^^^^^^^ help: try: `f32::from(x2)`

error: casting `i16` to `f64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:14:13
--> tests/ui/cast_lossless_float.rs:19:13
|
LL | let _ = x2 as f64;
| ^^^^^^^^^ help: try: `f64::from(x2)`

error: casting `u16` to `f32` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:16:13
--> tests/ui/cast_lossless_float.rs:21:13
|
LL | let _ = x3 as f32;
| ^^^^^^^^^ help: try: `f32::from(x3)`

error: casting `u16` to `f64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:17:13
--> tests/ui/cast_lossless_float.rs:22:13
|
LL | let _ = x3 as f64;
| ^^^^^^^^^ help: try: `f64::from(x3)`

error: casting `i32` to `f64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:19:13
--> tests/ui/cast_lossless_float.rs:24:13
|
LL | let _ = x4 as f64;
| ^^^^^^^^^ help: try: `f64::from(x4)`

error: casting `u32` to `f64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:21:13
--> tests/ui/cast_lossless_float.rs:26:13
|
LL | let _ = x5 as f64;
| ^^^^^^^^^ help: try: `f64::from(x5)`

error: casting `f32` to `f64` may become silently lossy if you later change the type
--> tests/ui/cast_lossless_float.rs:24:13
--> tests/ui/cast_lossless_float.rs:29:13
|
LL | let _ = 1.0f32 as f64;
| ^^^^^^^^^^^^^ help: try: `f64::from(1.0f32)`

error: aborting due to 11 previous errors
error: aborting due to 13 previous errors

4 changes: 4 additions & 0 deletions tests/ui/cast_lossless_integer.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(clippy::no_effect, clippy::unnecessary_operation, dead_code)]
#![warn(clippy::cast_lossless)]

type I64 = i64;

fn main() {
// Test clippy::cast_lossless with casts to integer types
let _ = i16::from(1i8);
Expand All @@ -24,6 +26,8 @@ fn main() {

// Test with an expression wrapped in parens
let _ = u16::from(1u8 + 1u8);

let _ = I64::from(1i8);
}

// The lint would suggest using `f64::from(input)` here but the `XX::from` function is not const,
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/cast_lossless_integer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(clippy::no_effect, clippy::unnecessary_operation, dead_code)]
#![warn(clippy::cast_lossless)]

type I64 = i64;

fn main() {
// Test clippy::cast_lossless with casts to integer types
let _ = 1i8 as i16;
Expand All @@ -24,6 +26,8 @@ fn main() {

// Test with an expression wrapped in parens
let _ = (1u8 + 1u8) as u16;

let _ = 1i8 as I64;
}

// The lint would suggest using `f64::from(input)` here but the `XX::from` function is not const,
Expand Down
Loading

0 comments on commit 92ca7c9

Please sign in to comment.