Extend useless_conversion lint with TryInto

ThibsG · ThibsG · commit 705bfdcc467c · 2020-05-25T20:00:39.000+02:00
diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs
@@ -10,8 +10,8 @@ use rustc_middle::ty;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for `Into`, `From`, `TryFrom`,`IntoIter` calls that useless converts
-    /// to the same type as caller.
+    /// **What it does:** Checks for `Into`, `TryInto`, `From`, `TryFrom`,`IntoIter` calls
+    /// that useless converts to the same type as caller.
     ///
     /// **Why is this bad?** Redundant code.
     ///
@@ -29,7 +29,7 @@ declare_clippy_lint! {
     /// ```
     pub USELESS_CONVERSION,
     complexity,
-    "calls to `Into`, `From`, `TryFrom`, `IntoIter` that performs useless conversions to the same type"
+    "calls to `Into`, `TryInto`, `From`, `TryFrom`, `IntoIter` that performs useless conversions to the same type"
 }
 
 #[derive(Default)]
@@ -39,6 +39,7 @@ pub struct UselessConversion {
 
 impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);
 
+#[allow(clippy::too_many_lines)]
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
         if e.span.from_expansion() {
@@ -66,7 +67,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
                     let b = cx.tables.expr_ty(&args[0]);
                     if same_tys(cx, a, b) {
                         let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string();
-
                         span_lint_and_sugg(
                             cx,
                             USELESS_CONVERSION,
@@ -94,6 +94,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
                         );
                     }
                 }
+                if match_trait_method(cx, e, &paths::TRY_INTO_TRAIT) && &*name.ident.as_str() == "try_into" {
+                    if_chain! {
+                        let a = cx.tables.expr_ty(e);
+                        let b = cx.tables.expr_ty(&args[0]);
+                        if is_type_diagnostic_item(cx, a, sym!(result_type));
+                        if let ty::Adt(_, substs) = a.kind;
+                        if let Some(a_type) = substs.types().next();
+                        if same_tys(cx, a_type, b);
+
+                        then {
+                            span_lint_and_help(
+                                cx,
+                                USELESS_CONVERSION,
+                                e.span,
+                                "Useless conversion to the same type",
+                                None,
+                                "consider removing `.try_into()`",
+                            );
+                        }
+                    }
+                }
             },
 
             ExprKind::Call(ref path, ref args) => {
@@ -109,7 +130,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
                             if match_def_path(cx, def_id, &paths::TRY_FROM);
                             if is_type_diagnostic_item(cx, a, sym!(result_type));
                             if let ty::Adt(_, substs) = a.kind;
-                            if let Some(a_type) = substs.types().nth(0);
+                            if let Some(a_type) = substs.types().next();
                             if same_tys(cx, a_type, b);
 
                             then {
@@ -125,8 +146,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
                             }
                         }
 
-                        if match_def_path(cx, def_id, &paths::FROM_FROM) {
-                            if same_tys(cx, a, b) {
+                        if_chain! {
+                            if match_def_path(cx, def_id, &paths::FROM_FROM);
+                            if same_tys(cx, a, b);
+
+                            then {
                                 let sugg = snippet(cx, args[0].span.source_callsite(), "<expr>").into_owned();
                                 let sugg_msg =
                                     format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
@@ -131,6 +131,7 @@ pub const TRANSMUTE: [&str; 4] = ["core", "intrinsics", "", "transmute"];
 pub const TRY_FROM: [&str; 4] = ["core", "convert", "TryFrom", "try_from"];
 pub const TRY_FROM_ERROR: [&str; 4] = ["std", "ops", "Try", "from_error"];
 pub const TRY_INTO_RESULT: [&str; 4] = ["std", "ops", "Try", "into_result"];
+pub const TRY_INTO_TRAIT: [&str; 3] = ["core", "convert", "TryInto"];
 pub const VEC: [&str; 3] = ["alloc", "vec", "Vec"];
 pub const VEC_AS_MUT_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_mut_slice"];
 pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -2421,7 +2421,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
     Lint {
         name: "useless_conversion",
         group: "complexity",
-        desc: "calls to `Into`/`From`/`IntoIter` that performs useless conversions to the same type",
+        desc: "calls to `Into`, `TryInto`, `From`, `TryFrom`, `IntoIter` that performs useless conversions to the same type",
         deprecation: None,
         module: "useless_conversion",
     },
diff --git a/tests/ui/useless_conversion_try.rs b/tests/ui/useless_conversion_try.rs
@@ -1,25 +1,34 @@
 #![deny(clippy::useless_conversion)]
 
-use std::convert::TryFrom;
+use std::convert::{TryFrom, TryInto};
 
 fn test_generic<T: Copy>(val: T) -> T {
-    T::try_from(val).unwrap()
+    let _ = T::try_from(val).unwrap();
+    val.try_into().unwrap()
 }
 
 fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
+    // ok
+    let _: i32 = val.try_into().unwrap();
+    let _: U = val.try_into().unwrap();
     let _ = U::try_from(val).unwrap();
 }
 
 fn main() {
     test_generic(10i32);
     test_generic2::<i32, i32>(10i32);
 
+    let _: String = "foo".try_into().unwrap();
     let _: String = TryFrom::try_from("foo").unwrap();
     let _ = String::try_from("foo").unwrap();
     #[allow(clippy::useless_conversion)]
-    let _ = String::try_from("foo").unwrap();
-
+    {
+        let _ = String::try_from("foo").unwrap();
+        let _: String = "foo".try_into().unwrap();
+    }
+    let _: String = "foo".to_string().try_into().unwrap();
     let _: String = TryFrom::try_from("foo".to_string()).unwrap();
     let _ = String::try_from("foo".to_string()).unwrap();
     let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
+    let _: String = format!("Hello {}", "world").try_into().unwrap();
 }
diff --git a/tests/ui/useless_conversion_try.stderr b/tests/ui/useless_conversion_try.stderr
@@ -1,8 +1,8 @@
 error: Useless conversion to the same type
-  --> $DIR/useless_conversion_try.rs:6:5
+  --> $DIR/useless_conversion_try.rs:6:13
    |
-LL |     T::try_from(val).unwrap()
-   |     ^^^^^^^^^^^^^^^^
+LL |     let _ = T::try_from(val).unwrap();
+   |             ^^^^^^^^^^^^^^^^
    |
 note: the lint level is defined here
   --> $DIR/useless_conversion_try.rs:1:9
@@ -12,28 +12,52 @@ LL | #![deny(clippy::useless_conversion)]
    = help: consider removing `T::try_from()`
 
 error: Useless conversion to the same type
-  --> $DIR/useless_conversion_try.rs:22:21
+  --> $DIR/useless_conversion_try.rs:7:5
+   |
+LL |     val.try_into().unwrap()
+   |     ^^^^^^^^^^^^^^
+   |
+   = help: consider removing `.try_into()`
+
+error: Useless conversion to the same type
+  --> $DIR/useless_conversion_try.rs:29:21
+   |
+LL |     let _: String = "foo".to_string().try_into().unwrap();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider removing `.try_into()`
+
+error: Useless conversion to the same type
+  --> $DIR/useless_conversion_try.rs:30:21
    |
 LL |     let _: String = TryFrom::try_from("foo".to_string()).unwrap();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `TryFrom::try_from()`
 
 error: Useless conversion to the same type
-  --> $DIR/useless_conversion_try.rs:23:13
+  --> $DIR/useless_conversion_try.rs:31:13
    |
 LL |     let _ = String::try_from("foo".to_string()).unwrap();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `String::try_from()`
 
 error: Useless conversion to the same type
-  --> $DIR/useless_conversion_try.rs:24:13
+  --> $DIR/useless_conversion_try.rs:32:13
    |
 LL |     let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `String::try_from()`
 
-error: aborting due to 4 previous errors
+error: Useless conversion to the same type
+  --> $DIR/useless_conversion_try.rs:33:21
+   |
+LL |     let _: String = format!("Hello {}", "world").try_into().unwrap();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider removing `.try_into()`
+
+error: aborting due to 7 previous errors
 
