Added from_over_into lint

diff --git a/CHANGELOG.md b/CHANGELOG.md
index af3b1c1db2aedb79bcb117754f4e1f82fb4d2298..de8da99cdee12385b27570510ace9d77e5c54979 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1841,6 +1841,7 @@ Released 2018-09-13
 [`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_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
 [`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send
 [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
 [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap

diff --git a/clippy_lints/src/from_over_into.rs b/clippy_lints/src/from_over_into.rs
new file mode 100644 (file)
index 0000000..fe7120b
--- /dev/null
+++ b/clippy_lints/src/from_over_into.rs
@@ -0,0 +1,63 @@
+use crate::utils::paths::INTO;
+use crate::utils::{match_def_path, span_lint_and_help};
+use if_chain::if_chain;
+use rustc_hir as hir;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// **What it does:** Searches for implementations of the `Into<..>` trait and suggests to implement `From<..>` instead.
+    ///
+    /// **Why is this bad?** According the std docs implementing `From<..>` is preferred since it gives you `Into<..>` for free where the reverse isn't true.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// struct StringWrapper(String);
+    ///
+    /// impl Into<StringWrapper> for String {
+    ///     fn into(self) -> StringWrapper {
+    ///         StringWrapper(self)
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// struct StringWrapper(String);
+    ///
+    /// impl From<String> for StringWrapper {
+    ///     fn from(s: String) -> StringWrapper {
+    ///         StringWrapper(s)
+    ///     }
+    /// }
+    /// ```
+    pub FROM_OVER_INTO,
+    style,
+    "Warns on implementations of `Into<..>` to use `From<..>`"
+}
+
+declare_lint_pass!(FromOverInto => [FROM_OVER_INTO]);
+
+impl LateLintPass<'_> for FromOverInto {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
+        let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id);
+        if_chain! {
+            if let hir::ItemKind::Impl{ .. } = &item.kind;
+            if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id);
+            if match_def_path(cx, impl_trait_ref.def_id, &INTO);
+
+            then {
+                span_lint_and_help(
+                    cx,
+                    FROM_OVER_INTO,
+                    item.span,
+                    "An implementation of From is preferred since it gives you Into<..> for free where the reverse isn't true.",
+                    None,
+                    "consider implement From instead",
+                );
+            }
+        }
+    }
+}

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 02ba422a2f5b8c0287913a80aad522f5256c8152..58587481922cc8126764ac0601c2038899045d7e 100644 (file)
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -207,6 +207,7 @@ macro_rules! declare_clippy_lint {
 mod floating_point_arithmetic;
 mod format;
 mod formatting;
+mod from_over_into;
 mod functions;
 mod future_not_send;
 mod get_last_with_len;
@@ -614,6 +615,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_over_into::FROM_OVER_INTO,
         &functions::DOUBLE_MUST_USE,
         &functions::MUST_USE_CANDIDATE,
         &functions::MUST_USE_UNIT,
@@ -1203,6 +1205,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box manual_unwrap_or::ManualUnwrapOr);
     store.register_late_pass(|| box manual_ok_or::ManualOkOr);
     store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
+    store.register_late_pass(|| box from_over_into::FromOverInto);
     store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
     let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::<FxHashSet<_>>();
     store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods));
@@ -1417,6 +1420,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_over_into::FROM_OVER_INTO),
         LintId::of(&functions::DOUBLE_MUST_USE),
         LintId::of(&functions::MUST_USE_UNIT),
         LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF),
@@ -1663,6 +1667,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_over_into::FROM_OVER_INTO),
         LintId::of(&functions::DOUBLE_MUST_USE),
         LintId::of(&functions::MUST_USE_UNIT),
         LintId::of(&functions::RESULT_UNIT_ERR),

diff --git a/tests/ui/from_over_into.rs b/tests/ui/from_over_into.rs
new file mode 100644 (file)
index 0000000..292d092
--- /dev/null
+++ b/tests/ui/from_over_into.rs
@@ -0,0 +1,21 @@
+#![warn(clippy::from_over_into)]
+
+// this should throw an error
+struct StringWrapper(String);
+
+impl Into<StringWrapper> for String {
+    fn into(self) -> StringWrapper {
+        StringWrapper(self)
+    }
+}
+
+// this is fine
+struct A(String);
+
+impl From<String> for A {
+    fn from(s: String) -> A {
+        A(s)
+    }
+}
+
+fn main() {}

diff --git a/tests/ui/from_over_into.stderr b/tests/ui/from_over_into.stderr
new file mode 100644 (file)
index 0000000..17f30fa
--- /dev/null
+++ b/tests/ui/from_over_into.stderr
@@ -0,0 +1,15 @@
+error: An implementation of From is preferred since it gives you Into<..> for free where the reverse isn't true.
+  --> $DIR/from_over_into.rs:6:1
+   |
+LL | / impl Into<StringWrapper> for String {
+LL | |     fn into(self) -> StringWrapper {
+LL | |         StringWrapper(self)
+LL | |     }
+LL | | }
+   | |_^
+   |
+   = note: `-D clippy::from-over-into` implied by `-D warnings`
+   = help: consider implement From instead
+
+error: aborting due to previous error
+

diff --git a/tests/ui/unused_unit.fixed b/tests/ui/unused_unit.fixed
index 7afc5361356333235224a79b64710f13cd76e5cd..a192ebde3ebf4e73047dde8152c6dee234cf6bfc 100644 (file)
--- a/tests/ui/unused_unit.fixed
+++ b/tests/ui/unused_unit.fixed
@@ -11,6 +11,7 @@
 
 #![deny(clippy::unused_unit)]
 #![allow(dead_code)]
+#![allow(clippy::from_over_into)]
 
 struct Unitter;
 impl Unitter {

diff --git a/tests/ui/unused_unit.rs b/tests/ui/unused_unit.rs
index 96cef1ed5a5f213dddad7bb987fa034b39ec3f6b..96041a7dd850e3807504697a84b87026447a5955 100644 (file)
--- a/tests/ui/unused_unit.rs
+++ b/tests/ui/unused_unit.rs
@@ -11,6 +11,7 @@
 
 #![deny(clippy::unused_unit)]
 #![allow(dead_code)]
+#![allow(clippy::from_over_into)]
 
 struct Unitter;
 impl Unitter {

diff --git a/tests/ui/unused_unit.stderr b/tests/ui/unused_unit.stderr
index c45634c2b6df71a5131c4c7f8689e5a0edbf7607..02038b5fb6b5a7ed56ce688bdf1e01834d0a68c6 100644 (file)
--- a/tests/ui/unused_unit.stderr
+++ b/tests/ui/unused_unit.stderr
@@ -1,5 +1,5 @@
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:18:28
+  --> $DIR/unused_unit.rs:19:28
    |
 LL |     pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
    |                            ^^^^^^ help: remove the `-> ()`
@@ -11,109 +11,109 @@ LL | #![deny(clippy::unused_unit)]
    |         ^^^^^^^^^^^^^^^^^^^
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:19:18
+  --> $DIR/unused_unit.rs:20:18
    |
 LL |     where G: Fn() -> () {
    |                  ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:18:58
+  --> $DIR/unused_unit.rs:19:58
    |
 LL |     pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
    |                                                          ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:20:26
+  --> $DIR/unused_unit.rs:21:26
    |
 LL |         let _y: &dyn Fn() -> () = &f;
    |                          ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:27:18
+  --> $DIR/unused_unit.rs:28:18
    |
 LL |     fn into(self) -> () {
    |                  ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit expression
-  --> $DIR/unused_unit.rs:28:9
+  --> $DIR/unused_unit.rs:29:9
    |
 LL |         ()
    |         ^^ help: remove the final `()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:33:29
+  --> $DIR/unused_unit.rs:34:29
    |
 LL |     fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
    |                             ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:35:19
+  --> $DIR/unused_unit.rs:36:19
    |
 LL |         G: FnMut() -> (),
    |                   ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:36:16
+  --> $DIR/unused_unit.rs:37:16
    |
 LL |         H: Fn() -> ();
    |                ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:40:29
+  --> $DIR/unused_unit.rs:41:29
    |
 LL |     fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
    |                             ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:42:19
+  --> $DIR/unused_unit.rs:43:19
    |
 LL |         G: FnMut() -> (),
    |                   ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:43:16
+  --> $DIR/unused_unit.rs:44:16
    |
 LL |         H: Fn() -> () {}
    |                ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:46:17
+  --> $DIR/unused_unit.rs:47:17
    |
 LL | fn return_unit() -> () { () }
    |                 ^^^^^^ help: remove the `-> ()`
 
 error: unneeded unit expression
-  --> $DIR/unused_unit.rs:46:26
+  --> $DIR/unused_unit.rs:47:26
    |
 LL | fn return_unit() -> () { () }
    |                          ^^ help: remove the final `()`
 
 error: unneeded `()`
-  --> $DIR/unused_unit.rs:56:14
+  --> $DIR/unused_unit.rs:57:14
    |
 LL |         break();
    |              ^^ help: remove the `()`
 
 error: unneeded `()`
-  --> $DIR/unused_unit.rs:58:11
+  --> $DIR/unused_unit.rs:59:11
    |
 LL |     return();
    |           ^^ help: remove the `()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:75:10
+  --> $DIR/unused_unit.rs:76:10
    |
 LL | fn test()->(){}
    |          ^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:78:11
+  --> $DIR/unused_unit.rs:79:11
    |
 LL | fn test2() ->(){}
    |           ^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:81:11
+  --> $DIR/unused_unit.rs:82:11
    |
 LL | fn test3()-> (){}
    |           ^^^^^ help: remove the `-> ()`