From 00a5ef64a20c0b188ac27b5ba935cb291ea3bf85 Mon Sep 17 00:00:00 2001 From: pJunger Date: Tue, 14 May 2019 20:05:27 +0200 Subject: [PATCH] Added suggestion for conversion with is_ok. --- clippy_lints/src/checked_conversions.rs | 34 +++++--- tests/ui/checked_conversions.fixed | 106 ++++++++++++++++++++++++ tests/ui/checked_conversions.rs | 4 + tests/ui/checked_conversions.stderr | 48 +++++------ 4 files changed, 156 insertions(+), 36 deletions(-) create mode 100644 tests/ui/checked_conversions.fixed diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 48d941f9f8b..3001147f92f 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -4,9 +4,9 @@ use if_chain::if_chain; use rustc::hir::*; use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; +use rustc_errors::Applicability; use syntax::ast::LitKind; - -use crate::utils::{span_lint, SpanlessEq}; +use crate::utils::{span_lint_and_sugg, snippet_with_applicability, SpanlessEq}; declare_clippy_lint! { /// **What it does:** Checks for explicit bounds checking when casting. @@ -54,16 +54,26 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CheckedConversions { } }; - if let Some(cv) = result { - span_lint( - cx, - CHECKED_CONVERSIONS, - item.span, - &format!( - "Checked cast can be simplified: `{}::try_from`", - cv.to_type.unwrap_or_else(|| "IntegerType".to_string()), - ), - ); + if_chain! { + if let Some(cv) = result; + if let Some(to_type) = cv.to_type; + + then { + let mut applicability = Applicability::MachineApplicable; + let snippet = snippet_with_applicability(cx, cv.expr_to_cast.span, "_", &mut + applicability); + span_lint_and_sugg( + cx, + CHECKED_CONVERSIONS, + item.span, + "Checked cast can be simplified.", + "try", + format!("{}::try_from({}).is_ok()", + to_type, + snippet), + applicability + ); + } } } } diff --git a/tests/ui/checked_conversions.fixed b/tests/ui/checked_conversions.fixed new file mode 100644 index 00000000000..7febd6f3761 --- /dev/null +++ b/tests/ui/checked_conversions.fixed @@ -0,0 +1,106 @@ +// run-rustfix + +#![warn(clippy::checked_conversions)] +#![allow(clippy::cast_lossless)] +#![allow(dead_code)] +use std::convert::TryFrom; + +// Positive tests + +// Signed to unsigned + +fn i64_to_u32(value: i64) -> Option { + if u32::try_from(value).is_ok() { + Some(value as u32) + } else { + None + } +} + +fn i64_to_u16(value: i64) -> Option { + if u16::try_from(value).is_ok() { + Some(value as u16) + } else { + None + } +} + +fn isize_to_u8(value: isize) -> Option { + if u8::try_from(value).is_ok() { + Some(value as u8) + } else { + None + } +} + +// Signed to signed + +fn i64_to_i32(value: i64) -> Option { + if i32::try_from(value).is_ok() { + Some(value as i32) + } else { + None + } +} + +fn i64_to_i16(value: i64) -> Option { + if i16::try_from(value).is_ok() { + Some(value as i16) + } else { + None + } +} + +// Unsigned to X + +fn u32_to_i32(value: u32) -> Option { + if i32::try_from(value).is_ok() { + Some(value as i32) + } else { + None + } +} + +fn usize_to_isize(value: usize) -> isize { + if isize::try_from(value).is_ok() && value as i32 == 5 { + 5 + } else { + 1 + } +} + +fn u32_to_u16(value: u32) -> isize { + if u16::try_from(value).is_ok() && value as i32 == 5 { + 5 + } else { + 1 + } +} + +// Negative tests + +fn no_i64_to_i32(value: i64) -> Option { + if value <= (i32::max_value() as i64) && value >= 0 { + Some(value as i32) + } else { + None + } +} + +fn no_isize_to_u8(value: isize) -> Option { + if value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize) { + Some(value as u8) + } else { + None + } +} + +fn i8_to_u8(value: i8) -> Option { + if value >= 0 { + Some(value as u8) + } else { + None + } +} + +fn main() {} diff --git a/tests/ui/checked_conversions.rs b/tests/ui/checked_conversions.rs index 71699a0733d..a643354e243 100644 --- a/tests/ui/checked_conversions.rs +++ b/tests/ui/checked_conversions.rs @@ -1,5 +1,9 @@ +// run-rustfix + #![warn(clippy::checked_conversions)] #![allow(clippy::cast_lossless)] +#![allow(dead_code)] +use std::convert::TryFrom; // Positive tests diff --git a/tests/ui/checked_conversions.stderr b/tests/ui/checked_conversions.stderr index a3fed35eccc..f678f009621 100644 --- a/tests/ui/checked_conversions.stderr +++ b/tests/ui/checked_conversions.stderr @@ -1,52 +1,52 @@ -error: Checked cast can be simplified: `u32::try_from` - --> $DIR/checked_conversions.rs:9:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:13:8 | LL | if value <= (u32::max_value() as i64) && value >= 0 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()` | = note: `-D clippy::checked-conversions` implied by `-D warnings` -error: Checked cast can be simplified: `u16::try_from` - --> $DIR/checked_conversions.rs:17:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:21:8 | LL | if value <= i64::from(u16::max_value()) && value >= 0 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` -error: Checked cast can be simplified: `u8::try_from` - --> $DIR/checked_conversions.rs:25:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:29:8 | LL | if value <= (u8::max_value() as isize) && value >= 0 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()` -error: Checked cast can be simplified: `i32::try_from` - --> $DIR/checked_conversions.rs:35:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:39:8 | LL | if value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` -error: Checked cast can be simplified: `i16::try_from` - --> $DIR/checked_conversions.rs:43:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:47:8 | LL | if value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()` -error: Checked cast can be simplified: `i32::try_from` - --> $DIR/checked_conversions.rs:53:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:57:8 | LL | if value <= i32::max_value() as u32 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` -error: Checked cast can be simplified: `isize::try_from` - --> $DIR/checked_conversions.rs:61:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:65:8 | LL | if value <= isize::max_value() as usize && value as i32 == 5 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()` -error: Checked cast can be simplified: `u16::try_from` - --> $DIR/checked_conversions.rs:69:8 +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:73:8 | LL | if value <= u16::max_value() as u32 && value as i32 == 5 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` error: aborting due to 8 previous errors