diff options
| -rw-r--r-- | clippy_lints/src/bool_assert_comparison.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/casts/cast_lossless.rs | 41 | ||||
| -rw-r--r-- | clippy_lints/src/casts/mod.rs | 16 | ||||
| -rw-r--r-- | clippy_utils/src/msrvs.rs | 1 | ||||
| -rw-r--r-- | clippy_utils/src/source.rs | 2 | ||||
| -rw-r--r-- | tests/ui/cast_lossless_bool.fixed | 42 | ||||
| -rw-r--r-- | tests/ui/cast_lossless_bool.rs | 42 | ||||
| -rw-r--r-- | tests/ui/cast_lossless_bool.stderr | 82 | ||||
| -rw-r--r-- | tests/ui/min_rust_version_attr.rs | 6 | ||||
| -rw-r--r-- | tests/ui/min_rust_version_attr.stderr | 8 |
10 files changed, 221 insertions, 21 deletions
diff --git a/clippy_lints/src/bool_assert_comparison.rs b/clippy_lints/src/bool_assert_comparison.rs index a59abc66306..d0b8c52a36a 100644 --- a/clippy_lints/src/bool_assert_comparison.rs +++ b/clippy_lints/src/bool_assert_comparison.rs @@ -73,7 +73,7 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison { if let Some(span) = is_direct_expn_of(expr.span, mac) { if let Some(args) = higher::extract_assert_macro_args(expr) { if let [a, b, ..] = args[..] { - let nb_bool_args = is_bool_lit(a) as usize + is_bool_lit(b) as usize; + let nb_bool_args = usize::from(is_bool_lit(a)) + usize::from(is_bool_lit(b)); if nb_bool_args != 1 { // If there are two boolean arguments, we definitely don't understand diff --git a/clippy_lints/src/casts/cast_lossless.rs b/clippy_lints/src/casts/cast_lossless.rs index 869deecfbd5..4a95bed1148 100644 --- a/clippy_lints/src/casts/cast_lossless.rs +++ b/clippy_lints/src/casts/cast_lossless.rs @@ -1,16 +1,24 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::in_constant; use clippy_utils::source::snippet_opt; use clippy_utils::ty::is_isize_or_usize; +use clippy_utils::{in_constant, meets_msrv, msrvs}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; use rustc_middle::ty::{self, FloatTy, Ty}; +use rustc_semver::RustcVersion; use super::{utils, CAST_LOSSLESS}; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - if !should_lint(cx, expr, cast_from, cast_to) { +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + cast_op: &Expr<'_>, + cast_from: Ty<'_>, + cast_to: Ty<'_>, + msrv: &Option<RustcVersion>, +) { + if !should_lint(cx, expr, cast_from, cast_to, msrv) { return; } @@ -32,21 +40,36 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, c }, ); + let message = if cast_from.is_bool() { + format!( + "casting `{0:}` to `{1:}` is more cleanly stated with `{1:}::from(_)`", + cast_from, cast_to + ) + } else { + format!( + "casting `{}` to `{}` may become silently lossy if you later change the type", + cast_from, cast_to + ) + }; + span_lint_and_sugg( cx, CAST_LOSSLESS, expr.span, - &format!( - "casting `{}` to `{}` may become silently lossy if you later change the type", - cast_from, cast_to - ), + &message, "try", format!("{}::from({})", cast_to, sugg), applicability, ); } -fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool { +fn should_lint( + cx: &LateContext<'_>, + expr: &Expr<'_>, + cast_from: Ty<'_>, + cast_to: Ty<'_>, + msrv: &Option<RustcVersion>, +) -> 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; @@ -72,7 +95,7 @@ fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to }; from_nbits < to_nbits }, - + (false, true) if matches!(cast_from.kind(), ty::Bool) && meets_msrv(msrv.as_ref(), &msrvs::FROM_BOOL) => true, (_, _) => { matches!(cast_from.kind(), ty::Float(FloatTy::F32)) && matches!(cast_to.kind(), ty::Float(FloatTy::F64)) }, diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index c674327486c..aee1e50b94a 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -439,12 +439,16 @@ impl<'tcx> LateLintPass<'tcx> for Casts { fn_to_numeric_cast_any::check(cx, expr, cast_expr, cast_from, cast_to); fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to); fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to); - if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { - cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to); - cast_possible_wrap::check(cx, expr, cast_from, cast_to); - cast_precision_loss::check(cx, expr, cast_from, cast_to); - cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to); - cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); + + if cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { + if cast_from.is_numeric() { + cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to); + cast_possible_wrap::check(cx, expr, cast_from, cast_to); + cast_precision_loss::check(cx, expr, cast_from, cast_to); + cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); + } + + cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv); } } diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 66d07c9d0e8..0cec7d6a5e4 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -28,6 +28,7 @@ msrv_aliases! { 1,35,0 { OPTION_COPIED, RANGE_CONTAINS } 1,34,0 { TRY_FROM } 1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES } + 1,28,0 { FROM_BOOL } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST } 1,16,0 { STR_REPEAT } } diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 7f68cc388eb..d928317259d 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -128,7 +128,7 @@ pub fn reindent_multiline(s: Cow<'_, str>, ignore_first: bool, indent: Option<us fn reindent_multiline_inner(s: &str, ignore_first: bool, indent: Option<usize>, ch: char) -> String { let x = s .lines() - .skip(ignore_first as usize) + .skip(usize::from(ignore_first)) .filter_map(|l| { if l.is_empty() { None diff --git a/tests/ui/cast_lossless_bool.fixed b/tests/ui/cast_lossless_bool.fixed new file mode 100644 index 00000000000..9e2da45c378 --- /dev/null +++ b/tests/ui/cast_lossless_bool.fixed @@ -0,0 +1,42 @@ +// run-rustfix + +#![allow(dead_code)] +#![warn(clippy::cast_lossless)] + +fn main() { + // Test clippy::cast_lossless with casts to integer types + let _ = u8::from(true); + let _ = u16::from(true); + let _ = u32::from(true); + let _ = u64::from(true); + let _ = u128::from(true); + let _ = usize::from(true); + + let _ = i8::from(true); + let _ = i16::from(true); + let _ = i32::from(true); + let _ = i64::from(true); + let _ = i128::from(true); + let _ = isize::from(true); + + // Test with an expression wrapped in parens + let _ = u16::from(true | false); +} + +// 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 abc(input: bool) -> u32 { + input as u32 +} + +// Same as the above issue. We can't suggest `::from` in const fns in impls +mod cast_lossless_in_impl { + struct A; + + impl A { + pub const fn convert(x: bool) -> u64 { + x as u64 + } + } +} diff --git a/tests/ui/cast_lossless_bool.rs b/tests/ui/cast_lossless_bool.rs new file mode 100644 index 00000000000..b6f6c59a01f --- /dev/null +++ b/tests/ui/cast_lossless_bool.rs @@ -0,0 +1,42 @@ +// run-rustfix + +#![allow(dead_code)] +#![warn(clippy::cast_lossless)] + +fn main() { + // Test clippy::cast_lossless with casts to integer types + let _ = true as u8; + let _ = true as u16; + let _ = true as u32; + let _ = true as u64; + let _ = true as u128; + let _ = true as usize; + + let _ = true as i8; + let _ = true as i16; + let _ = true as i32; + let _ = true as i64; + let _ = true as i128; + let _ = true as isize; + + // Test with an expression wrapped in parens + let _ = (true | false) as u16; +} + +// 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 abc(input: bool) -> u32 { + input as u32 +} + +// Same as the above issue. We can't suggest `::from` in const fns in impls +mod cast_lossless_in_impl { + struct A; + + impl A { + pub const fn convert(x: bool) -> u64 { + x as u64 + } + } +} diff --git a/tests/ui/cast_lossless_bool.stderr b/tests/ui/cast_lossless_bool.stderr new file mode 100644 index 00000000000..6b148336011 --- /dev/null +++ b/tests/ui/cast_lossless_bool.stderr @@ -0,0 +1,82 @@ +error: casting `bool` to `u8` is more cleanly stated with `u8::from(_)` + --> $DIR/cast_lossless_bool.rs:8:13 + | +LL | let _ = true as u8; + | ^^^^^^^^^^ help: try: `u8::from(true)` + | + = note: `-D clippy::cast-lossless` implied by `-D warnings` + +error: casting `bool` to `u16` is more cleanly stated with `u16::from(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/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(_)` + --> $DIR/cast_lossless_bool.rs:23:13 + | +LL | let _ = (true | false) as u16; + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::from(true | false)` + +error: aborting due to 13 previous errors + diff --git a/tests/ui/min_rust_version_attr.rs b/tests/ui/min_rust_version_attr.rs index 57ed453da10..c5f221220ec 100644 --- a/tests/ui/min_rust_version_attr.rs +++ b/tests/ui/min_rust_version_attr.rs @@ -140,6 +140,11 @@ fn unnest_or_patterns() { #[cfg_attr(rustfmt, rustfmt_skip)] fn deprecated_cfg_attr() {} +#[warn(clippy::cast_lossless)] +fn int_from_bool() -> u8 { + true as u8 +} + fn main() { filter_map_next(); checked_conversion(); @@ -156,6 +161,7 @@ fn main() { map_unwrap_or(); missing_const_for_fn(); unnest_or_patterns(); + int_from_bool(); } mod just_under_msrv { diff --git a/tests/ui/min_rust_version_attr.stderr b/tests/ui/min_rust_version_attr.stderr index 4388b78f832..6b3fdb0844b 100644 --- a/tests/ui/min_rust_version_attr.stderr +++ b/tests/ui/min_rust_version_attr.stderr @@ -1,12 +1,12 @@ error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:180:24 + --> $DIR/min_rust_version_attr.rs:186:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-strip` implied by `-D warnings` note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:179:9 + --> $DIR/min_rust_version_attr.rs:185:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,13 +17,13 @@ LL ~ assert_eq!(<stripped>.to_uppercase(), "WORLD!"); | error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:192:24 + --> $DIR/min_rust_version_attr.rs:198:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:191:9 + --> $DIR/min_rust_version_attr.rs:197:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |