methods: try to allow value self when type is Copy (fixes #273)

diff --git a/src/methods.rs b/src/methods.rs
index 5a20ba0683135820af528cff64f4a5ceb00e68e5..d55cf49daabab04073c5614de4ccc7fd48b25412 100644 (file)
--- a/src/methods.rs
+++ b/src/methods.rs
@@ -68,10 +68,10 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
     }
 
     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;
-                if let MethodImplItem(ref sig, _) = item.node {
+        if let ItemImpl(_, _, _, None, ref ty, ref items) = item.node {
+            for implitem in items {
+                let name = implitem.ident.name;
+                if let MethodImplItem(ref sig, _) = implitem.node {
                     // check missing trait implementations
                     for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
                         if_let_chain! {
@@ -79,9 +79,9 @@ fn check_item(&mut self, cx: &Context, item: &Item) {
                                 name == method_name,
                                 sig.decl.inputs.len() == n_args,
                                 out_type.matches(&sig.decl.output),
-                                self_kind.matches(&sig.explicit_self.node)
+                                self_kind.matches(&sig.explicit_self.node, false)
                             ], {
-                                span_lint(cx, SHOULD_IMPLEMENT_TRAIT, item.span, &format!(
+                                span_lint(cx, SHOULD_IMPLEMENT_TRAIT, implitem.span, &format!(
                                     "defining a method called `{}` on this type; consider implementing \
                                      the `{}` trait or choosing a less ambiguous name", name, trait_name));
                             }
@@ -89,7 +89,8 @@ fn check_item(&mut self, cx: &Context, item: &Item) {
                     }
                     // check conventions w.r.t. conversion method names and predicates
                     for &(prefix, self_kind) in &CONVENTIONS {
-                        if name.as_str().starts_with(prefix) && !self_kind.matches(&sig.explicit_self.node) {
+                        if name.as_str().starts_with(prefix) &&
+                                !self_kind.matches(&sig.explicit_self.node, is_copy(cx, &ty, &item)) {
                             span_lint(cx, WRONG_SELF_CONVENTION, sig.explicit_self.span, &format!(
                                 "methods called `{}*` usually take {}; consider choosing a less \
                                  ambiguous name", prefix, self_kind.description()));
@@ -151,22 +152,26 @@ enum SelfKind {
 }
 
 impl SelfKind {
-    fn matches(&self, slf: &ExplicitSelf_) -> bool {
+    fn matches(&self, slf: &ExplicitSelf_, allow_value_for_ref: bool) -> bool {
         match (self, slf) {
             (&ValueSelf, &SelfValue(_)) => true,
             (&RefSelf, &SelfRegion(_, Mutability::MutImmutable, _)) => true,
             (&RefMutSelf, &SelfRegion(_, Mutability::MutMutable, _)) => true,
+            (&RefSelf, &SelfValue(_)) => allow_value_for_ref,
+            (&RefMutSelf, &SelfValue(_)) => allow_value_for_ref,
             (&NoSelf, &SelfStatic) => true,
-            (_, &SelfExplicit(ref ty, _)) => self.matches_explicit_type(ty),
+            (_, &SelfExplicit(ref ty, _)) => self.matches_explicit_type(ty, allow_value_for_ref),
             _ => false
         }
     }
 
-    fn matches_explicit_type(&self, ty: &Ty) -> bool {
+    fn matches_explicit_type(&self, ty: &Ty, allow_value_for_ref: bool) -> bool {
         match (self, &ty.node) {
             (&ValueSelf, &TyPath(..)) => true,
             (&RefSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutImmutable, .. })) => true,
             (&RefMutSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutMutable, .. })) => true,
+            (&RefSelf, &TyPath(..)) => allow_value_for_ref,
+            (&RefMutSelf, &TyPath(..)) => allow_value_for_ref,
             _ => false
         }
     }
@@ -212,3 +217,13 @@ fn is_bool(ty: &Ty) -> bool {
     }
     false
 }
+
+fn is_copy(cx: &Context, ast_ty: &Ty, item: &Item) -> bool {
+    match cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) {
+        None => false,
+        Some(ty) => {
+            let env = ty::ParameterEnvironment::for_item(cx.tcx, item.id);
+            !ty.moves_by_default(&env, ast_ty.span)
+        }
+    }
+}

diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs
index 560f36a9d5dfcdffee28dc6c3b5758e31cc00690..314601f6dbd3662435898de04a0223363403d8ba 100755 (executable)
--- a/tests/compile-fail/methods.rs
+++ b/tests/compile-fail/methods.rs
@@ -18,6 +18,15 @@ fn rem(self, other: T) { } // no error, wrong return type
 
     fn into_u32(self) -> u32 { 0 } // fine
     fn into_u16(&self) -> u16 { 0 } //~ERROR methods called `into_*` usually take self by value
+
+    fn to_something(self) -> u32 { 0 } //~ERROR methods called `to_*` usually take self by reference
+}
+
+#[derive(Clone,Copy)]
+struct U;
+
+impl U {
+    fn to_something(self) -> u32 { 0 } // ok because U is Copy
 }
 
 impl Mul<T> for T {