Skip to content

fix [cast_possible_truncation] offering wrong suggestion for casting float to integer #10496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 29 additions & 13 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::expr_or_init;
use clippy_utils::source::snippet;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{get_discriminant_value, is_isize_or_usize};
use rustc_errors::{Applicability, SuggestionStyle};
use rustc_errors::{Applicability, Diagnostic, SuggestionStyle};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
Expand Down Expand Up @@ -163,19 +164,34 @@ pub(super) fn check(
_ => return,
};

let name_of_cast_from = snippet(cx, cast_expr.span, "..");
let cast_to_snip = snippet(cx, cast_to_span, "..");
let suggestion = format!("{cast_to_snip}::try_from({name_of_cast_from})");

span_lint_and_then(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg, |diag| {
diag.help("if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...");
diag.span_suggestion_with_style(
expr.span,
"... or use `try_from` and handle the error accordingly",
suggestion,
Applicability::Unspecified,
// always show the suggestion in a separate line
SuggestionStyle::ShowAlways,
);
if !cast_from.is_floating_point() {
offer_suggestion(cx, expr, cast_expr, cast_to_span, diag);
}
});
}

fn offer_suggestion(
cx: &LateContext<'_>,
expr: &Expr<'_>,
cast_expr: &Expr<'_>,
cast_to_span: Span,
diag: &mut Diagnostic,
) {
let cast_to_snip = snippet(cx, cast_to_span, "..");
let suggestion = if cast_to_snip == "_" {
format!("{}.try_into()", Sugg::hir(cx, cast_expr, "..").maybe_par())
} else {
format!("{cast_to_snip}::try_from({})", Sugg::hir(cx, cast_expr, ".."))
};

diag.span_suggestion_with_style(
expr.span,
"... or use `try_from` and handle the error accordingly",
suggestion,
Applicability::Unspecified,
// always show the suggestion in a separate line
SuggestionStyle::ShowAlways,
);
}
6 changes: 6 additions & 0 deletions tests/ui/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ fn main() {
1f64 as isize;
1f64 as usize;
1f32 as u32 as u16;
{
let _x: i8 = 1i32 as _;
1f32 as i32;
1f64 as i32;
1f32 as u8;
}
// Test clippy::cast_possible_wrap
1u8 as i8;
1u16 as i16;
Expand Down
112 changes: 65 additions & 47 deletions tests/ui/cast.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ LL | 1f32 as i32;
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
= note: `-D clippy::cast-possible-truncation` implied by `-D warnings`
help: ... or use `try_from` and handle the error accordingly
|
LL | i32::try_from(1f32);
| ~~~~~~~~~~~~~~~~~~~

error: casting `f32` to `u32` may truncate the value
--> $DIR/cast.rs:25:5
Expand All @@ -56,10 +52,6 @@ LL | 1f32 as u32;
| ^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | u32::try_from(1f32);
| ~~~~~~~~~~~~~~~~~~~

error: casting `f32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:25:5
Expand All @@ -76,10 +68,6 @@ LL | 1f64 as f32;
| ^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | f32::try_from(1f64);
| ~~~~~~~~~~~~~~~~~~~

error: casting `i32` to `i8` may truncate the value
--> $DIR/cast.rs:27:5
Expand Down Expand Up @@ -112,10 +100,6 @@ LL | 1f64 as isize;
| ^^^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | isize::try_from(1f64);
| ~~~~~~~~~~~~~~~~~~~~~

error: casting `f64` to `usize` may truncate the value
--> $DIR/cast.rs:30:5
Expand All @@ -124,10 +108,6 @@ LL | 1f64 as usize;
| ^^^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | usize::try_from(1f64);
| ~~~~~~~~~~~~~~~~~~~~~

error: casting `f64` to `usize` may lose the sign of the value
--> $DIR/cast.rs:30:5
Expand All @@ -154,63 +134,101 @@ LL | 1f32 as u32 as u16;
| ^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | u32::try_from(1f32) as u16;
| ~~~~~~~~~~~~~~~~~~~

error: casting `f32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:31:5
|
LL | 1f32 as u32 as u16;
| ^^^^^^^^^^^

error: casting `i32` to `i8` may truncate the value
--> $DIR/cast.rs:33:22
|
LL | let _x: i8 = 1i32 as _;
| ^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | let _x: i8 = 1i32.try_into();
| ~~~~~~~~~~~~~~~

error: casting `f32` to `i32` may truncate the value
--> $DIR/cast.rs:34:9
|
LL | 1f32 as i32;
| ^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...

error: casting `f64` to `i32` may truncate the value
--> $DIR/cast.rs:35:9
|
LL | 1f64 as i32;
| ^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...

error: casting `f32` to `u8` may truncate the value
--> $DIR/cast.rs:36:9
|
LL | 1f32 as u8;
| ^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...

error: casting `f32` to `u8` may lose the sign of the value
--> $DIR/cast.rs:36:9
|
LL | 1f32 as u8;
| ^^^^^^^^^^

error: casting `u8` to `i8` may wrap around the value
--> $DIR/cast.rs:33:5
--> $DIR/cast.rs:39:5
|
LL | 1u8 as i8;
| ^^^^^^^^^
|
= note: `-D clippy::cast-possible-wrap` implied by `-D warnings`

error: casting `u16` to `i16` may wrap around the value
--> $DIR/cast.rs:34:5
--> $DIR/cast.rs:40:5
|
LL | 1u16 as i16;
| ^^^^^^^^^^^

error: casting `u32` to `i32` may wrap around the value
--> $DIR/cast.rs:35:5
--> $DIR/cast.rs:41:5
|
LL | 1u32 as i32;
| ^^^^^^^^^^^

error: casting `u64` to `i64` may wrap around the value
--> $DIR/cast.rs:36:5
--> $DIR/cast.rs:42:5
|
LL | 1u64 as i64;
| ^^^^^^^^^^^

error: casting `usize` to `isize` may wrap around the value
--> $DIR/cast.rs:37:5
--> $DIR/cast.rs:43:5
|
LL | 1usize as isize;
| ^^^^^^^^^^^^^^^

error: casting `i32` to `u32` may lose the sign of the value
--> $DIR/cast.rs:40:5
--> $DIR/cast.rs:46:5
|
LL | -1i32 as u32;
| ^^^^^^^^^^^^

error: casting `isize` to `usize` may lose the sign of the value
--> $DIR/cast.rs:42:5
--> $DIR/cast.rs:48:5
|
LL | -1isize as usize;
| ^^^^^^^^^^^^^^^^

error: casting `i64` to `i8` may truncate the value
--> $DIR/cast.rs:109:5
--> $DIR/cast.rs:115:5
|
LL | (-99999999999i64).min(1) as i8; // should be linted because signed
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -222,7 +240,7 @@ LL | i8::try_from((-99999999999i64).min(1)); // should be linted because sig
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: casting `u64` to `u8` may truncate the value
--> $DIR/cast.rs:121:5
--> $DIR/cast.rs:127:5
|
LL | 999999u64.clamp(0, 256) as u8; // should still be linted
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -234,7 +252,7 @@ LL | u8::try_from(999999u64.clamp(0, 256)); // should still be linted
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: casting `main::E2` to `u8` may truncate the value
--> $DIR/cast.rs:142:21
--> $DIR/cast.rs:148:21
|
LL | let _ = self as u8;
| ^^^^^^^^^^
Expand All @@ -246,15 +264,15 @@ LL | let _ = u8::try_from(self);
| ~~~~~~~~~~~~~~~~~~

error: casting `main::E2::B` to `u8` will truncate the value
--> $DIR/cast.rs:143:21
--> $DIR/cast.rs:149:21
|
LL | let _ = Self::B as u8;
| ^^^^^^^^^^^^^
|
= note: `-D clippy::cast-enum-truncation` implied by `-D warnings`

error: casting `main::E5` to `i8` may truncate the value
--> $DIR/cast.rs:179:21
--> $DIR/cast.rs:185:21
|
LL | let _ = self as i8;
| ^^^^^^^^^^
Expand All @@ -266,13 +284,13 @@ LL | let _ = i8::try_from(self);
| ~~~~~~~~~~~~~~~~~~

error: casting `main::E5::A` to `i8` will truncate the value
--> $DIR/cast.rs:180:21
--> $DIR/cast.rs:186:21
|
LL | let _ = Self::A as i8;
| ^^^^^^^^^^^^^

error: casting `main::E6` to `i16` may truncate the value
--> $DIR/cast.rs:194:21
--> $DIR/cast.rs:200:21
|
LL | let _ = self as i16;
| ^^^^^^^^^^^
Expand All @@ -284,7 +302,7 @@ LL | let _ = i16::try_from(self);
| ~~~~~~~~~~~~~~~~~~~

error: casting `main::E7` to `usize` may truncate the value on targets with 32-bit wide pointers
--> $DIR/cast.rs:209:21
--> $DIR/cast.rs:215:21
|
LL | let _ = self as usize;
| ^^^^^^^^^^^^^
Expand All @@ -296,7 +314,7 @@ LL | let _ = usize::try_from(self);
| ~~~~~~~~~~~~~~~~~~~~~

error: casting `main::E10` to `u16` may truncate the value
--> $DIR/cast.rs:250:21
--> $DIR/cast.rs:256:21
|
LL | let _ = self as u16;
| ^^^^^^^^^^^
Expand All @@ -308,28 +326,28 @@ LL | let _ = u16::try_from(self);
| ~~~~~~~~~~~~~~~~~~~

error: casting `u32` to `u8` may truncate the value
--> $DIR/cast.rs:258:13
--> $DIR/cast.rs:264:13
|
LL | let c = (q >> 16) as u8;
| ^^^^^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | let c = u8::try_from((q >> 16));
| ~~~~~~~~~~~~~~~~~~~~~~~
LL | let c = u8::try_from(q >> 16);
| ~~~~~~~~~~~~~~~~~~~~~

error: casting `u32` to `u8` may truncate the value
--> $DIR/cast.rs:261:13
--> $DIR/cast.rs:267:13
|
LL | let c = (q / 1000) as u8;
| ^^^^^^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | let c = u8::try_from((q / 1000));
| ~~~~~~~~~~~~~~~~~~~~~~~~
LL | let c = u8::try_from(q / 1000);
| ~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 36 previous errors
error: aborting due to 41 previous errors