Skip to content
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

Issue 9231: add cast_integer lint #11955

Closed
wants to merge 6 commits into from
Closed
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4968,6 +4968,7 @@ Released 2018-09-13
[`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_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_integer
[`cast_lossless`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless
[`cast_nan_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_nan_to_int
[`cast_possible_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation
Expand Down
40 changes: 40 additions & 0 deletions clippy_lints/src/casts/cast_integer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::Ty;
use clippy_utils::in_constant;

use super::CAST_INTEGER;

pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
from_ty: Ty<'_>,
to_ty: Ty<'_>
) {

if !should_lint(cx, expr, from_ty, to_ty) {
return;
}

span_lint_and_help(
cx,
CAST_INTEGER,
expr.span,
"Integer casts can introduce subtle surprises and should be done with From/TryFrom.",
None,
"Try T::from(_) or T::try_from(_) instead",
)
}

fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool {
// Do not suggest using From in consts/statics until it is valid to do so (see #2267).
if in_constant(cx, expr.hir_id) {
return false;
}

match (cast_from.is_integral(), cast_to.is_integral()) {
(true, true) => true,
(_, _) => false,
}
}
26 changes: 26 additions & 0 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mod as_underscore;
mod borrow_as_ptr;
mod cast_abs_to_unsigned;
mod cast_enum_constructor;
mod cast_integer;
mod cast_lossless;
mod cast_nan_to_int;
mod cast_possible_truncation;
Expand Down Expand Up @@ -689,6 +690,29 @@ declare_clippy_lint! {
"using `0 as *{const, mut} T`"
}

declare_clippy_lint! {
/// ### What it does
/// Trigger on every integer cast using `as`, recommending From/TryFrom instead
///
/// ### Why is this bad?
/// Because integer casts might introduce subtle unintended changes
///
/// ### Example
/// ```no_run
/// let u = 1usize;
/// let _ = u as u64;
/// ```
/// Use instead:
/// ```no_run
/// let u = 1usize;
/// let _ = u64::try_from(u);
/// ```
#[clippy::version = "1.75.0"]
pub CAST_INTEGER,
pedantic,
"casting integer types using `as` instead of From/TryFrom"
}

pub struct Casts {
msrv: Msrv,
}
Expand Down Expand Up @@ -724,6 +748,7 @@ impl_lint_pass!(Casts => [
AS_PTR_CAST_MUT,
CAST_NAN_TO_INT,
ZERO_PTR,
CAST_INTEGER,
]);

impl<'tcx> LateLintPass<'tcx> for Casts {
Expand Down Expand Up @@ -764,6 +789,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
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_nan_to_int::check(cx, expr, cast_expr, cast_from, cast_to);
cast_integer::check(cx, expr, cast_from, cast_to);
}
cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
cast_enum_constructor::check(cx, expr, cast_expr, cast_from);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::casts::CAST_ABS_TO_UNSIGNED_INFO,
crate::casts::CAST_ENUM_CONSTRUCTOR_INFO,
crate::casts::CAST_ENUM_TRUNCATION_INFO,
crate::casts::CAST_INTEGER_INFO,
crate::casts::CAST_LOSSLESS_INFO,
crate::casts::CAST_NAN_TO_INT_INFO,
crate::casts::CAST_POSSIBLE_TRUNCATION_INFO,
Expand Down
90 changes: 90 additions & 0 deletions tests/ui/cast_integer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
#![warn(clippy::cast_integer)]

fn main() {
let u = 1usize;
let _ = u as u64;
let _ = u64::try_from(u);

// unsigned
let u64= 1u64;
let _ = u64 as u128;
let _ = u128::from(u64);

let u32 = 1u32;
let _ = u32 as u128;
let _ = u128::from(u32);
let _ = u32 as u64;
let _ = u64::from(u32);

let u16= 1u16;
let _ = u16 as u128;
let _ = u128::from(u16);
let _ = u16 as u64;
let _ = u64::from(u16);
let _ = u16 as u32;
let _ = u32::from(u16);

let u8 = 1u8;
let _ = u8 as u128;
let _ = u128::from(u8);
let _ = u8 as u64;
let _ = u64::from(u8);
let _ = u8 as u32;
let _ = u32::from(u8);
let _ = u8 as u16;
let _ = u16::from(u8);

// signed

let isize= 1isize;
let _ = isize as i64;
let _ = i64::try_from(isize);

let i64 = 1i64;
let _ = i64 as i128;
let _ = i128::from(i64);

let i32 = 1i32;
let _ = i32 as i128;
let _ = i128::from(i32);
let _ = i32 as i64;
let _ = i64::from(i32);

let i16 = 1i16;
let _ = i16 as i128;
let _ = i128::from(i16);
let _ = i16 as i64;
let _ = i64::from(i16);
let _ = i16 as i32;
let _ = i32::from(i16);

let i8 = 1i8;
let _ = i8 as i128;
let _ = i128::from(i8);
let _ = i8 as i64;
let _ = i64::from(i8);
let _ = i8 as i32;
let _ = i32::from(i8);
let _ = i8 as i16;
let _ = i16::from(i8);
}

pub fn non_const_function(x: usize) -> u64 { x as u64 }

// The lint would suggest using `u32::from(input)` here but the `XX::from` function is not const,
// so we skip the lint if the expression is in a const fn.
// See #3656
const fn const_function(input: u32) -> u64 {
input as u64
}

// Same as the above issue. We can't suggest `::from` in const fns in impls
mod cast_integer_in_impl {
struct A;

impl A {
pub const fn convert(x: u32) -> u64 {
x as u64
}
}
}
188 changes: 188 additions & 0 deletions tests/ui/cast_integer.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:5:13
|
LL | let _ = u as u64;
| ^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead
= note: `-D clippy::cast-integer` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::cast_integer)]`

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:10:13
|
LL | let _ = u64 as u128;
| ^^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:14:13
|
LL | let _ = u32 as u128;
| ^^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:16:13
|
LL | let _ = u32 as u64;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:20:13
|
LL | let _ = u16 as u128;
| ^^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:22:13
|
LL | let _ = u16 as u64;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:24:13
|
LL | let _ = u16 as u32;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:28:13
|
LL | let _ = u8 as u128;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:30:13
|
LL | let _ = u8 as u64;
| ^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:32:13
|
LL | let _ = u8 as u32;
| ^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:34:13
|
LL | let _ = u8 as u16;
| ^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:40:13
|
LL | let _ = isize as i64;
| ^^^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:44:13
|
LL | let _ = i64 as i128;
| ^^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:48:13
|
LL | let _ = i32 as i128;
| ^^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:50:13
|
LL | let _ = i32 as i64;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:54:13
|
LL | let _ = i16 as i128;
| ^^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:56:13
|
LL | let _ = i16 as i64;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:58:13
|
LL | let _ = i16 as i32;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:62:13
|
LL | let _ = i8 as i128;
| ^^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:64:13
|
LL | let _ = i8 as i64;
| ^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:66:13
|
LL | let _ = i8 as i32;
| ^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:68:13
|
LL | let _ = i8 as i16;
| ^^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: Integer casts can introduce subtle surprises and should be done with From/TryFrom.
--> $DIR/cast_integer.rs:72:46
|
LL | pub fn non_const_function(x: usize) -> u64 { x as u64 }
| ^^^^^^^^
|
= help: Try T::from(_) or T::try_from(_) instead

error: aborting due to 23 previous errors

Loading