diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e76c0d0234..19353ab0d530 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/casts/cast_integer.rs b/clippy_lints/src/casts/cast_integer.rs new file mode 100644 index 000000000000..9be510fbad61 --- /dev/null +++ b/clippy_lints/src/casts/cast_integer.rs @@ -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, + } +} \ No newline at end of file diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index e05b8f66d861..bd2a71df8c55 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -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; @@ -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, } @@ -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 { @@ -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); diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a9ae22e8d43a..12a4627db57d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -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, diff --git a/tests/ui/cast_integer.rs b/tests/ui/cast_integer.rs new file mode 100644 index 000000000000..b9a7af9e2bf4 --- /dev/null +++ b/tests/ui/cast_integer.rs @@ -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 + } + } +} \ No newline at end of file diff --git a/tests/ui/cast_integer.stderr b/tests/ui/cast_integer.stderr new file mode 100644 index 000000000000..6e3316d0839d --- /dev/null +++ b/tests/ui/cast_integer.stderr @@ -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 +