]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/checked_conversions.rs
Improve `implicit_return`
[rust.git] / clippy_lints / src / checked_conversions.rs
index d9776dd50a836add3ab5635534b41bbe225c6180..6a2666bc6c0111026890f8ef361af6cb1fa0682d 100644 (file)
@@ -1,14 +1,18 @@
 //! lint on manually implemented checked conversions that could be transformed into `try_from`
 
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{meets_msrv, SpanlessEq};
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, QPath, TyKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_semver::RustcVersion;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 
-use crate::utils::{snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
+const CHECKED_CONVERSIONS_MSRV: RustcVersion = RustcVersion::new(1, 34, 0);
 
 declare_clippy_lint! {
     /// **What it does:** Checks for explicit bounds checking when casting.
     "`try_from` could replace manual bounds checking when casting"
 }
 
-declare_lint_pass!(CheckedConversions => [CHECKED_CONVERSIONS]);
+pub struct CheckedConversions {
+    msrv: Option<RustcVersion>,
+}
+
+impl CheckedConversions {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self { msrv }
+    }
+}
+
+impl_lint_pass!(CheckedConversions => [CHECKED_CONVERSIONS]);
+
+impl<'tcx> LateLintPass<'tcx> for CheckedConversions {
+    fn check_expr(&mut self, cx: &LateContext<'_>, item: &Expr<'_>) {
+        if !meets_msrv(self.msrv.as_ref(), &CHECKED_CONVERSIONS_MSRV) {
+            return;
+        }
 
-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.kind;
+            if let ExprKind::Binary(op, left, right) = &item.kind;
 
             then {
                 match op.node {
@@ -58,28 +77,24 @@ fn check_expr(&mut self, cx: &LateContext<'_, '_>, item: &Expr<'_>) {
             }
         };
 
-        if_chain! {
-            if let Some(cv) = result;
-            if let Some(to_type) = cv.to_type;
-
-            then {
+        if let Some(cv) = result {
+            if let Some(to_type) = cv.to_type {
                 let mut applicability = Applicability::MachineApplicable;
-                let snippet = snippet_with_applicability(cx, cv.expr_to_cast.span, "_", &mut
-                                applicability);
+                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.",
+                    "checked cast can be simplified",
                     "try",
-                    format!("{}::try_from({}).is_ok()",
-                            to_type,
-                            snippet),
-                    applicability
+                    format!("{}::try_from({}).is_ok()", to_type, snippet),
+                    applicability,
                 );
             }
         }
     }
+
+    extract_msrv_attr!(LateContext);
 }
 
 /// Searches for a single check from unsigned to _ is done
@@ -89,12 +104,12 @@ fn single_check<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> {
 }
 
 /// Searches for a combination of upper & lower bound checks
-fn double_check<'a>(cx: &LateContext<'_, '_>, left: &'a Expr<'_>, right: &'a Expr<'_>) -> Option<Conversion<'a>> {
+fn double_check<'a>(cx: &LateContext<'_>, left: &'a Expr<'_>, right: &'a Expr<'_>) -> Option<Conversion<'a>> {
     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.zip(lower).and_then(|(l, r)| l.combine(r, cx))
     };
 
     upper_lower(left, right).or_else(|| upper_lower(right, left))
@@ -118,7 +133,7 @@ enum ConversionType {
 
 impl<'a> Conversion<'a> {
     /// Combine multiple conversions if the are compatible
-    pub fn combine(self, other: Self, cx: &LateContext<'_, '_>) -> Option<Conversion<'a>> {
+    pub fn combine(self, other: Self, cx: &LateContext<'_>) -> Option<Conversion<'a>> {
         if self.is_compatible(&other, cx) {
             // Prefer a Conversion that contains a type-constraint
             Some(if self.to_type.is_some() { self } else { other })
@@ -129,7 +144,7 @@ pub fn combine(self, other: Self, cx: &LateContext<'_, '_>) -> Option<Conversion
 
     /// 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 {
+    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))
@@ -137,7 +152,10 @@ pub fn is_compatible(&self, other: &Self, cx: &LateContext<'_, '_>) -> bool {
 
     /// 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_or(true, |(l, r)| l == r)
+        match (self.to_type, other.to_type) {
+            (Some(l), Some(r)) => l == r,
+            _ => true,
+        }
     }
 
     /// Try to construct a new conversion if the conversion type is valid
@@ -182,9 +200,9 @@ fn try_new(from: &str, to: &str) -> Option<Self> {
 /// Check for `expr <= (to_type::MAX as from_type)`
 fn check_upper_bound<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> {
     if_chain! {
-         if let ExprKind::Binary(ref op, ref left, ref right) = &expr.kind;
+         if let ExprKind::Binary(ref op, left, right) = &expr.kind;
          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 {
              Conversion::try_new(candidate, from, to)
@@ -201,7 +219,7 @@ fn check_function<'a>(candidate: &'a Expr<'a>, check: &'a Expr<'a>) -> Option<Co
     }
 
     // First of we need a binary containing the expression & the cast
-    if let ExprKind::Binary(ref op, ref left, ref right) = &expr.kind {
+    if let ExprKind::Binary(ref op, left, right) = &expr.kind {
         normalize_le_ge(op, right, left).and_then(|(l, r)| check_function(l, r))
     } else {
         None
@@ -224,7 +242,7 @@ fn check_lower_bound_zero<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> O
 
 /// Check for `expr >= (to_type::MIN as from_type)`
 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)
     } else {
         None
@@ -232,11 +250,17 @@ 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
-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`
+fn get_types_from_cast<'a>(
+    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! {
-        // to_type::maxmin_value(), from_type
-        if let ExprKind::Cast(ref limit, ref from_type) = &expr.kind;
+        // to_type::max_value(), from_type
+        if let ExprKind::Cast(limit, from_type) = &expr.kind;
         if let TyKind::Path(ref from_type_path) = &from_type.kind;
         if let Some(from_sym) = int_ty_to_sym(from_type_path);
 
@@ -247,17 +271,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(|| {
         if_chain! {
-            // `from_type::from, to_type::maxmin_value()`
-            if let ExprKind::Call(ref from_func, ref args) = &expr.kind;
-            // `to_type::maxmin_value()`
+            // `from_type::from, to_type::max_value()`
+            if let ExprKind::Call(from_func, args) = &expr.kind;
+            // `to_type::max_value()`
             if args.len() == 1;
             if let limit = &args[0];
             // `from_type::from`
             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 {
                 Some((limit, from_sym))
@@ -268,35 +292,38 @@ fn get_types_from_cast<'a>(expr: &'a Expr<'_>, func: &'a str, types: &'a [&str])
     });
 
     if let Some((limit, from_type)) = limit_from {
-        if_chain! {
-            if let ExprKind::Call(ref fun_name, _) = &limit.kind;
-            // `to_type, maxmin_value`
-            if let ExprKind::Path(ref path) = &fun_name.kind;
-            // `to_type`
-            if let Some(to_type) = get_implementing_type(path, types, func);
-
-            then {
-                Some((from_type, to_type))
-            } else {
-                None
-            }
+        match limit.kind {
+            // `from_type::from(_)`
+            ExprKind::Call(path, _) => {
+                if let ExprKind::Path(ref path) = path.kind {
+                    // `to_type`
+                    if let Some(to_type) = get_implementing_type(path, types, func) {
+                        return Some((from_type, to_type));
+                    }
+                }
+            },
+            // `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
 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 let QPath::TypeRelative(ty, path) = &path;
         if path.ident.name.as_str() == function;
-        if let TyKind::Path(QPath::Resolved(None, ref tp)) = &ty.kind;
+        if let TyKind::Path(QPath::Resolved(None, tp)) = &ty.kind;
         if let [int] = &*tp.segments;
-        let name = &int.ident.name.as_str();
-
         then {
-            candidates.iter().find(|c| name == *c).cloned()
+            let name = &int.ident.name.as_str();
+            candidates.iter().find(|c| name == *c).copied()
         } else {
             None
         }
@@ -306,26 +333,17 @@ fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function:
 /// Gets the type as a string, if it is a supported integer
 fn int_ty_to_sym<'tcx>(path: &QPath<'_>) -> Option<&'tcx str> {
     if_chain! {
-        if let QPath::Resolved(_, ref path) = *path;
+        if let QPath::Resolved(_, path) = *path;
         if let [ty] = &*path.segments;
-        let name = &ty.ident.name.as_str();
-
         then {
-            INTS.iter().find(|c| name == *c).cloned()
+            let name = &ty.ident.name.as_str();
+            INTS.iter().find(|c| name == *c).copied()
         } else {
             None
         }
     }
 }
 
-/// (Option<T>, Option<U>) -> Option<(T, U)>
-fn transpose<T, U>(lhs: Option<T>, rhs: Option<U>) -> 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: &BinOp, left: &'a Expr<'a>, right: &'a Expr<'a>) -> Option<(&'a Expr<'a>, &'a Expr<'a>)> {
     match op.node {
@@ -336,10 +354,6 @@ fn normalize_le_ge<'a>(op: &BinOp, left: &'a Expr<'a>, right: &'a Expr<'a>) -> O
 }
 
 // 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"];