diff --git a/README.md b/README.md index c2d3b16074c..5a405c6d4ca 100644 --- 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 675dbdd2dd7..8d29d6ab68a 100644 --- 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 3787474e28c..ed3f61e4a72 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -278,6 +278,23 @@ declare_lint! { 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 @@ impl LintPass for MethodsPass { OR_FUN_CALL, CHARS_NEXT_CMP, CLONE_ON_COPY, - CLONE_DOUBLE_REF) + CLONE_DOUBLE_REF, + NEW_RET_NO_SELF) } } @@ -390,6 +408,29 @@ impl LateLintPass for MethodsPass { .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 9a10b336e0e..afe056e3d05 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -23,16 +23,27 @@ impl T { 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 { + _dummy: T +} + +impl V { + fn new() -> Option> { None } +} + impl Mul for T { type Output = T; fn mul(self, other: T) -> T { self } // no error, obviously