about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-04-24 12:40:15 +0000
committerbors <bors@rust-lang.org>2022-04-24 12:40:15 +0000
commit6aa3684431e85609eab1168098915afcb4e20d65 (patch)
treebb33a2047ac17874b8a71b84fb3a2d9ef1b795c1
parent2a5ee682cc60e2cbb0edaa1e97e171f934ed309b (diff)
parent51db157fb4c44212fec54d61af19f80bd4335286 (diff)
downloadrust-6aa3684431e85609eab1168098915afcb4e20d65.tar.gz
rust-6aa3684431e85609eab1168098915afcb4e20d65.zip
Auto merge of #8738 - tamaroning:fix_wrong_self_convention, r=xFrednet
wrong_self_convention allows `is_*` to take `&mut self`

fix #8480 and #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
-rw-r--r--clippy_lints/src/methods/mod.rs18
-rw-r--r--clippy_lints/src/methods/wrong_self_convention.rs2
-rw-r--r--tests/ui/wrong_self_convention.rs5
-rw-r--r--tests/ui/wrong_self_convention.stderr18
-rw-r--r--tests/ui/wrong_self_convention2.rs10
5 files changed, 25 insertions, 28 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 7a1bf36489a..f3be71f6b8b 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/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.
diff --git a/clippy_lints/src/methods/wrong_self_convention.rs b/clippy_lints/src/methods/wrong_self_convention.rs
index aecfea9c141..4b368d3ffae 100644
--- a/clippy_lints/src/methods/wrong_self_convention.rs
+++ b/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]),
 
diff --git a/tests/ui/wrong_self_convention.rs b/tests/ui/wrong_self_convention.rs
index f8fee4b3ab2..e3cc90ee222 100644
--- a/tests/ui/wrong_self_convention.rs
+++ b/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
diff --git a/tests/ui/wrong_self_convention.stderr b/tests/ui/wrong_self_convention.stderr
index 5493a99572e..2e7ee51d7e1 100644
--- a/tests/ui/wrong_self_convention.stderr
+++ b/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
 
diff --git a/tests/ui/wrong_self_convention2.rs b/tests/ui/wrong_self_convention2.rs
index a8fe8331133..0dcf4743e8b 100644
--- a/tests/ui/wrong_self_convention2.rs
+++ b/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!();
+        }
+    }
+}