Auto merge of rust-lang#6339 - CDirkx:redundant-pattern-match-poll, r…

…=ebroto

Change `redundant_pattern_matching` to also lint `std::task::Poll`

`reduntant_pattern_matching` currently lints pattern matching on `Option` and `Result` where the `is_variant` utility methods could be used instead: `is_some`, `is_none`, `is_ok`, `is_err`. This PR extends this behaviour to `std::task::Poll`, suggesting the methods `is_pending` and `is_ready`.

Motivation: The current description of `redundant_pattern_matching` mentions

> It's more concise and clear to just use the proper utility function

which in my mind applies to `Poll` as well.

changelog: Enhance [`redundant_pattern_matching`] to also lint on `std::task::Poll`

clippy_lints/src/matches.rs

@@ -411,8 +411,8 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Lint for redundant pattern matching over `Result` or
-    /// `Option`
+    /// **What it does:** Lint for redundant pattern matching over `Result`, `Option` or
+    /// `std::task::Poll`
     ///
     /// **Why is this bad?** It's more concise and clear to just use the proper
     /// utility function
@@ -422,10 +422,13 @@ declare_clippy_lint! {
     /// **Example:**
     ///
     /// ```rust
+    /// # use std::task::Poll;
     /// if let Ok(_) = Ok::<i32, i32>(42) {}
     /// if let Err(_) = Err::<i32, i32>(42) {}
     /// if let None = None::<()> {}
     /// if let Some(_) = Some(42) {}
+    /// if let Poll::Pending = Poll::Pending::<()> {}
+    /// if let Poll::Ready(_) = Poll::Ready(42) {}
     /// match Ok::<i32, i32>(42) {
     ///     Ok(_) => true,
     ///     Err(_) => false,
@@ -435,10 +438,13 @@ declare_clippy_lint! {
     /// The more idiomatic use would be:
     ///
     /// ```rust
+    /// # use std::task::Poll;
     /// if Ok::<i32, i32>(42).is_ok() {}
     /// if Err::<i32, i32>(42).is_err() {}
     /// if None::<()>.is_none() {}
     /// if Some(42).is_some() {}
+    /// if Poll::Pending::<()>.is_pending() {}
+    /// if Poll::Ready(42).is_ready() {}
     /// Ok::<i32, i32>(42).is_ok();
     /// ```
     pub REDUNDANT_PATTERN_MATCHING,
@@ -1538,14 +1544,24 @@ mod redundant_pattern_match {
                         "is_err()"
                     } else if match_qpath(path, &paths::OPTION_SOME) {
                         "is_some()"
+                    } else if match_qpath(path, &paths::POLL_READY) {
+                        "is_ready()"
                     } else {
                         return;
                     }
                 } else {
                     return;
                 }
             },
-            PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
+            PatKind::Path(ref path) => {
+                if match_qpath(path, &paths::OPTION_NONE) {
+                    "is_none()"
+                } else if match_qpath(path, &paths::POLL_PENDING) {
+                    "is_pending()"
+                } else {
+                    return;
+                }
+            },
             _ => return,
         };
 
@@ -1628,6 +1644,17 @@ mod redundant_pattern_match {
                             "is_some()",
                             "is_none()",
                         )
+                        .or_else(|| {
+                            find_good_method_for_match(
+                                arms,
+                                path_left,
+                                path_right,
+                                &paths::POLL_READY,
+                                &paths::POLL_PENDING,
+                                "is_ready()",
+                                "is_pending()",
+                            )
+                        })
                     } else {
                         None
                     }

clippy_lints/src/utils/paths.rs

@@ -90,6 +90,8 @@ pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"];
 pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
 pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
 pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"];
+pub const POLL_PENDING: [&str; 5] = ["core", "task", "poll", "Poll", "Pending"];
+pub const POLL_READY: [&str; 5] = ["core", "task", "poll", "Poll", "Ready"];
 pub const PTR_EQ: [&str; 3] = ["core", "ptr", "eq"];
 pub const PTR_NULL: [&str; 3] = ["core", "ptr", "null"];
 pub const PTR_NULL_MUT: [&str; 3] = ["core", "ptr", "null_mut"];

tests/ui/redundant_pattern_matching_option.fixed

@@ -2,13 +2,7 @@
 
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
-#![allow(
-    clippy::unit_arg,
-    unused_must_use,
-    clippy::needless_bool,
-    clippy::match_like_matches_macro,
-    deprecated
-)]
+#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
 
 fn main() {
     if None::<()>.is_none() {}

tests/ui/redundant_pattern_matching_option.rs

@@ -2,13 +2,7 @@
 
 #![warn(clippy::all)]
 #![warn(clippy::redundant_pattern_matching)]
-#![allow(
-    clippy::unit_arg,
-    unused_must_use,
-    clippy::needless_bool,
-    clippy::match_like_matches_macro,
-    deprecated
-)]
+#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
 
 fn main() {
     if let None = None::<()> {}

tests/ui/redundant_pattern_matching_option.stderr

@@ -1,49 +1,49 @@
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:14:12
+  --> $DIR/redundant_pattern_matching_option.rs:8:12
    |
 LL |     if let None = None::<()> {}
    |     -------^^^^------------- help: try this: `if None::<()>.is_none()`
    |
    = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:16:12
+  --> $DIR/redundant_pattern_matching_option.rs:10:12
    |
 LL |     if let Some(_) = Some(42) {}
    |     -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:18:12
+  --> $DIR/redundant_pattern_matching_option.rs:12:12
    |
 LL |     if let Some(_) = Some(42) {
    |     -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:24:15
+  --> $DIR/redundant_pattern_matching_option.rs:18:15
    |
 LL |     while let Some(_) = Some(42) {}
    |     ----------^^^^^^^----------- help: try this: `while Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:26:15
+  --> $DIR/redundant_pattern_matching_option.rs:20:15
    |
 LL |     while let None = Some(42) {}
    |     ----------^^^^----------- help: try this: `while Some(42).is_none()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:28:15
+  --> $DIR/redundant_pattern_matching_option.rs:22:15
    |
 LL |     while let None = None::<()> {}
    |     ----------^^^^------------- help: try this: `while None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:31:15
+  --> $DIR/redundant_pattern_matching_option.rs:25:15
    |
 LL |     while let Some(_) = v.pop() {
    |     ----------^^^^^^^---------- help: try this: `while v.pop().is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:39:5
+  --> $DIR/redundant_pattern_matching_option.rs:33:5
    |
 LL | /     match Some(42) {
 LL | |         Some(_) => true,
@@ -52,7 +52,7 @@ LL | |     };
    | |_____^ help: try this: `Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:44:5
+  --> $DIR/redundant_pattern_matching_option.rs:38:5
    |
 LL | /     match None::<()> {
 LL | |         Some(_) => false,
@@ -61,7 +61,7 @@ LL | |     };
    | |_____^ help: try this: `None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:49:13
+  --> $DIR/redundant_pattern_matching_option.rs:43:13
    |
 LL |       let _ = match None::<()> {
    |  _____________^
@@ -71,49 +71,49 @@ LL | |     };
    | |_____^ help: try this: `None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:55:20
+  --> $DIR/redundant_pattern_matching_option.rs:49:20
    |
 LL |     let x = if let Some(_) = opt { true } else { false };
    |             -------^^^^^^^------ help: try this: `if opt.is_some()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:60:20
+  --> $DIR/redundant_pattern_matching_option.rs:54:20
    |
 LL |     let _ = if let Some(_) = gen_opt() {
    |             -------^^^^^^^------------ help: try this: `if gen_opt().is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:62:19
+  --> $DIR/redundant_pattern_matching_option.rs:56:19
    |
 LL |     } else if let None = gen_opt() {
    |            -------^^^^------------ help: try this: `if gen_opt().is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:83:12
+  --> $DIR/redundant_pattern_matching_option.rs:77:12
    |
 LL |     if let Some(_) = Some(42) {}
    |     -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:85:12
+  --> $DIR/redundant_pattern_matching_option.rs:79:12
    |
 LL |     if let None = None::<()> {}
    |     -------^^^^------------- help: try this: `if None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:87:15
+  --> $DIR/redundant_pattern_matching_option.rs:81:15
    |
 LL |     while let Some(_) = Some(42) {}
    |     ----------^^^^^^^----------- help: try this: `while Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:89:15
+  --> $DIR/redundant_pattern_matching_option.rs:83:15
    |
 LL |     while let None = None::<()> {}
    |     ----------^^^^------------- help: try this: `while None::<()>.is_none()`
 
 error: redundant pattern matching, consider using `is_some()`
-  --> $DIR/redundant_pattern_matching_option.rs:91:5
+  --> $DIR/redundant_pattern_matching_option.rs:85:5
    |
 LL | /     match Some(42) {
 LL | |         Some(_) => true,
@@ -122,7 +122,7 @@ LL | |     };
    | |_____^ help: try this: `Some(42).is_some()`
 
 error: redundant pattern matching, consider using `is_none()`
-  --> $DIR/redundant_pattern_matching_option.rs:96:5
+  --> $DIR/redundant_pattern_matching_option.rs:90:5
    |
 LL | /     match None::<()> {
 LL | |         Some(_) => false,

tests/ui/redundant_pattern_matching_poll.fixed

@@ -0,0 +1,73 @@
+// run-rustfix
+
+#![warn(clippy::all)]
+#![warn(clippy::redundant_pattern_matching)]
+#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
+
+use std::task::Poll::{self, Pending, Ready};
+
+fn main() {
+    if Pending::<()>.is_pending() {}
+
+    if Ready(42).is_ready() {}
+
+    if Ready(42).is_ready() {
+        foo();
+    } else {
+        bar();
+    }
+
+    while Ready(42).is_ready() {}
+
+    while Ready(42).is_pending() {}
+
+    while Pending::<()>.is_pending() {}
+
+    if Pending::<i32>.is_pending() {}
+
+    if Ready(42).is_ready() {}
+
+    Ready(42).is_ready();
+
+    Pending::<()>.is_pending();
+
+    let _ = Pending::<()>.is_pending();
+
+    let poll = Ready(false);
+    let x = if poll.is_ready() { true } else { false };
+    takes_poll(x);
+
+    poll_const();
+
+    let _ = if gen_poll().is_ready() {
+        1
+    } else if gen_poll().is_pending() {
+        2
+    } else {
+        3
+    };
+}
+
+fn gen_poll() -> Poll<()> {
+    Pending
+}
+
+fn takes_poll(_: bool) {}
+
+fn foo() {}
+
+fn bar() {}
+
+const fn poll_const() {
+    if Ready(42).is_ready() {}
+
+    if Pending::<()>.is_pending() {}
+
+    while Ready(42).is_ready() {}
+
+    while Pending::<()>.is_pending() {}
+
+    Ready(42).is_ready();
+
+    Pending::<()>.is_pending();
+}