diff --git a/README.md b/README.md
index be7154c8c62..9d86ece99d6 100644
--- a/README.md
+++ b/README.md
@@ -4,7 +4,7 @@
A collection of lints that give helpful tips to newbies and catch oversights.
##Lints
-There are 45 lints included in this crate:
+There are 46 lints included in this crate:
name | default | meaning
-------------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -44,6 +44,7 @@ ptr_arg | allow | fn arguments of the type `&Vec<...>` or `&S
range_step_by_zero | warn | using Range::step_by(0), which produces an infinite iterator
redundant_closure | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
result_unwrap_used | allow | using `Result.unwrap()`, which might be better handled
+should_implement_trait | warn | defining a method that should be implementing a std trait
single_match | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
str_to_string | warn | using `to_string()` on a str, which should be `to_owned()`
string_add | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead
diff --git a/src/lib.rs b/src/lib.rs
index 863ba2624dd..bacff19addc 100755
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -92,6 +92,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
matches::SINGLE_MATCH,
methods::OPTION_UNWRAP_USED,
methods::RESULT_UNWRAP_USED,
+ methods::SHOULD_IMPLEMENT_TRAIT,
methods::STR_TO_STRING,
methods::STRING_TO_STRING,
misc::CMP_NAN,
diff --git a/src/methods.rs b/src/methods.rs
index df8e35d98fb..73cf81fbdae 100644
--- a/src/methods.rs
+++ b/src/methods.rs
@@ -2,9 +2,12 @@ use syntax::ast::*;
use rustc::lint::*;
use rustc::middle::ty;
-use utils::{span_lint, match_type, walk_ptrs_ty};
+use utils::{span_lint, match_path, match_type, walk_ptrs_ty};
use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
+use self::SelfKind::*;
+use self::OutType::*;
+
#[derive(Copy,Clone)]
pub struct MethodsPass;
@@ -16,10 +19,13 @@ declare_lint!(pub STR_TO_STRING, Warn,
"using `to_string()` on a str, which should be `to_owned()`");
declare_lint!(pub STRING_TO_STRING, Warn,
"calling `String.to_string()` which is a no-op");
+declare_lint!(pub SHOULD_IMPLEMENT_TRAIT, Warn,
+ "defining a method that should be implementing a std trait");
impl LintPass for MethodsPass {
fn get_lints(&self) -> LintArray {
- lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING)
+ lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING,
+ SHOULD_IMPLEMENT_TRAIT)
}
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
@@ -46,4 +52,112 @@ impl LintPass for MethodsPass {
}
}
}
+
+ fn check_item(&mut self, cx: &Context, item: &Item) {
+ if let ItemImpl(_, _, _, None, _, ref items) = item.node {
+ for item in items {
+ let name = item.ident.name;
+ for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
+ if_let_chain! {
+ [
+ name == method_name,
+ let MethodImplItem(ref sig, _) = item.node,
+ sig.decl.inputs.len() == n_args,
+ out_type.matches(&sig.decl.output),
+ self_kind.matches(&sig.explicit_self.node)
+ ], {
+ span_lint(cx, SHOULD_IMPLEMENT_TRAIT, item.span, &format!(
+ "defining a method called `{}` on this type; consider implementing \
+ the `{}` trait or choosing a less ambiguous name", name, trait_name));
+ }
+ }
+ }
+ }
+ }
+ }
+}
+
+const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [
+ ("add", 2, ValueSelf, AnyType, "std::ops::Add`"),
+ ("sub", 2, ValueSelf, AnyType, "std::ops::Sub"),
+ ("mul", 2, ValueSelf, AnyType, "std::ops::Mul"),
+ ("div", 2, ValueSelf, AnyType, "std::ops::Div"),
+ ("rem", 2, ValueSelf, AnyType, "std::ops::Rem"),
+ ("shl", 2, ValueSelf, AnyType, "std::ops::Shl"),
+ ("shr", 2, ValueSelf, AnyType, "std::ops::Shr"),
+ ("bitand", 2, ValueSelf, AnyType, "std::ops::BitAnd"),
+ ("bitor", 2, ValueSelf, AnyType, "std::ops::BitOr"),
+ ("bitxor", 2, ValueSelf, AnyType, "std::ops::BitXor"),
+ ("neg", 1, ValueSelf, AnyType, "std::ops::Neg"),
+ ("not", 1, ValueSelf, AnyType, "std::ops::Not"),
+ ("drop", 1, RefMutSelf, UnitType, "std::ops::Drop"),
+ ("index", 2, RefSelf, RefType, "std::ops::Index"),
+ ("index_mut", 2, RefMutSelf, RefType, "std::ops::IndexMut"),
+ ("deref", 1, RefSelf, RefType, "std::ops::Deref"),
+ ("deref_mut", 1, RefMutSelf, RefType, "std::ops::DerefMut"),
+ ("clone", 1, RefSelf, AnyType, "std::clone::Clone"),
+ ("borrow", 1, RefSelf, RefType, "std::borrow::Borrow"),
+ ("borrow_mut", 1, RefMutSelf, RefType, "std::borrow::BorrowMut"),
+ ("as_ref", 1, RefSelf, RefType, "std::convert::AsRef"),
+ ("as_mut", 1, RefMutSelf, RefType, "std::convert::AsMut"),
+ ("eq", 2, RefSelf, BoolType, "std::cmp::PartialEq"),
+ ("cmp", 2, RefSelf, AnyType, "std::cmp::Ord"),
+ ("default", 0, NoSelf, AnyType, "std::default::Default"),
+ ("hash", 2, RefSelf, UnitType, "std::hash::Hash"),
+ ("next", 1, RefMutSelf, AnyType, "std::iter::Iterator"),
+ ("into_iter", 1, ValueSelf, AnyType, "std::iter::IntoIterator"),
+ ("from_iter", 1, NoSelf, AnyType, "std::iter::FromIterator"),
+ ("from_str", 1, NoSelf, AnyType, "std::str::FromStr"),
+];
+
+#[derive(Clone, Copy)]
+enum SelfKind {
+ ValueSelf,
+ RefSelf,
+ RefMutSelf,
+ NoSelf
+}
+
+impl SelfKind {
+ fn matches(&self, slf: &ExplicitSelf_) -> bool {
+ match (self, slf) {
+ (&ValueSelf, &SelfValue(_)) => true,
+ (&RefSelf, &SelfRegion(_, Mutability::MutImmutable, _)) => true,
+ (&RefMutSelf, &SelfRegion(_, Mutability::MutMutable, _)) => true,
+ (&NoSelf, &SelfStatic) => true,
+ _ => false
+ }
+ }
+}
+
+#[derive(Clone, Copy)]
+enum OutType {
+ UnitType,
+ BoolType,
+ AnyType,
+ RefType,
+}
+
+impl OutType {
+ fn matches(&self, ty: &FunctionRetTy) -> bool {
+ match (self, ty) {
+ (&UnitType, &DefaultReturn(_)) => true,
+ (&UnitType, &Return(ref ty)) if ty.node == TyTup(vec![]) => true,
+ (&BoolType, &Return(ref ty)) if is_bool(ty) => true,
+ (&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![]) => true,
+ (&RefType, &Return(ref ty)) => {
+ if let TyRptr(_, _) = ty.node { true } else { false }
+ }
+ _ => false
+ }
+ }
+}
+
+fn is_bool(ty: &Ty) -> bool {
+ if let TyPath(None, ref p) = ty.node {
+ if match_path(p, &["bool"]) {
+ return true;
+ }
+ }
+ false
}
diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs
index 91d3b72de84..cb77d79f0ff 100755
--- a/tests/compile-fail/methods.rs
+++ b/tests/compile-fail/methods.rs
@@ -1,8 +1,27 @@
#![feature(plugin)]
#![plugin(clippy)]
-#[deny(option_unwrap_used, result_unwrap_used)]
-#[deny(str_to_string, string_to_string)]
+#![allow(unused)]
+#![deny(clippy)]
+
+use std::ops::Mul;
+
+struct T;
+
+impl T {
+ fn add(self, other: T) -> T { self } //~ERROR defining a method called `add`
+ fn drop(&mut self) { } //~ERROR defining a method called `drop`
+
+ fn sub(&self, other: T) -> &T { self } // no error, self is a ref
+ fn div(self) -> T { self } // no error, different #arguments
+ fn rem(self, other: T) { } // no error, wrong return type
+}
+
+impl Mul<T> for T {
+ type Output = T;
+ fn mul(self, other: T) -> T { self } // no error, obviously
+}
+
fn main() {
let opt = Some(0);
let _ = opt.unwrap(); //~ERROR used unwrap() on an Option