Extend `useless_conversion` lint with TryFrom

5 files changed, 112 insertions, 20 deletions

diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs
index 95921518986..0b080d9be2c 100644
--- a/clippy_lints/src/useless_conversion.rs
+++ b/clippy_lints/src/useless_conversion.rs
@@ -1,13 +1,16 @@
 use crate::utils::{
-    match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_sugg,
+    is_type_diagnostic_item, match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite,
+    span_lint_and_help, span_lint_and_sugg,
 };
+use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for `Into`/`From`/`IntoIter` calls that useless converts
+    /// **What it does:** Checks for `Into`, `From`, `TryFrom`,`IntoIter` calls that useless converts
     /// to the same type as caller.
     ///
     /// **Why is this bad?** Redundant code.
@@ -26,7 +29,7 @@ declare_clippy_lint! {
     /// ```
     pub USELESS_CONVERSION,
     complexity,
-    "calls to `Into`/`From`/`IntoIter` that performs useless conversions to the same type"
+    "calls to `Into`, `From`, `TryFrom`, `IntoIter` that performs useless conversions to the same type"
 }
 
 #[derive(Default)]
@@ -68,7 +71,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
                             cx,
                             USELESS_CONVERSION,
                             e.span,
-                            "useless conversion",
+                            "Useless conversion to the same type",
                             "consider removing `.into()`",
                             sugg,
                             Applicability::MachineApplicable, // snippet
@@ -84,7 +87,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
                             cx,
                             USELESS_CONVERSION,
                             e.span,
-                            "useless conversion",
+                            "Useless conversion to the same type",
                             "consider removing `.into_iter()`",
                             sugg,
                             Applicability::MachineApplicable, // snippet
@@ -94,11 +97,35 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
             },
 
             ExprKind::Call(ref path, ref args) => {
-                if let ExprKind::Path(ref qpath) = path.kind {
-                    if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id() {
+                if_chain! {
+                    if args.len() == 1;
+                    if let ExprKind::Path(ref qpath) = path.kind;
+                    if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id();
+                    let a = cx.tables.expr_ty(e);
+                    let b = cx.tables.expr_ty(&args[0]);
+
+                    then {
+                        if_chain! {
+                            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 same_tys(cx, a_type, b);
+
+                            then {
+                                let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from"));
+                                span_lint_and_help(
+                                    cx,
+                                    USELESS_CONVERSION,
+                                    e.span,
+                                    "Useless conversion to the same type",
+                                    None,
+                                    &hint,
+                                );
+                            }
+                        }
+
                         if match_def_path(cx, def_id, &paths::FROM_FROM) {
-                            let a = cx.tables.expr_ty(e);
-                            let b = cx.tables.expr_ty(&args[0]);
                             if same_tys(cx, a, b) {
                                 let sugg = snippet(cx, args[0].span.source_callsite(), "<expr>").into_owned();
                                 let sugg_msg =
@@ -107,7 +134,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
                                     cx,
                                     USELESS_CONVERSION,
                                     e.span,
-                                    "useless conversion",
+                                    "Useless conversion to the same type",
                                     &sugg_msg,
                                     sugg,
                                     Applicability::MachineApplicable, // snippet
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
index b3ad2ad9d99..e00d726282a 100644
--- a/clippy_lints/src/utils/paths.rs
+++ b/clippy_lints/src/utils/paths.rs
@@ -128,6 +128,7 @@ pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"
 pub const TO_STRING: [&str; 3] = ["alloc", "string", "ToString"];
 pub const TO_STRING_METHOD: [&str; 4] = ["alloc", "string", "ToString", "to_string"];
 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 VEC: [&str; 3] = ["alloc", "vec", "Vec"];
diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr
index 7df3507edfd..0b2947f7d62 100644
--- a/tests/ui/useless_conversion.stderr
+++ b/tests/ui/useless_conversion.stderr
@@ -1,4 +1,4 @@
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:6:13
    |
 LL |     let _ = T::from(val);
@@ -10,55 +10,55 @@ note: the lint level is defined here
 LL | #![deny(clippy::useless_conversion)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:7:5
    |
 LL |     val.into()
    |     ^^^^^^^^^^ help: consider removing `.into()`: `val`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:19:22
    |
 LL |         let _: i32 = 0i32.into();
    |                      ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:51:21
    |
 LL |     let _: String = "foo".to_string().into();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:52:21
    |
 LL |     let _: String = From::from("foo".to_string());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:53:13
    |
 LL |     let _ = String::from("foo".to_string());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:54:13
    |
 LL |     let _ = String::from(format!("A: {:04}", 123));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:55:13
    |
 LL |     let _ = "".lines().into_iter();
    |             ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:56:13
    |
 LL |     let _ = vec![1, 2, 3].into_iter().into_iter();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()`
 
-error: useless conversion
+error: Useless conversion to the same type
   --> $DIR/useless_conversion.rs:57:21
    |
 LL |     let _: String = format!("Hello {}", "world").into();
diff --git a/tests/ui/useless_conversion_try.rs b/tests/ui/useless_conversion_try.rs
new file mode 100644
index 00000000000..abf0c891b52
--- /dev/null
+++ b/tests/ui/useless_conversion_try.rs
@@ -0,0 +1,25 @@
+#![deny(clippy::useless_conversion)]
+
+use std::convert::TryFrom;
+
+fn test_generic<T: Copy>(val: T) -> T {
+    T::try_from(val).unwrap()
+}
+
+fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
+    let _ = U::try_from(val).unwrap();
+}
+
+fn main() {
+    test_generic(10i32);
+    test_generic2::<i32, i32>(10i32);
+
+    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 = TryFrom::try_from("foo".to_string()).unwrap();
+    let _ = String::try_from("foo".to_string()).unwrap();
+    let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
+}
diff --git a/tests/ui/useless_conversion_try.stderr b/tests/ui/useless_conversion_try.stderr
new file mode 100644
index 00000000000..b3cb01fbe32
--- /dev/null
+++ b/tests/ui/useless_conversion_try.stderr
@@ -0,0 +1,39 @@
+error: Useless conversion to the same type
+  --> $DIR/useless_conversion_try.rs:6:5
+   |
+LL |     T::try_from(val).unwrap()
+   |     ^^^^^^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/useless_conversion_try.rs:1:9
+   |
+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
+   |
+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
+   |
+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
+   |
+LL |     let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider removing `String::try_from()`
+
+error: aborting due to 4 previous errors
+