Lint about `new` methods not returning `Self`

diff --git a/README.md b/README.md
index c2d3b16074cc4f8043e349778d338451ab110bff..5a405c6d4caf7f905f4a30ee5fd67b33ff3fee4a 100644 (file)
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 120 lints included in this crate:
+There are 121 lints included in this crate:
 
 name                                                                                                           | default | meaning
 ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -76,6 +76,7 @@ name
 [needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop)                     | warn    | for-looping over a range of indices where an iterator over items would do
 [needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return)                             | warn    | using a return statement like `return expr;` where an expression would suffice
 [needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update)                             | warn    | using `{ ..base }` when there are no missing fields
+[new_ret_no_self](https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self)                             | warn    | not returning `Self` in a `new` method
 [no_effect](https://github.com/Manishearth/rust-clippy/wiki#no_effect)                                         | warn    | statements with no effect
 [non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal)                         | allow   | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
 [nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options)           | warn    | nonsensical combination of options for opening a file

diff --git a/src/lib.rs b/src/lib.rs
index 675dbdd2dd712500ea8acfc76439c26eeb8c4f75..8d29d6ab68aa6822136be7639afe73c692f70542 100644 (file)
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -234,6 +234,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         methods::CLONE_ON_COPY,
         methods::EXTEND_FROM_SLICE,
         methods::FILTER_NEXT,
+        methods::NEW_RET_NO_SELF,
         methods::OK_EXPECT,
         methods::OPTION_MAP_UNWRAP_OR,
         methods::OPTION_MAP_UNWRAP_OR_ELSE,

diff --git a/src/methods.rs b/src/methods.rs
index 3787474e28cc0d8a0bd816dc7623572b36c6454f..ed3f61e4a727297720932b7e1b4e84ea70a68e52 100644 (file)
--- a/src/methods.rs
+++ b/src/methods.rs
@@ -278,6 +278,23 @@
     pub CLONE_DOUBLE_REF, Warn, "using `clone` on `&&T`"
 }
 
+/// **What it does:** This lint warns about `new` not returning `Self`.
+///
+/// **Why is this bad?** As a convention, `new` methods are used to make a new instance of a type.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// impl Foo {
+///     fn new(..) -> NotAFoo {
+///     }
+/// }
+/// ```
+declare_lint! {
+    pub NEW_RET_NO_SELF, Warn, "not returning `Self` in a `new` method"
+}
+
 impl LintPass for MethodsPass {
     fn get_lints(&self) -> LintArray {
         lint_array!(EXTEND_FROM_SLICE,
@@ -294,7 +311,8 @@ fn get_lints(&self) -> LintArray {
                     OR_FUN_CALL,
                     CHARS_NEXT_CMP,
                     CLONE_ON_COPY,
-                    CLONE_DOUBLE_REF)
+                    CLONE_DOUBLE_REF,
+                    NEW_RET_NO_SELF)
     }
 }
 
@@ -390,6 +408,29 @@ fn check_item(&mut self, cx: &LateContext, item: &Item) {
                                                           .join(" or ")));
                         }
                     }
+
+                    if &name.as_str() == &"new" {
+                        let returns_self = if let FunctionRetTy::Return(ref ret_ty) = sig.decl.output {
+                            let ast_ty_to_ty_cache = cx.tcx.ast_ty_to_ty_cache.borrow();
+                            let ty = ast_ty_to_ty_cache.get(&ty.id);
+                            let ret_ty = ast_ty_to_ty_cache.get(&ret_ty.id);
+
+                            match (ty, ret_ty) {
+                                (Some(&ty), Some(&ret_ty)) => ret_ty.walk().any(|t| t == ty),
+                                _ => false,
+                            }
+                        }
+                        else {
+                            false
+                        };
+
+                        if !returns_self {
+                            span_lint(cx,
+                                      NEW_RET_NO_SELF,
+                                      sig.explicit_self.span,
+                                      "methods called `new` usually return `Self`");
+                        }
+                    }
                 }
             }
         }

diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs
index 9a10b336e0ecef0c84eca53e257476e84855d3a3..afe056e3d05fabe01472fae36e2b3acfb812462b 100644 (file)
--- a/tests/compile-fail/methods.rs
+++ b/tests/compile-fail/methods.rs
@@ -23,16 +23,27 @@ fn into_u16(&self) -> u16 { 0 } //~ERROR methods called `into_*` usually take se
 
     fn to_something(self) -> u32 { 0 } //~ERROR methods called `to_*` usually take self by reference
 
-    fn new(self) {} //~ERROR methods called `new` usually take no self
+    fn new(self) {}
+    //~^ ERROR methods called `new` usually take no self
+    //~| ERROR methods called `new` usually return `Self`
 }
 
 #[derive(Clone,Copy)]
 struct U;
 
 impl U {
+    fn new() -> Self { U }
     fn to_something(self) -> u32 { 0 } // ok because U is Copy
 }
 
+struct V<T> {
+    _dummy: T
+}
+
+impl<T> V<T> {
+    fn new() -> Option<V<T>> { None }
+}
+
 impl Mul<T> for T {
     type Output = T;
     fn mul(self, other: T) -> T { self } // no error, obviously