From 47444c44adf457a63e46d7249424793823c0cfa3 Mon Sep 17 00:00:00 2001 From: pJunger Date: Sun, 12 May 2019 14:46:16 +0200 Subject: [PATCH 1/9] Added lint for TryFrom for checked integer conversion rust-lang#3947. --- CHANGELOG.md | 1 + clippy_lints/src/checked_conversions.rs | 332 ++++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + tests/ui/checked_conversions.rs | 102 ++++++++ tests/ui/checked_conversions.stderr | 52 ++++ tests/ui/checked_conversions.stdout | 0 6 files changed, 489 insertions(+) create mode 100644 clippy_lints/src/checked_conversions.rs create mode 100644 tests/ui/checked_conversions.rs create mode 100644 tests/ui/checked_conversions.stderr create mode 100644 tests/ui/checked_conversions.stdout diff --git a/CHANGELOG.md b/CHANGELOG.md index 44787f4f2818..08fcbc127b89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -846,6 +846,7 @@ All notable changes to this project will be documented in this file. [`char_lit_as_u8`]: https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp [`chars_next_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_next_cmp +[`checked_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#checked_conversions [`clone_double_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref [`clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy [`clone_on_ref_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs new file mode 100644 index 000000000000..55a496f1fce6 --- /dev/null +++ b/clippy_lints/src/checked_conversions.rs @@ -0,0 +1,332 @@ +//! lint on manually implemented checked conversions that could be transformed into try_from + +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 syntax::ast::LitKind; + +use crate::utils::{span_lint, SpanlessEq}; + +declare_clippy_lint! { + /// **What it does:** Checks for explicit bounds checking when casting. + /// + /// **Why is this bad?** Reduces the readability of statements & is error prone. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// # let foo: u32 = 5; + /// # let _ = + /// foo <= i32::max_value() as u32 + /// # ; + /// ``` + /// + /// Could be written: + /// + /// ```rust + /// # let _ = + /// i32::try_from(foo).is_ok() + /// # ; + /// ``` + pub CHECKED_CONVERSIONS, + pedantic, + "`try_from` could replace manual bounds checking when casting" +} + +declare_lint_pass!(CheckedConversions => [CHECKED_CONVERSIONS]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CheckedConversions { + fn check_expr(&mut self, cx: &LateContext<'_, '_>, item: &Expr) { + let result = if_chain! { + if !in_external_macro(cx.sess(), item.span); + if let ExprKind::Binary(op, ref left, ref right) = &item.node; + + then { + match op.node { + BinOpKind::Ge | BinOpKind::Le => single_check(item), + BinOpKind::And => double_check(cx, left, right), + _ => None, + } + } else { + None + } + }; + + 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("IntegerType".to_string()), + ), + ); + } + } +} + +/// Searches for a single check from unsigned to _ is done +/// todo: check for case signed -> larger unsigned == only x >= 0 +fn single_check(expr: &Expr) -> Option> { + check_upper_bound(expr).filter(|cv| cv.cvt == ConversionType::FromUnsigned) +} + +/// Searches for a combination of upper & lower bound checks +fn double_check<'a>(cx: &LateContext<'_, '_>, left: &'a Expr, right: &'a Expr) -> Option> { + let upper_lower = |l, r| { + let upper = check_upper_bound(l); + let lower = check_lower_bound(r); + + transpose(upper, lower).and_then(|(l, r)| l.combine(r, cx)) + }; + + upper_lower(left, right).or_else(|| upper_lower(right, left)) +} + +/// Contains the result of a tried conversion check +#[derive(Clone, Debug)] +struct Conversion<'a> { + cvt: ConversionType, + expr_to_cast: &'a Expr, + to_type: Option, +} + +/// The kind of conversion that is checked +#[derive(Copy, Clone, Debug, PartialEq)] +enum ConversionType { + SignedToUnsigned, + SignedToSigned, + FromUnsigned, +} + +impl<'a> Conversion<'a> { + /// Combine multiple conversions if the are compatible + pub fn combine(self, other: Self, cx: &LateContext<'_, '_>) -> Option> { + if self.is_compatible(&other, cx) { + // Prefer a Conversion that contains a type-constraint + Some(if self.to_type.is_some() { self } else { other }) + } else { + None + } + } + + /// Checks if two conversions are compatible + /// same type of conversion, same 'castee' and same 'to type' + pub fn is_compatible(&self, other: &Self, cx: &LateContext<'_, '_>) -> bool { + (self.cvt == other.cvt) + && (SpanlessEq::new(cx).eq_expr(self.expr_to_cast, other.expr_to_cast)) + && (self.has_compatible_to_type(other)) + } + + /// Checks if the to-type is the same (if there is a type constraint) + fn has_compatible_to_type(&self, other: &Self) -> bool { + transpose(self.to_type.as_ref(), other.to_type.as_ref()) + .map(|(l, r)| l == r) + .unwrap_or(true) + } + + /// Try to construct a new conversion if the conversion type is valid + fn try_new<'b>(expr_to_cast: &'a Expr, from_type: &'b str, to_type: String) -> Option> { + ConversionType::try_new(from_type, &to_type).map(|cvt| Conversion { + cvt, + expr_to_cast, + to_type: Some(to_type), + }) + } + + /// Construct a new conversion without type constraint + fn new_any(expr_to_cast: &'a Expr) -> Conversion<'a> { + Conversion { + cvt: ConversionType::SignedToUnsigned, + expr_to_cast, + to_type: None, + } + } +} + +impl ConversionType { + /// Creates a conversion type if the type is allowed & conversion is valid + fn try_new(from: &str, to: &str) -> Option { + if UNSIGNED_TYPES.contains(&from) { + Some(ConversionType::FromUnsigned) + } else if SIGNED_TYPES.contains(&from) { + if UNSIGNED_TYPES.contains(&to) { + Some(ConversionType::SignedToUnsigned) + } else if SIGNED_TYPES.contains(&to) { + Some(ConversionType::SignedToSigned) + } else { + None + } + } else { + None + } + } +} + +/// Check for `expr <= (to_type::max_value() as from_type)` +fn check_upper_bound(expr: &Expr) -> Option> { + if_chain! { + if let ExprKind::Binary(ref op, ref left, ref right) = &expr.node; + if let Some((candidate, check)) = normalize_le_ge(op, left, right); + if let Some((from, to)) = get_types_from_cast(check, "max_value", INT_TYPES); + + then { + Conversion::try_new(candidate, &from, to) + } else { + None + } + } +} + +/// Check for `expr >= 0|(to_type::min_value() as from_type)` +fn check_lower_bound(expr: &Expr) -> Option> { + fn check_function<'a>(candidate: &'a Expr, check: &'a Expr) -> Option> { + (check_lower_bound_zero(candidate, check)).or_else(|| (check_lower_bound_min(candidate, check))) + } + + // First of we need a binary containing the expression & the cast + if let ExprKind::Binary(ref op, ref left, ref right) = &expr.node { + normalize_le_ge(op, right, left).and_then(|(l, r)| check_function(l, r)) + } else { + None + } +} + +/// Check for `expr >= 0` +fn check_lower_bound_zero<'a>(candidate: &'a Expr, check: &'a Expr) -> Option> { + if_chain! { + if let ExprKind::Lit(ref lit) = &check.node; + if let LitKind::Int(0, _) = &lit.node; + + then { + Some(Conversion::new_any(candidate)) + } else { + None + } + } +} + +/// Check for `expr >= (to_type::min_value() as from_type)` +fn check_lower_bound_min<'a>(candidate: &'a Expr, check: &'a Expr) -> Option> { + if let Some((from, to)) = get_types_from_cast(check, "min_value", SIGNED_TYPES) { + Conversion::try_new(candidate, &from, to) + } else { + None + } +} + +/// Tries to extract the from- and to-type from a cast expression +fn get_types_from_cast(expr: &Expr, func: &str, types: &[&str]) -> Option<(String, String)> { + // `to_type::maxmin_value() as from_type` + let call_from_cast: Option<(&Expr, String)> = if_chain! { + // to_type::maxmin_value(), from_type + if let ExprKind::Cast(ref limit, ref from_type) = &expr.node; + if let TyKind::Path(ref from_type_path) = &from_type.node; + if let Some(from_type_str) = int_ty_to_str(from_type_path); + + then { + Some((limit, from_type_str.to_string())) + } else { + None + } + }; + + // `from_type::from(to_type::maxmin_value())` + let limit_from: Option<(&Expr, String)> = call_from_cast.or_else(|| { + if_chain! { + // `from_type::from, to_type::maxmin_value()` + if let ExprKind::Call(ref from_func, ref args) = &expr.node; + // `to_type::maxmin_value()` + if args.len() == 1; + if let limit = &args[0]; + // `from_type::from` + if let ExprKind::Path(ref path) = &from_func.node; + if let Some(from_type) = get_implementing_type(path, INT_TYPES, "from"); + + then { + Some((limit, from_type)) + } else { + None + } + } + }); + + if let Some((limit, from_type)) = limit_from { + if_chain! { + if let ExprKind::Call(ref fun_name, _) = &limit.node; + // `to_type, maxmin_value` + if let ExprKind::Path(ref path) = &fun_name.node; + // `to_type` + if let Some(to_type) = get_implementing_type(path, types, func); + + then { + Some((from_type, to_type)) + } else { + None + } + } + } else { + None + } +} + +/// Gets the type which implements the called function +fn get_implementing_type(path: &QPath, candidates: &[&str], function: &str) -> Option { + if_chain! { + if let QPath::TypeRelative(ref ty, ref path) = &path; + if path.ident.name == function; + if let TyKind::Path(QPath::Resolved(None, ref tp)) = &ty.node; + if let [int] = &*tp.segments; + let name = int.ident.as_str().get(); + if candidates.contains(&name); + + then { + Some(name.to_string()) + } else { + None + } + } +} + +/// Gets the type as a string, if it is a supported integer +fn int_ty_to_str(path: &QPath) -> Option<&str> { + if_chain! { + if let QPath::Resolved(_, ref path) = *path; + if let [ty] = &*path.segments; + + then { + INT_TYPES + .into_iter() + .find(|c| (&ty.ident.name) == *c) + .cloned() + } else { + None + } + } +} + +/// (Option, Option) -> Option<(T, U)> +fn transpose(lhs: Option, rhs: Option) -> Option<(T, U)> { + match (lhs, rhs) { + (Some(l), Some(r)) => Some((l, r)), + _ => None, + } +} + +/// Will return the expressions as if they were expr1 <= expr2 +fn normalize_le_ge<'a>(op: &'a BinOp, left: &'a Expr, right: &'a Expr) -> Option<(&'a Expr, &'a Expr)> { + match op.node { + BinOpKind::Le => Some((left, right)), + BinOpKind::Ge => Some((right, left)), + _ => None, + } +} + +const UNSIGNED_TYPES: &[&str] = &["u8", "u16", "u32", "u64", "u128", "usize"]; +const SIGNED_TYPES: &[&str] = &["i8", "i16", "i32", "i64", "i128", "isize"]; +const INT_TYPES: &[&str] = &[ + "u8", "u16", "u32", "u64", "u128", "usize", "i8", "i16", "i32", "i64", "i128", "isize", +]; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 400abb5a4980..3049ff672c57 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -154,6 +154,7 @@ pub mod block_in_if_condition; pub mod booleans; pub mod bytecount; pub mod cargo_common_metadata; +pub mod checked_conversions; pub mod cognitive_complexity; pub mod collapsible_if; pub mod const_static_lifetime; @@ -605,6 +606,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::pedantic", Some("clippy_pedantic"), vec![ attrs::INLINE_ALWAYS, + checked_conversions::CHECKED_CONVERSIONS, copies::MATCH_SAME_ARMS, copy_iterator::COPY_ITERATOR, default_trait_access::DEFAULT_TRAIT_ACCESS, diff --git a/tests/ui/checked_conversions.rs b/tests/ui/checked_conversions.rs new file mode 100644 index 000000000000..71699a0733d8 --- /dev/null +++ b/tests/ui/checked_conversions.rs @@ -0,0 +1,102 @@ +#![warn(clippy::checked_conversions)] +#![allow(clippy::cast_lossless)] + +// Positive tests + +// Signed to unsigned + +fn i64_to_u32(value: i64) -> Option { + if value <= (u32::max_value() as i64) && value >= 0 { + Some(value as u32) + } else { + None + } +} + +fn i64_to_u16(value: i64) -> Option { + if value <= i64::from(u16::max_value()) && value >= 0 { + Some(value as u16) + } else { + None + } +} + +fn isize_to_u8(value: isize) -> Option { + if value <= (u8::max_value() as isize) && value >= 0 { + Some(value as u8) + } else { + None + } +} + +// Signed to signed + +fn i64_to_i32(value: i64) -> Option { + if value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64) { + Some(value as i32) + } else { + None + } +} + +fn i64_to_i16(value: i64) -> Option { + if value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()) { + Some(value as i16) + } else { + None + } +} + +// Unsigned to X + +fn u32_to_i32(value: u32) -> Option { + if value <= i32::max_value() as u32 { + Some(value as i32) + } else { + None + } +} + +fn usize_to_isize(value: usize) -> isize { + if value <= isize::max_value() as usize && value as i32 == 5 { + 5 + } else { + 1 + } +} + +fn u32_to_u16(value: u32) -> isize { + if value <= u16::max_value() as u32 && 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.stderr b/tests/ui/checked_conversions.stderr new file mode 100644 index 000000000000..a3fed35eccca --- /dev/null +++ b/tests/ui/checked_conversions.stderr @@ -0,0 +1,52 @@ +error: Checked cast can be simplified: `u32::try_from` + --> $DIR/checked_conversions.rs:9:8 + | +LL | if value <= (u32::max_value() as i64) && value >= 0 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::checked-conversions` implied by `-D warnings` + +error: Checked cast can be simplified: `u16::try_from` + --> $DIR/checked_conversions.rs:17:8 + | +LL | if value <= i64::from(u16::max_value()) && value >= 0 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Checked cast can be simplified: `u8::try_from` + --> $DIR/checked_conversions.rs:25:8 + | +LL | if value <= (u8::max_value() as isize) && value >= 0 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Checked cast can be simplified: `i32::try_from` + --> $DIR/checked_conversions.rs:35:8 + | +LL | if value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Checked cast can be simplified: `i16::try_from` + --> $DIR/checked_conversions.rs:43:8 + | +LL | if value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Checked cast can be simplified: `i32::try_from` + --> $DIR/checked_conversions.rs:53:8 + | +LL | if value <= i32::max_value() as u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Checked cast can be simplified: `isize::try_from` + --> $DIR/checked_conversions.rs:61:8 + | +LL | if value <= isize::max_value() as usize && value as i32 == 5 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Checked cast can be simplified: `u16::try_from` + --> $DIR/checked_conversions.rs:69:8 + | +LL | if value <= u16::max_value() as u32 && value as i32 == 5 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors + diff --git a/tests/ui/checked_conversions.stdout b/tests/ui/checked_conversions.stdout new file mode 100644 index 000000000000..e69de29bb2d1 From 14d948c5601ce5a4970bf8f1fc3d16d302289488 Mon Sep 17 00:00:00 2001 From: pJunger Date: Sun, 12 May 2019 20:42:13 +0200 Subject: [PATCH 2/9] Registered lint. --- clippy_lints/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3049ff672c57..f74593662634 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -576,6 +576,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box missing_const_for_fn::MissingConstForFn); reg.register_late_lint_pass(box transmuting_null::TransmutingNull); reg.register_late_lint_pass(box path_buf_push_overwrite::PathBufPushOverwrite); + reg.register_late_lint_pass(box checked_conversions::CheckedConversions); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, From 1c86b3758d5ee0ee0b151d4f4b0dcc06c70ed1ae Mon Sep 17 00:00:00 2001 From: pJunger Date: Sun, 12 May 2019 21:14:44 +0200 Subject: [PATCH 3/9] Fixed clippy lints in checked_conversions.rs. --- clippy_lints/src/checked_conversions.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 55a496f1fce6..0269a1959b76 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -1,4 +1,4 @@ -//! lint on manually implemented checked conversions that could be transformed into try_from +//! lint on manually implemented checked conversions that could be transformed into `try_from` use if_chain::if_chain; use rustc::hir::*; @@ -61,7 +61,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CheckedConversions { item.span, &format!( "Checked cast can be simplified: `{}::try_from`", - cv.to_type.unwrap_or("IntegerType".to_string()), + cv.to_type.unwrap_or_else(|| "IntegerType".to_string()), ), ); } @@ -299,7 +299,7 @@ fn int_ty_to_str(path: &QPath) -> Option<&str> { then { INT_TYPES - .into_iter() + .iter() .find(|c| (&ty.ident.name) == *c) .cloned() } else { From 0a43dcfd042dbc7a8bd30a17a5bcf53c80c497f9 Mon Sep 17 00:00:00 2001 From: pJunger Date: Sun, 12 May 2019 21:53:28 +0200 Subject: [PATCH 4/9] Fixed more lint findings. --- clippy_lints/src/checked_conversions.rs | 6 ++---- clippy_lints/src/enum_clike.rs | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 0269a1959b76..48d941f9f8ba 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -123,9 +123,7 @@ impl<'a> Conversion<'a> { /// Checks if the to-type is the same (if there is a type constraint) fn has_compatible_to_type(&self, other: &Self) -> bool { - transpose(self.to_type.as_ref(), other.to_type.as_ref()) - .map(|(l, r)| l == r) - .unwrap_or(true) + transpose(self.to_type.as_ref(), other.to_type.as_ref()).map_or(true, |(l, r)| l == r) } /// Try to construct a new conversion if the conversion type is valid @@ -149,7 +147,7 @@ impl<'a> Conversion<'a> { impl ConversionType { /// Creates a conversion type if the type is allowed & conversion is valid - fn try_new(from: &str, to: &str) -> Option { + fn try_new(from: &str, to: &str) -> Option { if UNSIGNED_TYPES.contains(&from) { Some(ConversionType::FromUnsigned) } else if SIGNED_TYPES.contains(&from) { diff --git a/clippy_lints/src/enum_clike.rs b/clippy_lints/src/enum_clike.rs index 90cb78ae0366..f289c278a8c9 100644 --- a/clippy_lints/src/enum_clike.rs +++ b/clippy_lints/src/enum_clike.rs @@ -10,6 +10,7 @@ use rustc::ty; use rustc::ty::subst::InternalSubsts; use rustc::ty::util::IntTypeExt; use rustc::{declare_lint_pass, declare_tool_lint}; +use std::convert::TryFrom; use syntax::ast::{IntTy, UintTy}; declare_clippy_lint! { @@ -65,7 +66,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnportableVariant { match ty.sty { ty::Int(IntTy::Isize) => { let val = ((val as i128) << 64) >> 64; - if val <= i128::from(i32::max_value()) && val >= i128::from(i32::min_value()) { + if i32::try_from(val).is_ok() { continue; } }, From 00a5ef64a20c0b188ac27b5ba935cb291ea3bf85 Mon Sep 17 00:00:00 2001 From: pJunger Date: Tue, 14 May 2019 20:05:27 +0200 Subject: [PATCH 5/9] 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 48d941f9f8ba..3001147f92f5 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 000000000000..7febd6f37613 --- /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 71699a0733d8..a643354e2438 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 a3fed35eccca..f678f009621f 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 From ce9494a3df6ff490efd38ee3a6fbd0a7ba8c95dd Mon Sep 17 00:00:00 2001 From: pJunger Date: Tue, 14 May 2019 21:39:12 +0200 Subject: [PATCH 6/9] Changed impl to use symbols. --- clippy_lints/src/checked_conversions.rs | 76 +++++++++++++++---------- 1 file changed, 45 insertions(+), 31 deletions(-) diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 3001147f92f5..351928ca784c 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -1,12 +1,15 @@ //! lint on manually implemented checked conversions that could be transformed into `try_from` use if_chain::if_chain; +use lazy_static::lazy_static; 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_and_sugg, snippet_with_applicability, SpanlessEq}; +use syntax::symbol::Symbol; + +use crate::utils::{snippet_with_applicability, span_lint_and_sugg, sym, SpanlessEq}; declare_clippy_lint! { /// **What it does:** Checks for explicit bounds checking when casting. @@ -101,7 +104,7 @@ fn double_check<'a>(cx: &LateContext<'_, '_>, left: &'a Expr, right: &'a Expr) - struct Conversion<'a> { cvt: ConversionType, expr_to_cast: &'a Expr, - to_type: Option, + to_type: Option, } /// The kind of conversion that is checked @@ -137,8 +140,8 @@ impl<'a> Conversion<'a> { } /// Try to construct a new conversion if the conversion type is valid - fn try_new<'b>(expr_to_cast: &'a Expr, from_type: &'b str, to_type: String) -> Option> { - ConversionType::try_new(from_type, &to_type).map(|cvt| Conversion { + fn try_new<'b>(expr_to_cast: &'a Expr, from_type: Symbol, to_type: Symbol) -> Option> { + ConversionType::try_new(from_type, to_type).map(|cvt| Conversion { cvt, expr_to_cast, to_type: Some(to_type), @@ -157,13 +160,13 @@ impl<'a> Conversion<'a> { impl ConversionType { /// Creates a conversion type if the type is allowed & conversion is valid - fn try_new(from: &str, to: &str) -> Option { - if UNSIGNED_TYPES.contains(&from) { + fn try_new(from: Symbol, to: Symbol) -> Option { + if UINTS.contains(&from) { Some(ConversionType::FromUnsigned) - } else if SIGNED_TYPES.contains(&from) { - if UNSIGNED_TYPES.contains(&to) { + } else if SINTS.contains(&from) { + if UINTS.contains(&to) { Some(ConversionType::SignedToUnsigned) - } else if SIGNED_TYPES.contains(&to) { + } else if SINTS.contains(&to) { Some(ConversionType::SignedToSigned) } else { None @@ -179,10 +182,10 @@ fn check_upper_bound(expr: &Expr) -> Option> { if_chain! { if let ExprKind::Binary(ref op, ref left, ref right) = &expr.node; if let Some((candidate, check)) = normalize_le_ge(op, left, right); - if let Some((from, to)) = get_types_from_cast(check, "max_value", INT_TYPES); + if let Some((from, to)) = get_types_from_cast(check, *sym::max_value, &*INTS); then { - Conversion::try_new(candidate, &from, to) + Conversion::try_new(candidate, from, to) } else { None } @@ -219,31 +222,31 @@ fn check_lower_bound_zero<'a>(candidate: &'a Expr, check: &'a Expr) -> Option= (to_type::min_value() as from_type)` fn check_lower_bound_min<'a>(candidate: &'a Expr, check: &'a Expr) -> Option> { - if let Some((from, to)) = get_types_from_cast(check, "min_value", SIGNED_TYPES) { - Conversion::try_new(candidate, &from, to) + if let Some((from, to)) = get_types_from_cast(check, *sym::min_value, &*SINTS) { + Conversion::try_new(candidate, from, to) } else { None } } /// Tries to extract the from- and to-type from a cast expression -fn get_types_from_cast(expr: &Expr, func: &str, types: &[&str]) -> Option<(String, String)> { +fn get_types_from_cast(expr: &Expr, func: Symbol, types: &[Symbol]) -> Option<(Symbol, Symbol)> { // `to_type::maxmin_value() as from_type` - let call_from_cast: Option<(&Expr, String)> = if_chain! { + let call_from_cast: Option<(&Expr, Symbol)> = if_chain! { // to_type::maxmin_value(), from_type if let ExprKind::Cast(ref limit, ref from_type) = &expr.node; if let TyKind::Path(ref from_type_path) = &from_type.node; - if let Some(from_type_str) = int_ty_to_str(from_type_path); + if let Some(from_sym) = int_ty_to_sym(from_type_path); then { - Some((limit, from_type_str.to_string())) + Some((limit, from_sym)) } else { None } }; // `from_type::from(to_type::maxmin_value())` - let limit_from: Option<(&Expr, String)> = call_from_cast.or_else(|| { + let limit_from: Option<(&Expr, Symbol)> = call_from_cast.or_else(|| { if_chain! { // `from_type::from, to_type::maxmin_value()` if let ExprKind::Call(ref from_func, ref args) = &expr.node; @@ -252,10 +255,10 @@ fn get_types_from_cast(expr: &Expr, func: &str, types: &[&str]) -> Option<(Strin if let limit = &args[0]; // `from_type::from` if let ExprKind::Path(ref path) = &from_func.node; - if let Some(from_type) = get_implementing_type(path, INT_TYPES, "from"); + if let Some(from_sym) = get_implementing_type(path, &*INTS, *sym::from); then { - Some((limit, from_type)) + Some((limit, from_sym)) } else { None } @@ -282,17 +285,17 @@ fn get_types_from_cast(expr: &Expr, func: &str, types: &[&str]) -> Option<(Strin } /// Gets the type which implements the called function -fn get_implementing_type(path: &QPath, candidates: &[&str], function: &str) -> Option { +fn get_implementing_type(path: &QPath, candidates: &[Symbol], function: Symbol) -> Option { if_chain! { if let QPath::TypeRelative(ref ty, ref path) = &path; if path.ident.name == function; if let TyKind::Path(QPath::Resolved(None, ref tp)) = &ty.node; if let [int] = &*tp.segments; - let name = int.ident.as_str().get(); + let name = int.ident.name; if candidates.contains(&name); then { - Some(name.to_string()) + Some(name) } else { None } @@ -300,15 +303,15 @@ fn get_implementing_type(path: &QPath, candidates: &[&str], function: &str) -> O } /// Gets the type as a string, if it is a supported integer -fn int_ty_to_str(path: &QPath) -> Option<&str> { +fn int_ty_to_sym(path: &QPath) -> Option { if_chain! { if let QPath::Resolved(_, ref path) = *path; if let [ty] = &*path.segments; then { - INT_TYPES + INTS .iter() - .find(|c| (&ty.ident.name) == *c) + .find(|c| ty.ident.name == **c) .cloned() } else { None @@ -333,8 +336,19 @@ fn normalize_le_ge<'a>(op: &'a BinOp, left: &'a Expr, right: &'a Expr) -> Option } } -const UNSIGNED_TYPES: &[&str] = &["u8", "u16", "u32", "u64", "u128", "usize"]; -const SIGNED_TYPES: &[&str] = &["i8", "i16", "i32", "i64", "i128", "isize"]; -const INT_TYPES: &[&str] = &[ - "u8", "u16", "u32", "u64", "u128", "usize", "i8", "i16", "i32", "i64", "i128", "isize", -]; +lazy_static! { + static ref UINTS: [Symbol; 5] = [*sym::u8, *sym::u16, *sym::u32, *sym::u64, *sym::usize]; + static ref SINTS: [Symbol; 5] = [*sym::i8, *sym::i16, *sym::i32, *sym::i64, *sym::isize]; + static ref INTS: [Symbol; 10] = [ + *sym::u8, + *sym::u16, + *sym::u32, + *sym::u64, + *sym::usize, + *sym::i8, + *sym::i16, + *sym::i32, + *sym::i64, + *sym::isize + ]; +} From 7e0f2e3f1e6b2425e515f931a88f6ead570b28d3 Mon Sep 17 00:00:00 2001 From: pJunger Date: Tue, 14 May 2019 22:40:39 +0200 Subject: [PATCH 7/9] Removed unused lifetime. --- clippy_lints/src/checked_conversions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 351928ca784c..11ea8b7ea7a1 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -140,7 +140,7 @@ impl<'a> Conversion<'a> { } /// Try to construct a new conversion if the conversion type is valid - fn try_new<'b>(expr_to_cast: &'a Expr, from_type: Symbol, to_type: Symbol) -> Option> { + fn try_new(expr_to_cast: &'a Expr, from_type: Symbol, to_type: Symbol) -> Option> { ConversionType::try_new(from_type, to_type).map(|cvt| Conversion { cvt, expr_to_cast, From f627fbdc598e556e24ad1c4813ff074e37c3ba07 Mon Sep 17 00:00:00 2001 From: pJunger Date: Sat, 18 May 2019 10:54:03 +0200 Subject: [PATCH 8/9] Removed symbols again. --- clippy_lints/src/checked_conversions.rs | 65 ++++++++++--------------- 1 file changed, 26 insertions(+), 39 deletions(-) diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 11ea8b7ea7a1..3c335acb4d3a 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -1,15 +1,13 @@ //! lint on manually implemented checked conversions that could be transformed into `try_from` use if_chain::if_chain; -use lazy_static::lazy_static; 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 syntax::symbol::Symbol; -use crate::utils::{snippet_with_applicability, span_lint_and_sugg, sym, SpanlessEq}; +use crate::utils::{snippet_with_applicability, span_lint_and_sugg, SpanlessEq}; declare_clippy_lint! { /// **What it does:** Checks for explicit bounds checking when casting. @@ -104,7 +102,7 @@ fn double_check<'a>(cx: &LateContext<'_, '_>, left: &'a Expr, right: &'a Expr) - struct Conversion<'a> { cvt: ConversionType, expr_to_cast: &'a Expr, - to_type: Option, + to_type: Option<&'a str>, } /// The kind of conversion that is checked @@ -140,7 +138,7 @@ impl<'a> Conversion<'a> { } /// Try to construct a new conversion if the conversion type is valid - fn try_new(expr_to_cast: &'a Expr, from_type: Symbol, to_type: Symbol) -> Option> { + fn try_new(expr_to_cast: &'a Expr, from_type: &str, to_type: &'a str) -> Option> { ConversionType::try_new(from_type, to_type).map(|cvt| Conversion { cvt, expr_to_cast, @@ -160,7 +158,7 @@ impl<'a> Conversion<'a> { impl ConversionType { /// Creates a conversion type if the type is allowed & conversion is valid - fn try_new(from: Symbol, to: Symbol) -> Option { + fn try_new(from: &str, to: &str) -> Option { if UINTS.contains(&from) { Some(ConversionType::FromUnsigned) } else if SINTS.contains(&from) { @@ -182,7 +180,7 @@ fn check_upper_bound(expr: &Expr) -> Option> { if_chain! { if let ExprKind::Binary(ref op, ref left, ref right) = &expr.node; if let Some((candidate, check)) = normalize_le_ge(op, left, right); - if let Some((from, to)) = get_types_from_cast(check, *sym::max_value, &*INTS); + if let Some((from, to)) = get_types_from_cast(check, MAX_VALUE, INTS); then { Conversion::try_new(candidate, from, to) @@ -222,7 +220,7 @@ fn check_lower_bound_zero<'a>(candidate: &'a Expr, check: &'a Expr) -> Option= (to_type::min_value() as from_type)` fn check_lower_bound_min<'a>(candidate: &'a Expr, check: &'a Expr) -> Option> { - if let Some((from, to)) = get_types_from_cast(check, *sym::min_value, &*SINTS) { + if let Some((from, to)) = get_types_from_cast(check, MIN_VALUE, SINTS) { Conversion::try_new(candidate, from, to) } else { None @@ -230,9 +228,9 @@ fn check_lower_bound_min<'a>(candidate: &'a Expr, check: &'a Expr) -> Option Option<(Symbol, Symbol)> { +fn get_types_from_cast<'a>(expr: &'a Expr, func: &'a str, types: &'a [&str]) -> Option<(&'a str, &'a str)> { // `to_type::maxmin_value() as from_type` - let call_from_cast: Option<(&Expr, Symbol)> = if_chain! { + let call_from_cast: Option<(&Expr, &str)> = if_chain! { // to_type::maxmin_value(), from_type if let ExprKind::Cast(ref limit, ref from_type) = &expr.node; if let TyKind::Path(ref from_type_path) = &from_type.node; @@ -246,7 +244,7 @@ fn get_types_from_cast(expr: &Expr, func: Symbol, types: &[Symbol]) -> Option<(S }; // `from_type::from(to_type::maxmin_value())` - let limit_from: Option<(&Expr, Symbol)> = call_from_cast.or_else(|| { + let limit_from: Option<(&Expr, &str)> = call_from_cast.or_else(|| { if_chain! { // `from_type::from, to_type::maxmin_value()` if let ExprKind::Call(ref from_func, ref args) = &expr.node; @@ -255,7 +253,7 @@ fn get_types_from_cast(expr: &Expr, func: Symbol, types: &[Symbol]) -> Option<(S if let limit = &args[0]; // `from_type::from` if let ExprKind::Path(ref path) = &from_func.node; - if let Some(from_sym) = get_implementing_type(path, &*INTS, *sym::from); + if let Some(from_sym) = get_implementing_type(path, INTS, FROM); then { Some((limit, from_sym)) @@ -285,17 +283,16 @@ fn get_types_from_cast(expr: &Expr, func: Symbol, types: &[Symbol]) -> Option<(S } /// Gets the type which implements the called function -fn get_implementing_type(path: &QPath, candidates: &[Symbol], function: Symbol) -> Option { +fn get_implementing_type<'a>(path: &QPath, candidates: &'a [&str], function: &str) -> Option<&'a str> { if_chain! { if let QPath::TypeRelative(ref ty, ref path) = &path; - if path.ident.name == function; + if path.ident.name.as_str() == function; if let TyKind::Path(QPath::Resolved(None, ref tp)) = &ty.node; if let [int] = &*tp.segments; - let name = int.ident.name; - if candidates.contains(&name); + let name = &int.ident.name.as_str(); then { - Some(name) + candidates.iter().find(|c| name == *c).cloned() } else { None } @@ -303,16 +300,14 @@ fn get_implementing_type(path: &QPath, candidates: &[Symbol], function: Symbol) } /// Gets the type as a string, if it is a supported integer -fn int_ty_to_sym(path: &QPath) -> Option { +fn int_ty_to_sym(path: &QPath) -> Option<&str> { if_chain! { if let QPath::Resolved(_, ref path) = *path; if let [ty] = &*path.segments; + let name = &ty.ident.name.as_str(); then { - INTS - .iter() - .find(|c| ty.ident.name == **c) - .cloned() + INTS.iter().find(|c| name == *c).cloned() } else { None } @@ -328,7 +323,7 @@ fn transpose(lhs: Option, rhs: Option) -> Option<(T, U)> { } /// Will return the expressions as if they were expr1 <= expr2 -fn normalize_le_ge<'a>(op: &'a BinOp, left: &'a Expr, right: &'a Expr) -> Option<(&'a Expr, &'a Expr)> { +fn normalize_le_ge<'a>(op: &BinOp, left: &'a Expr, right: &'a Expr) -> Option<(&'a Expr, &'a Expr)> { match op.node { BinOpKind::Le => Some((left, right)), BinOpKind::Ge => Some((right, left)), @@ -336,19 +331,11 @@ fn normalize_le_ge<'a>(op: &'a BinOp, left: &'a Expr, right: &'a Expr) -> Option } } -lazy_static! { - static ref UINTS: [Symbol; 5] = [*sym::u8, *sym::u16, *sym::u32, *sym::u64, *sym::usize]; - static ref SINTS: [Symbol; 5] = [*sym::i8, *sym::i16, *sym::i32, *sym::i64, *sym::isize]; - static ref INTS: [Symbol; 10] = [ - *sym::u8, - *sym::u16, - *sym::u32, - *sym::u64, - *sym::usize, - *sym::i8, - *sym::i16, - *sym::i32, - *sym::i64, - *sym::isize - ]; -} +// 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 SINTS: &[&str] = &["i8", "i16", "i32", "i64", "isize"]; +const INTS: &[&str] = &["u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize"]; From 565feb0bc11116671bb1b71db6fe602ef73b3ef1 Mon Sep 17 00:00:00 2001 From: pJunger Date: Sat, 18 May 2019 14:53:56 +0200 Subject: [PATCH 9/9] Updated README. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e670924de2b1..a779d0dd1f8a 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 302 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 303 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: