Fix4894 Using Into and TryInto instead of From and TryFrom

CHANGELOG.md

@@ -2091,6 +2091,7 @@ Released 2018-09-13
 [`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
 [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
 [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
+[`from_instead_of_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_instead_of_into
 [`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect
 [`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into
 [`from_str_radix_10`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_str_radix_10

clippy_lints/src/from_instead_of_into.rs

@@ -0,0 +1,119 @@
+use crate::utils::span_lint_and_then;
+use crate::utils::snippet_opt;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{GenericBound, WherePredicate};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::sym;
+
+declare_clippy_lint! {
+    /// **What it does:** Checking for using of From or TryFrom trait as a generic bound.
+    ///
+    /// **Why is this bad?** Into and TryInto are supersets of From and TryFrom. Due to
+    /// coherence rules, sometimes From and TryFrom are forbid to implemented but Into and
+    /// TryInto are not. So Into is a more generic bound than From, We should choose Into or
+    /// TryInto instead of From or TryFrom.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// fn foo<T>(a: T) where u32: From<T> {}
+    /// fn bar<T>(a: T) where u32: TryFrom<T> {}
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn foo<T>(a: T) where T: Into<u32> {}
+    /// fn bar<T>(a: T) where T: TryInto<u32> {}
+    /// ```
+    pub FROM_INSTEAD_OF_INTO,
+    style,
+    "Into or TryInto trait is a better choice than From or TryFrom trait as a generic bound"
+}
+
+declare_lint_pass!(FromInsteadOfInto => [FROM_INSTEAD_OF_INTO]);
+
+impl LateLintPass<'tcx> for FromInsteadOfInto {
+    fn check_where_predicate(&mut self, cx: &LateContext<'tcx>, wp: &'tcx WherePredicate<'tcx>) {
+        fn is_target_generic_bound(cx: &LateContext<'tcx>, b: &GenericBound<'_>) -> bool {
+            if_chain! {
+                if let Some(r) = b.trait_ref();
+                if let Some(def_id) = r.trait_def_id();
+                then {
+                    cx.tcx.is_diagnostic_item(sym::from_trait, def_id) ||
+                    cx.tcx.is_diagnostic_item(sym::try_from_trait, def_id)
+                } else {
+                    false
+                }
+            }
+        }
+
+        if let WherePredicate::BoundPredicate(wbp) = wp {
+            if_chain! {
+                let bounds = wbp.bounds;
+                if let Some(position) = bounds.iter().position(|b| is_target_generic_bound(cx, b));
+                let target_bound = &bounds[position];
+                if let Some(tr_ref) = target_bound.trait_ref();
+                if let Some(def_id) = tr_ref.trait_def_id();
+                if let Some(last_seg) = tr_ref.path.segments.last();
+                if let Some(generic_arg) = last_seg.args().args.first();
+                if let Some(bounded_ty) = snippet_opt(cx, wbp.bounded_ty.span);
+                if let Some(generic_arg_of_from_or_try_from) = snippet_opt(cx, generic_arg.span());
+                then {
+                    let replace_trait_name;
+                    let target_trait_name;
+                    if cx.tcx.is_diagnostic_item(sym::from_trait, def_id) {
+                        replace_trait_name = "Into";
+                        target_trait_name = "From";
+                    } else if cx.tcx.is_diagnostic_item(sym::try_from_trait, def_id) {
+                        replace_trait_name = "TryInto";
+                        target_trait_name = "TryFrom";
+                    } else {
+                        return;
+                    }
+                    let message = format!("{} trait is preferable than {} as a generic bound", replace_trait_name, target_trait_name);
+                    let switched_predicate = format!("{}: {}<{}>", generic_arg_of_from_or_try_from, replace_trait_name, bounded_ty);
+
+                    let low;
+                    if position == 0 {
+                        low = wp.span().lo();
+                    } else {
+                        let previous_bound = &bounds[position -1];
+                        low = previous_bound.span().hi();
+                    }
+                    let removed_span = target_bound.span().with_lo(low);
+
+                    span_lint_and_then(
+                        cx,
+                        FROM_INSTEAD_OF_INTO,
+                        wp.span(),
+                        &message,
+                        |diag| {
+                            diag.span_suggestion(
+                                removed_span,
+                                &format!("remove {} bound", target_trait_name),
+                                "".to_string(),
+                                Applicability::MaybeIncorrect,
+                            );
+
+                            let sugg;
+                            if bounds.len() == 1 {
+                                sugg = switched_predicate;
+                            } else {
+                                sugg = format!(", {}", switched_predicate);
+                            }
+                            diag.span_suggestion(
+                                wp.span().with_lo(wp.span().hi()),
+                                "Add this bound predicate",
+                                sugg,
+                                Applicability::MaybeIncorrect,
+                            );
+                        }
+                    );
+                }
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -12,6 +12,7 @@
 #![feature(rustc_private)]
 #![feature(stmt_expr_attributes)]
 #![feature(control_flow_enum)]
+#![feature(array_map)]
 #![recursion_limit = "512"]
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 #![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)]
@@ -210,6 +211,7 @@ mod float_literal;
 mod floating_point_arithmetic;
 mod format;
 mod formatting;
+mod from_instead_of_into;
 mod from_over_into;
 mod from_str_radix_10;
 mod functions;
@@ -641,6 +643,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         &formatting::SUSPICIOUS_ELSE_FORMATTING,
         &formatting::SUSPICIOUS_UNARY_OP_FORMATTING,
+        &from_instead_of_into::FROM_INSTEAD_OF_INTO,
         &from_over_into::FROM_OVER_INTO,
         &from_str_radix_10::FROM_STR_RADIX_10,
         &functions::DOUBLE_MUST_USE,
@@ -1268,6 +1271,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box redundant_slicing::RedundantSlicing);
     store.register_late_pass(|| box from_str_radix_10::FromStrRadix10);
     store.register_late_pass(|| box manual_map::ManualMap);
+    store.register_late_pass(|| box from_instead_of_into::FromInsteadOfInto);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1482,6 +1486,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
         LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
         LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
+        LintId::of(&from_instead_of_into::FROM_INSTEAD_OF_INTO),
         LintId::of(&from_over_into::FROM_OVER_INTO),
         LintId::of(&from_str_radix_10::FROM_STR_RADIX_10),
         LintId::of(&functions::DOUBLE_MUST_USE),
@@ -1739,6 +1744,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
         LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
         LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
+        LintId::of(&from_instead_of_into::FROM_INSTEAD_OF_INTO),
         LintId::of(&from_over_into::FROM_OVER_INTO),
         LintId::of(&from_str_radix_10::FROM_STR_RADIX_10),
         LintId::of(&functions::DOUBLE_MUST_USE),

tests/ui/from_instead_of_into.fixed

@@ -0,0 +1,39 @@
+// run-rustfix
+#![allow(unused_imports)]
+#![allow(unused_variables)]
+#![allow(dead_code)]
+#![warn(clippy::from_instead_of_into)]
+use std::convert::TryFrom;
+use std::convert::TryInto;
+
+fn foo<T>(a: T)
+where
+    T: Into<u32>,
+{
+}
+
+fn foo1<T>(a: T)
+where
+    T: Into<u32>, u32: Copy + Clone,
+{
+}
+
+fn bar<T>(a: T)
+where
+    T: TryInto<u32>,
+{
+}
+
+fn bar1<T>(a: T)
+where
+    T: TryInto<u32>, u32: Copy + Clone,
+{
+}
+
+fn bar2<T>(a: T)
+where
+    T: TryInto<u32>, u32: Copy + Clone,
+{
+}
+
+fn main() {}

tests/ui/from_instead_of_into.rs

@@ -0,0 +1,39 @@
+// run-rustfix
+#![allow(unused_imports)]
+#![allow(unused_variables)]
+#![allow(dead_code)]
+#![warn(clippy::from_instead_of_into)]
+use std::convert::TryFrom;
+use std::convert::TryInto;
+
+fn foo<T>(a: T)
+where
+    u32: From<T>,
+{
+}
+
+fn foo1<T>(a: T)
+where
+    u32: Copy + Clone + From<T>,
+{
+}
+
+fn bar<T>(a: T)
+where
+    u32: TryFrom<T>,
+{
+}
+
+fn bar1<T>(a: T)
+where
+    u32: Copy + TryFrom<T> + Clone,
+{
+}
+
+fn bar2<T>(a: T)
+where
+    u32: TryFrom<T> + Copy + Clone,
+{
+}
+
+fn main() {}

tests/ui/from_instead_of_into.stderr

@@ -0,0 +1,78 @@
+error: Into trait is preferable than From as a generic bound
+  --> $DIR/from_instead_of_into.rs:11:5
+   |
+LL |     u32: From<T>,
+   |     ^^^^^^^^^^^^
+   |
+   = note: `-D clippy::from-instead-of-into` implied by `-D warnings`
+help: remove From bound
+   |
+LL |     ,
+   |    --
+help: Add this bound predicate
+   |
+LL |     u32: From<T>T: Into<u32>,
+   |                 ^^^^^^^^^^^^
+
+error: Into trait is preferable than From as a generic bound
+  --> $DIR/from_instead_of_into.rs:17:5
+   |
+LL |     u32: Copy + Clone + From<T>,
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove From bound
+   |
+LL |     u32: Copy + Clone,
+   |                     --
+help: Add this bound predicate
+   |
+LL |     u32: Copy + Clone + From<T>, T: Into<u32>,
+   |                                ^^^^^^^^^^^^^^
+
+error: TryInto trait is preferable than TryFrom as a generic bound
+  --> $DIR/from_instead_of_into.rs:23:5
+   |
+LL |     u32: TryFrom<T>,
+   |     ^^^^^^^^^^^^^^^
+   |
+help: remove TryFrom bound
+   |
+LL |     ,
+   |    --
+help: Add this bound predicate
+   |
+LL |     u32: TryFrom<T>T: TryInto<u32>,
+   |                    ^^^^^^^^^^^^^^^
+
+error: TryInto trait is preferable than TryFrom as a generic bound
+  --> $DIR/from_instead_of_into.rs:29:5
+   |
+LL |     u32: Copy + TryFrom<T> + Clone,
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove TryFrom bound
+   |
+LL |     u32: Copy + Clone,
+   |             --
+help: Add this bound predicate
+   |
+LL |     u32: Copy + TryFrom<T> + Clone, T: TryInto<u32>,
+   |                                   ^^^^^^^^^^^^^^^^^
+
+error: TryInto trait is preferable than TryFrom as a generic bound
+  --> $DIR/from_instead_of_into.rs:35:5
+   |
+LL |     u32: TryFrom<T> + Copy + Clone,
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove TryFrom bound
+   |
+LL |      + Copy + Clone,
+   |    --
+help: Add this bound predicate
+   |
+LL |     u32: TryFrom<T> + Copy + Clone, T: TryInto<u32>,
+   |                                   ^^^^^^^^^^^^^^^^^
+
+error: aborting due to 5 previous errors
+

tests/ui/needless_question_mark.fixed

@@ -87,6 +87,7 @@ fn also_bad(tr: Result<TR, bool>) -> Result<usize, bool> {
     Err(false)
 }
 
+#[allow(clippy::from_instead_of_into)]
 fn false_positive_test<U, T>(x: Result<(), U>) -> Result<(), T>
 where
     T: From<U>,

tests/ui/needless_question_mark.rs

@@ -87,6 +87,7 @@ fn also_bad(tr: Result<TR, bool>) -> Result<usize, bool> {
     Err(false)
 }
 
+#[allow(clippy::from_instead_of_into)]
 fn false_positive_test<U, T>(x: Result<(), U>) -> Result<(), T>
 where
     T: From<U>,

tests/ui/needless_question_mark.stderr

@@ -67,19 +67,19 @@ LL |         return Ok(t.magic?);
    |                ^^^^^^^^^^^^ help: try: `t.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:138:9
+  --> $DIR/needless_question_mark.rs:139:9
    |
 LL |         Ok(to.magic?) // should be triggered
    |         ^^^^^^^^^^^^^ help: try: `to.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:154:9
+  --> $DIR/needless_question_mark.rs:155:9
    |
 LL |         Some(to.magic?) // should be triggered
    |         ^^^^^^^^^^^^^^^ help: try: `to.magic`
 
 error: Question mark operator is useless here
-  --> $DIR/needless_question_mark.rs:162:9
+  --> $DIR/needless_question_mark.rs:163:9
    |
 LL |         Ok(to.magic?) // should be triggered
    |         ^^^^^^^^^^^^^ help: try: `to.magic`