Auto merge of rust-lang#8738 - tamaroning:fix_wrong_self_convention, r=xFrednet

wrong_self_convention allows `is_*` to take `&mut self`

fix rust-lang#8480 and rust-lang#8513
Allowing `is_*` to take `&self` or none is too restrictive.

changelog: FPs: [`wrong_self_convention`] now allows `&mut self` and no self as arguments for `is_*` methods

Author: bors

clippy_lints/src/methods/mod.rs

@@ -296,15 +296,15 @@ declare_clippy_lint! {
     /// Checks for methods with certain name prefixes and which
     /// doesn't match how self is taken. The actual rules are:
     ///
-    /// |Prefix |Postfix     |`self` taken           | `self` type  |
-    /// |-------|------------|-----------------------|--------------|
-    /// |`as_`  | none       |`&self` or `&mut self` | any          |
-    /// |`from_`| none       | none                  | any          |
-    /// |`into_`| none       |`self`                 | any          |
-    /// |`is_`  | none       |`&self` or none        | any          |
-    /// |`to_`  | `_mut`     |`&mut self`            | any          |
-    /// |`to_`  | not `_mut` |`self`                 | `Copy`       |
-    /// |`to_`  | not `_mut` |`&self`                | not `Copy`   |
+    /// |Prefix |Postfix     |`self` taken                   | `self` type  |
+    /// |-------|------------|-------------------------------|--------------|
+    /// |`as_`  | none       |`&self` or `&mut self`         | any          |
+    /// |`from_`| none       | none                          | any          |
+    /// |`into_`| none       |`self`                         | any          |
+    /// |`is_`  | none       |`&mut self` or `&self` or none | any          |
+    /// |`to_`  | `_mut`     |`&mut self`                    | any          |
+    /// |`to_`  | not `_mut` |`self`                         | `Copy`       |
+    /// |`to_`  | not `_mut` |`&self`                        | not `Copy`   |
     ///
     /// Note: Clippy doesn't trigger methods with `to_` prefix in:
     /// - Traits definition.

clippy_lints/src/methods/wrong_self_convention.rs

@@ -14,7 +14,7 @@ const CONVENTIONS: [(&[Convention], &[SelfKind]); 9] = [
     (&[Convention::StartsWith("as_")], &[SelfKind::Ref, SelfKind::RefMut]),
     (&[Convention::StartsWith("from_")], &[SelfKind::No]),
     (&[Convention::StartsWith("into_")], &[SelfKind::Value]),
-    (&[Convention::StartsWith("is_")], &[SelfKind::Ref, SelfKind::No]),
+    (&[Convention::StartsWith("is_")], &[SelfKind::RefMut, SelfKind::Ref, SelfKind::No]),
     (&[Convention::Eq("to_mut")], &[SelfKind::RefMut]),
     (&[Convention::StartsWith("to_"), Convention::EndsWith("_mut")], &[SelfKind::RefMut]),
 

tests/ui/wrong_self_convention.rs

@@ -193,11 +193,6 @@ pub mod issue8142 {
     struct S;
 
     impl S {
-        // Should lint: is_ methods should only take &self, or no self at all.
-        fn is_still_buggy(&mut self) -> bool {
-            false
-        }
-
         // Should not lint: "no self at all" is allowed.
         fn is_forty_two(x: u32) -> bool {
             x == 42

tests/ui/wrong_self_convention.stderr

@@ -31,7 +31,7 @@ LL |     fn into_i32(&self) {}
    |
    = help: consider choosing a less ambiguous name
 
-error: methods called `is_*` usually take `self` by reference or no `self`
+error: methods called `is_*` usually take `self` by mutable reference or `self` by reference or no `self`
   --> $DIR/wrong_self_convention.rs:38:15
    |
 LL |     fn is_i32(self) {}
@@ -71,7 +71,7 @@ LL |     pub fn into_i64(&self) {}
    |
    = help: consider choosing a less ambiguous name
 
-error: methods called `is_*` usually take `self` by reference or no `self`
+error: methods called `is_*` usually take `self` by mutable reference or `self` by reference or no `self`
   --> $DIR/wrong_self_convention.rs:46:19
    |
 LL |     pub fn is_i64(self) {}
@@ -111,7 +111,7 @@ LL |         fn into_i32_ref(&self) {}
    |
    = help: consider choosing a less ambiguous name
 
-error: methods called `is_*` usually take `self` by reference or no `self`
+error: methods called `is_*` usually take `self` by mutable reference or `self` by reference or no `self`
   --> $DIR/wrong_self_convention.rs:98:19
    |
 LL |         fn is_i32(self) {}
@@ -143,7 +143,7 @@ LL |         fn into_i32_ref(&self);
    |
    = help: consider choosing a less ambiguous name
 
-error: methods called `is_*` usually take `self` by reference or no `self`
+error: methods called `is_*` usually take `self` by mutable reference or `self` by reference or no `self`
   --> $DIR/wrong_self_convention.rs:122:19
    |
 LL |         fn is_i32(self);
@@ -191,13 +191,5 @@ LL |         fn to_u64(self) -> u64 {
    |
    = help: consider choosing a less ambiguous name
 
-error: methods called `is_*` usually take `self` by reference or no `self`
-  --> $DIR/wrong_self_convention.rs:197:27
-   |
-LL |         fn is_still_buggy(&mut self) -> bool {
-   |                           ^^^^^^^^^
-   |
-   = help: consider choosing a less ambiguous name
-
-error: aborting due to 25 previous errors
+error: aborting due to 24 previous errors
 

tests/ui/wrong_self_convention2.rs

@@ -104,3 +104,13 @@ mod issue4546 {
         pub fn to_other_thingy(self: Pin<&Self>) {}
     }
 }
+
+mod issue_8480_8513 {
+    struct Cat(String);
+
+    impl Cat {
+        fn is_animal(&mut self) -> bool {
+            todo!();
+        }
+    }
+}