Fix false negative of checked_conversion lint

This commit is contained in:
Lzu Tao 2020-06-03 09:04:24 +07:00
parent 6c833df69b
commit b39fd5f62f
5 changed files with 188 additions and 200 deletions

View File

@ -58,24 +58,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CheckedConversions {
} }
}; };
if_chain! { if let Some(cv) = result {
if let Some(cv) = result; if let Some(to_type) = cv.to_type {
if let Some(to_type) = cv.to_type;
then {
let mut applicability = Applicability::MachineApplicable; let mut applicability = Applicability::MachineApplicable;
let snippet = snippet_with_applicability(cx, cv.expr_to_cast.span, "_", &mut let snippet = snippet_with_applicability(cx, cv.expr_to_cast.span, "_", &mut applicability);
applicability);
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
CHECKED_CONVERSIONS, CHECKED_CONVERSIONS,
item.span, item.span,
"Checked cast can be simplified.", "Checked cast can be simplified.",
"try", "try",
format!("{}::try_from({}).is_ok()", format!("{}::try_from({}).is_ok()", to_type, snippet),
to_type, applicability,
snippet),
applicability
); );
} }
} }
@ -184,7 +178,7 @@ fn check_upper_bound<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> {
if_chain! { if_chain! {
if let ExprKind::Binary(ref op, ref left, ref right) = &expr.kind; if let ExprKind::Binary(ref op, ref left, ref right) = &expr.kind;
if let Some((candidate, check)) = normalize_le_ge(op, left, right); if let Some((candidate, check)) = normalize_le_ge(op, left, right);
if let Some((from, to)) = get_types_from_cast(check, MAX_VALUE, INTS); if let Some((from, to)) = get_types_from_cast(check, INTS, "max_value", "MAX");
then { then {
Conversion::try_new(candidate, from, to) Conversion::try_new(candidate, from, to)
@ -224,7 +218,7 @@ fn check_lower_bound_zero<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> O
/// Check for `expr >= (to_type::MIN as from_type)` /// Check for `expr >= (to_type::MIN as from_type)`
fn check_lower_bound_min<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Option<Conversion<'a>> { fn check_lower_bound_min<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Option<Conversion<'a>> {
if let Some((from, to)) = get_types_from_cast(check, MIN_VALUE, SINTS) { if let Some((from, to)) = get_types_from_cast(check, SINTS, "min_value", "MIN") {
Conversion::try_new(candidate, from, to) Conversion::try_new(candidate, from, to)
} else { } else {
None None
@ -232,10 +226,16 @@ fn check_lower_bound_min<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Op
} }
/// Tries to extract the from- and to-type from a cast expression /// Tries to extract the from- and to-type from a cast expression
fn get_types_from_cast<'a>(expr: &'a Expr<'_>, func: &'a str, types: &'a [&str]) -> Option<(&'a str, &'a str)> { fn get_types_from_cast<'a>(
// `to_type::maxmin_value() as from_type` expr: &'a Expr<'_>,
types: &'a [&str],
func: &'a str,
assoc_const: &'a str,
) -> Option<(&'a str, &'a str)> {
// `to_type::max_value() as from_type`
// or `to_type::MAX as from_type`
let call_from_cast: Option<(&Expr<'_>, &str)> = if_chain! { let call_from_cast: Option<(&Expr<'_>, &str)> = if_chain! {
// to_type::maxmin_value(), from_type // to_type::max_value(), from_type
if let ExprKind::Cast(ref limit, ref from_type) = &expr.kind; if let ExprKind::Cast(ref limit, ref from_type) = &expr.kind;
if let TyKind::Path(ref from_type_path) = &from_type.kind; if let TyKind::Path(ref from_type_path) = &from_type.kind;
if let Some(from_sym) = int_ty_to_sym(from_type_path); if let Some(from_sym) = int_ty_to_sym(from_type_path);
@ -247,17 +247,17 @@ fn get_types_from_cast<'a>(expr: &'a Expr<'_>, func: &'a str, types: &'a [&str])
} }
}; };
// `from_type::from(to_type::maxmin_value())` // `from_type::from(to_type::max_value())`
let limit_from: Option<(&Expr<'_>, &str)> = call_from_cast.or_else(|| { let limit_from: Option<(&Expr<'_>, &str)> = call_from_cast.or_else(|| {
if_chain! { if_chain! {
// `from_type::from, to_type::maxmin_value()` // `from_type::from, to_type::max_value()`
if let ExprKind::Call(ref from_func, ref args) = &expr.kind; if let ExprKind::Call(ref from_func, ref args) = &expr.kind;
// `to_type::maxmin_value()` // `to_type::max_value()`
if args.len() == 1; if args.len() == 1;
if let limit = &args[0]; if let limit = &args[0];
// `from_type::from` // `from_type::from`
if let ExprKind::Path(ref path) = &from_func.kind; if let ExprKind::Path(ref path) = &from_func.kind;
if let Some(from_sym) = get_implementing_type(path, INTS, FROM); if let Some(from_sym) = get_implementing_type(path, INTS, "from");
then { then {
Some((limit, from_sym)) Some((limit, from_sym))
@ -268,22 +268,26 @@ fn get_types_from_cast<'a>(expr: &'a Expr<'_>, func: &'a str, types: &'a [&str])
}); });
if let Some((limit, from_type)) = limit_from { if let Some((limit, from_type)) = limit_from {
if_chain! { match limit.kind {
if let ExprKind::Call(ref fun_name, _) = &limit.kind; // `from_type::from(_)`
// `to_type, maxmin_value` ExprKind::Call(path, _) => {
if let ExprKind::Path(ref path) = &fun_name.kind; if let ExprKind::Path(ref path) = path.kind {
// `to_type` // `to_type`
if let Some(to_type) = get_implementing_type(path, types, func); if let Some(to_type) = get_implementing_type(path, types, func) {
return Some((from_type, to_type));
then { }
Some((from_type, to_type)) }
} else { },
None // `to_type::MAX`
} ExprKind::Path(ref path) => {
if let Some(to_type) = get_implementing_type(path, types, assoc_const) {
return Some((from_type, to_type));
}
},
_ => {},
} }
} else { };
None None
}
} }
/// Gets the type which implements the called function /// Gets the type which implements the called function
@ -336,10 +340,6 @@ fn normalize_le_ge<'a>(op: &BinOp, left: &'a Expr<'a>, right: &'a Expr<'a>) -> O
} }
// Constants // Constants
const FROM: &str = "from";
const MAX_VALUE: &str = "max_value";
const MIN_VALUE: &str = "min_value";
const UINTS: &[&str] = &["u8", "u16", "u32", "u64", "usize"]; const UINTS: &[&str] = &["u8", "u16", "u32", "u64", "usize"];
const SINTS: &[&str] = &["i8", "i16", "i32", "i64", "isize"]; const SINTS: &[&str] = &["i8", "i16", "i32", "i64", "isize"];
const INTS: &[&str] = &["u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize"]; const INTS: &[&str] = &["u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize"];

View File

@ -1,106 +1,76 @@
// run-rustfix // run-rustfix
#![allow(
clippy::cast_lossless,
// Int::max_value will be deprecated in the future
deprecated,
)]
#![warn(clippy::checked_conversions)] #![warn(clippy::checked_conversions)]
#![allow(clippy::cast_lossless)]
#![allow(dead_code)]
use std::convert::TryFrom; use std::convert::TryFrom;
// Positive tests // Positive tests
// Signed to unsigned // Signed to unsigned
fn i64_to_u32(value: i64) -> Option<u32> { pub fn i64_to_u32(value: i64) {
if u32::try_from(value).is_ok() { let _ = u32::try_from(value).is_ok();
Some(value as u32) let _ = u32::try_from(value).is_ok();
} else {
None
}
} }
fn i64_to_u16(value: i64) -> Option<u16> { pub fn i64_to_u16(value: i64) {
if u16::try_from(value).is_ok() { let _ = u16::try_from(value).is_ok();
Some(value as u16) let _ = u16::try_from(value).is_ok();
} else {
None
}
} }
fn isize_to_u8(value: isize) -> Option<u8> { pub fn isize_to_u8(value: isize) {
if u8::try_from(value).is_ok() { let _ = u8::try_from(value).is_ok();
Some(value as u8) let _ = u8::try_from(value).is_ok();
} else {
None
}
} }
// Signed to signed // Signed to signed
fn i64_to_i32(value: i64) -> Option<i32> { pub fn i64_to_i32(value: i64) {
if i32::try_from(value).is_ok() { let _ = i32::try_from(value).is_ok();
Some(value as i32) let _ = i32::try_from(value).is_ok();
} else {
None
}
} }
fn i64_to_i16(value: i64) -> Option<i16> { pub fn i64_to_i16(value: i64) {
if i16::try_from(value).is_ok() { let _ = i16::try_from(value).is_ok();
Some(value as i16) let _ = i16::try_from(value).is_ok();
} else {
None
}
} }
// Unsigned to X // Unsigned to X
fn u32_to_i32(value: u32) -> Option<i32> { pub fn u32_to_i32(value: u32) {
if i32::try_from(value).is_ok() { let _ = i32::try_from(value).is_ok();
Some(value as i32) let _ = i32::try_from(value).is_ok();
} else {
None
}
} }
fn usize_to_isize(value: usize) -> isize { pub fn usize_to_isize(value: usize) {
if isize::try_from(value).is_ok() && value as i32 == 5 { let _ = isize::try_from(value).is_ok() && value as i32 == 5;
5 let _ = isize::try_from(value).is_ok() && value as i32 == 5;
} else {
1
}
} }
fn u32_to_u16(value: u32) -> isize { pub fn u32_to_u16(value: u32) {
if u16::try_from(value).is_ok() && value as i32 == 5 { let _ = u16::try_from(value).is_ok() && value as i32 == 5;
5 let _ = u16::try_from(value).is_ok() && value as i32 == 5;
} else {
1
}
} }
// Negative tests // Negative tests
fn no_i64_to_i32(value: i64) -> Option<i32> { pub fn no_i64_to_i32(value: i64) {
if value <= (i32::max_value() as i64) && value >= 0 { let _ = value <= (i32::max_value() as i64) && value >= 0;
Some(value as i32) let _ = value <= (i32::MAX as i64) && value >= 0;
} else {
None
}
} }
fn no_isize_to_u8(value: isize) -> Option<u8> { pub fn no_isize_to_u8(value: isize) {
if value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize) { let _ = value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize);
Some(value as u8) let _ = value <= (u8::MAX as isize) && value >= (u8::MIN as isize);
} else {
None
}
} }
fn i8_to_u8(value: i8) -> Option<u8> { pub fn i8_to_u8(value: i8) {
if value >= 0 { let _ = value >= 0;
Some(value as u8)
} else {
None
}
} }
fn main() {} fn main() {}

View File

@ -1,106 +1,76 @@
// run-rustfix // run-rustfix
#![allow(
clippy::cast_lossless,
// Int::max_value will be deprecated in the future
deprecated,
)]
#![warn(clippy::checked_conversions)] #![warn(clippy::checked_conversions)]
#![allow(clippy::cast_lossless)]
#![allow(dead_code)]
use std::convert::TryFrom; use std::convert::TryFrom;
// Positive tests // Positive tests
// Signed to unsigned // Signed to unsigned
fn i64_to_u32(value: i64) -> Option<u32> { pub fn i64_to_u32(value: i64) {
if value <= (u32::max_value() as i64) && value >= 0 { let _ = value <= (u32::max_value() as i64) && value >= 0;
Some(value as u32) let _ = value <= (u32::MAX as i64) && value >= 0;
} else {
None
}
} }
fn i64_to_u16(value: i64) -> Option<u16> { pub fn i64_to_u16(value: i64) {
if value <= i64::from(u16::max_value()) && value >= 0 { let _ = value <= i64::from(u16::max_value()) && value >= 0;
Some(value as u16) let _ = value <= i64::from(u16::MAX) && value >= 0;
} else {
None
}
} }
fn isize_to_u8(value: isize) -> Option<u8> { pub fn isize_to_u8(value: isize) {
if value <= (u8::max_value() as isize) && value >= 0 { let _ = value <= (u8::max_value() as isize) && value >= 0;
Some(value as u8) let _ = value <= (u8::MAX as isize) && value >= 0;
} else {
None
}
} }
// Signed to signed // Signed to signed
fn i64_to_i32(value: i64) -> Option<i32> { pub fn i64_to_i32(value: i64) {
if value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64) { let _ = value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64);
Some(value as i32) let _ = value <= (i32::MAX as i64) && value >= (i32::MIN as i64);
} else {
None
}
} }
fn i64_to_i16(value: i64) -> Option<i16> { pub fn i64_to_i16(value: i64) {
if value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()) { let _ = value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value());
Some(value as i16) let _ = value <= i64::from(i16::MAX) && value >= i64::from(i16::MIN);
} else {
None
}
} }
// Unsigned to X // Unsigned to X
fn u32_to_i32(value: u32) -> Option<i32> { pub fn u32_to_i32(value: u32) {
if value <= i32::max_value() as u32 { let _ = value <= i32::max_value() as u32;
Some(value as i32) let _ = value <= i32::MAX as u32;
} else {
None
}
} }
fn usize_to_isize(value: usize) -> isize { pub fn usize_to_isize(value: usize) {
if value <= isize::max_value() as usize && value as i32 == 5 { let _ = value <= isize::max_value() as usize && value as i32 == 5;
5 let _ = value <= isize::MAX as usize && value as i32 == 5;
} else {
1
}
} }
fn u32_to_u16(value: u32) -> isize { pub fn u32_to_u16(value: u32) {
if value <= u16::max_value() as u32 && value as i32 == 5 { let _ = value <= u16::max_value() as u32 && value as i32 == 5;
5 let _ = value <= u16::MAX as u32 && value as i32 == 5;
} else {
1
}
} }
// Negative tests // Negative tests
fn no_i64_to_i32(value: i64) -> Option<i32> { pub fn no_i64_to_i32(value: i64) {
if value <= (i32::max_value() as i64) && value >= 0 { let _ = value <= (i32::max_value() as i64) && value >= 0;
Some(value as i32) let _ = value <= (i32::MAX as i64) && value >= 0;
} else {
None
}
} }
fn no_isize_to_u8(value: isize) -> Option<u8> { pub fn no_isize_to_u8(value: isize) {
if value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize) { let _ = value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize);
Some(value as u8) let _ = value <= (u8::MAX as isize) && value >= (u8::MIN as isize);
} else {
None
}
} }
fn i8_to_u8(value: i8) -> Option<u8> { pub fn i8_to_u8(value: i8) {
if value >= 0 { let _ = value >= 0;
Some(value as u8)
} else {
None
}
} }
fn main() {} fn main() {}

View File

@ -1,52 +1,100 @@
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:13:8 --> $DIR/checked_conversions.rs:17:13
| |
LL | if value <= (u32::max_value() as i64) && value >= 0 { LL | let _ = value <= (u32::max_value() as i64) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()`
| |
= note: `-D clippy::checked-conversions` implied by `-D warnings` = note: `-D clippy::checked-conversions` implied by `-D warnings`
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:21:8 --> $DIR/checked_conversions.rs:18:13
| |
LL | if value <= i64::from(u16::max_value()) && value >= 0 { LL | let _ = value <= (u32::MAX as i64) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()`
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:29:8 --> $DIR/checked_conversions.rs:22:13
| |
LL | if value <= (u8::max_value() as isize) && value >= 0 { LL | let _ = value <= i64::from(u16::max_value()) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:39:8 --> $DIR/checked_conversions.rs:23:13
| |
LL | if value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64) { LL | let _ = value <= i64::from(u16::MAX) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:47:8 --> $DIR/checked_conversions.rs:27:13
| |
LL | if value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()) { LL | let _ = value <= (u8::max_value() as isize) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()`
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:57:8 --> $DIR/checked_conversions.rs:28:13
| |
LL | if value <= i32::max_value() as u32 { LL | let _ = value <= (u8::MAX as isize) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()`
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:65:8 --> $DIR/checked_conversions.rs:34:13
| |
LL | if value <= isize::max_value() as usize && value as i32 == 5 { LL | let _ = value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`
error: Checked cast can be simplified. error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:73:8 --> $DIR/checked_conversions.rs:35:13
| |
LL | if value <= u16::max_value() as u32 && value as i32 == 5 { LL | let _ = value <= (i32::MAX as i64) && value >= (i32::MIN as i64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`
error: aborting due to 8 previous errors error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:39:13
|
LL | let _ = 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.
--> $DIR/checked_conversions.rs:40:13
|
LL | let _ = value <= i64::from(i16::MAX) && value >= i64::from(i16::MIN);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()`
error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:46:13
|
LL | let _ = value <= i32::max_value() as u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`
error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:47:13
|
LL | let _ = value <= i32::MAX as u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`
error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:51:13
|
LL | let _ = value <= isize::max_value() as usize && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()`
error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:52:13
|
LL | let _ = value <= isize::MAX as usize && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()`
error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:56:13
|
LL | let _ = value <= u16::max_value() as u32 && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`
error: Checked cast can be simplified.
--> $DIR/checked_conversions.rs:57:13
|
LL | let _ = value <= u16::MAX as u32 && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`
error: aborting due to 16 previous errors