Skip to content

Commit f897d27

Browse files
borsbors
committed
Auto merge of #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`
2 parents 5b40ce3 + 5a83968 commit f897d27

11 files changed

+364
-60
lines changed

‎clippy_lints/src/matches.rs‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,8 @@ declare_clippy_lint! {
411411
}
412412

413413
declare_clippy_lint! {
414-
/// **What it does:** Lint for redundant pattern matching over `Result` or
415-
/// `Option`
414+
/// **What it does:** Lint for redundant pattern matching over `Result`, `Option` or
415+
/// `std::task::Poll`
416416
///
417417
/// **Why is this bad?** It's more concise and clear to just use the proper
418418
/// utility function
@@ -422,10 +422,13 @@ declare_clippy_lint! {
422422
/// **Example:**
423423
///
424424
/// ```rust
425+
/// # use std::task::Poll;
425426
/// if let Ok(_) = Ok::<i32, i32>(42) {}
426427
/// if let Err(_) = Err::<i32, i32>(42) {}
427428
/// if let None = None::<()> {}
428429
/// if let Some(_) = Some(42) {}
430+
/// if let Poll::Pending = Poll::Pending::<()> {}
431+
/// if let Poll::Ready(_) = Poll::Ready(42) {}
429432
/// match Ok::<i32, i32>(42) {
430433
/// Ok(_) => true,
431434
/// Err(_) => false,
@@ -435,10 +438,13 @@ declare_clippy_lint! {
435438
/// The more idiomatic use would be:
436439
///
437440
/// ```rust
441+
/// # use std::task::Poll;
438442
/// if Ok::<i32, i32>(42).is_ok() {}
439443
/// if Err::<i32, i32>(42).is_err() {}
440444
/// if None::<()>.is_none() {}
441445
/// if Some(42).is_some() {}
446+
/// if Poll::Pending::<()>.is_pending() {}
447+
/// if Poll::Ready(42).is_ready() {}
442448
/// Ok::<i32, i32>(42).is_ok();
443449
/// ```
444450
pub REDUNDANT_PATTERN_MATCHING,
@@ -1538,14 +1544,24 @@ mod redundant_pattern_match {
15381544
"is_err()"
15391545
} else if match_qpath(path, &paths::OPTION_SOME) {
15401546
"is_some()"
1547+
} else if match_qpath(path, &paths::POLL_READY) {
1548+
"is_ready()"
15411549
} else {
15421550
return;
15431551
}
15441552
} else {
15451553
return;
15461554
}
15471555
},
1548-
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
1556+
PatKind::Path(ref path) => {
1557+
if match_qpath(path, &paths::OPTION_NONE) {
1558+
"is_none()"
1559+
} else if match_qpath(path, &paths::POLL_PENDING) {
1560+
"is_pending()"
1561+
} else {
1562+
return;
1563+
}
1564+
},
15491565
_ => return,
15501566
};
15511567

@@ -1628,6 +1644,17 @@ mod redundant_pattern_match {
16281644
"is_some()",
16291645
"is_none()",
16301646
)
1647+
.or_else(|| {
1648+
find_good_method_for_match(
1649+
arms,
1650+
path_left,
1651+
path_right,
1652+
&paths::POLL_READY,
1653+
&paths::POLL_PENDING,
1654+
"is_ready()",
1655+
"is_pending()",
1656+
)
1657+
})
16311658
} else {
16321659
None
16331660
}

‎clippy_lints/src/utils/paths.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"];
9090
pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
9191
pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
9292
pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"];
93+
pub const POLL_PENDING: [&str; 5] = ["core", "task", "poll", "Poll", "Pending"];
94+
pub const POLL_READY: [&str; 5] = ["core", "task", "poll", "Poll", "Ready"];
9395
pub const PTR_EQ: [&str; 3] = ["core", "ptr", "eq"];
9496
pub const PTR_NULL: [&str; 3] = ["core", "ptr", "null"];
9597
pub const PTR_NULL_MUT: [&str; 3] = ["core", "ptr", "null_mut"];

‎tests/ui/redundant_pattern_matching_option.fixed‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,7 @@
22

33
#![warn(clippy::all)]
44
#![warn(clippy::redundant_pattern_matching)]
5-
#![allow(
6-
clippy::unit_arg,
7-
unused_must_use,
8-
clippy::needless_bool,
9-
clippy::match_like_matches_macro,
10-
deprecated
11-
)]
5+
#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
126

137
fn main() {
148
if None::<()>.is_none() {}

‎tests/ui/redundant_pattern_matching_option.rs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,7 @@
22

33
#![warn(clippy::all)]
44
#![warn(clippy::redundant_pattern_matching)]
5-
#![allow(
6-
clippy::unit_arg,
7-
unused_must_use,
8-
clippy::needless_bool,
9-
clippy::match_like_matches_macro,
10-
deprecated
11-
)]
5+
#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
126

137
fn main() {
148
if let None = None::<()> {}

‎tests/ui/redundant_pattern_matching_option.stderr‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,49 @@
11
error: redundant pattern matching, consider using `is_none()`
2-
--> $DIR/redundant_pattern_matching_option.rs:14:12
2+
--> $DIR/redundant_pattern_matching_option.rs:8:12
33
|
44
LL | if let None = None::<()> {}
55
| -------^^^^------------- help: try this: `if None::<()>.is_none()`
66
|
77
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
88

99
error: redundant pattern matching, consider using `is_some()`
10-
--> $DIR/redundant_pattern_matching_option.rs:16:12
10+
--> $DIR/redundant_pattern_matching_option.rs:10:12
1111
|
1212
LL | if let Some(_) = Some(42) {}
1313
| -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
1414

1515
error: redundant pattern matching, consider using `is_some()`
16-
--> $DIR/redundant_pattern_matching_option.rs:18:12
16+
--> $DIR/redundant_pattern_matching_option.rs:12:12
1717
|
1818
LL | if let Some(_) = Some(42) {
1919
| -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
2020

2121
error: redundant pattern matching, consider using `is_some()`
22-
--> $DIR/redundant_pattern_matching_option.rs:24:15
22+
--> $DIR/redundant_pattern_matching_option.rs:18:15
2323
|
2424
LL | while let Some(_) = Some(42) {}
2525
| ----------^^^^^^^----------- help: try this: `while Some(42).is_some()`
2626

2727
error: redundant pattern matching, consider using `is_none()`
28-
--> $DIR/redundant_pattern_matching_option.rs:26:15
28+
--> $DIR/redundant_pattern_matching_option.rs:20:15
2929
|
3030
LL | while let None = Some(42) {}
3131
| ----------^^^^----------- help: try this: `while Some(42).is_none()`
3232

3333
error: redundant pattern matching, consider using `is_none()`
34-
--> $DIR/redundant_pattern_matching_option.rs:28:15
34+
--> $DIR/redundant_pattern_matching_option.rs:22:15
3535
|
3636
LL | while let None = None::<()> {}
3737
| ----------^^^^------------- help: try this: `while None::<()>.is_none()`
3838

3939
error: redundant pattern matching, consider using `is_some()`
40-
--> $DIR/redundant_pattern_matching_option.rs:31:15
40+
--> $DIR/redundant_pattern_matching_option.rs:25:15
4141
|
4242
LL | while let Some(_) = v.pop() {
4343
| ----------^^^^^^^---------- help: try this: `while v.pop().is_some()`
4444

4545
error: redundant pattern matching, consider using `is_some()`
46-
--> $DIR/redundant_pattern_matching_option.rs:39:5
46+
--> $DIR/redundant_pattern_matching_option.rs:33:5
4747
|
4848
LL | / match Some(42) {
4949
LL | | Some(_) => true,
@@ -52,7 +52,7 @@ LL | | };
5252
| |_____^ help: try this: `Some(42).is_some()`
5353

5454
error: redundant pattern matching, consider using `is_none()`
55-
--> $DIR/redundant_pattern_matching_option.rs:44:5
55+
--> $DIR/redundant_pattern_matching_option.rs:38:5
5656
|
5757
LL | / match None::<()> {
5858
LL | | Some(_) => false,
@@ -61,7 +61,7 @@ LL | | };
6161
| |_____^ help: try this: `None::<()>.is_none()`
6262

6363
error: redundant pattern matching, consider using `is_none()`
64-
--> $DIR/redundant_pattern_matching_option.rs:49:13
64+
--> $DIR/redundant_pattern_matching_option.rs:43:13
6565
|
6666
LL | let _ = match None::<()> {
6767
| _____________^
@@ -71,49 +71,49 @@ LL | | };
7171
| |_____^ help: try this: `None::<()>.is_none()`
7272

7373
error: redundant pattern matching, consider using `is_some()`
74-
--> $DIR/redundant_pattern_matching_option.rs:55:20
74+
--> $DIR/redundant_pattern_matching_option.rs:49:20
7575
|
7676
LL | let x = if let Some(_) = opt { true } else { false };
7777
| -------^^^^^^^------ help: try this: `if opt.is_some()`
7878

7979
error: redundant pattern matching, consider using `is_some()`
80-
--> $DIR/redundant_pattern_matching_option.rs:60:20
80+
--> $DIR/redundant_pattern_matching_option.rs:54:20
8181
|
8282
LL | let _ = if let Some(_) = gen_opt() {
8383
| -------^^^^^^^------------ help: try this: `if gen_opt().is_some()`
8484

8585
error: redundant pattern matching, consider using `is_none()`
86-
--> $DIR/redundant_pattern_matching_option.rs:62:19
86+
--> $DIR/redundant_pattern_matching_option.rs:56:19
8787
|
8888
LL | } else if let None = gen_opt() {
8989
| -------^^^^------------ help: try this: `if gen_opt().is_none()`
9090

9191
error: redundant pattern matching, consider using `is_some()`
92-
--> $DIR/redundant_pattern_matching_option.rs:83:12
92+
--> $DIR/redundant_pattern_matching_option.rs:77:12
9393
|
9494
LL | if let Some(_) = Some(42) {}
9595
| -------^^^^^^^----------- help: try this: `if Some(42).is_some()`
9696

9797
error: redundant pattern matching, consider using `is_none()`
98-
--> $DIR/redundant_pattern_matching_option.rs:85:12
98+
--> $DIR/redundant_pattern_matching_option.rs:79:12
9999
|
100100
LL | if let None = None::<()> {}
101101
| -------^^^^------------- help: try this: `if None::<()>.is_none()`
102102

103103
error: redundant pattern matching, consider using `is_some()`
104-
--> $DIR/redundant_pattern_matching_option.rs:87:15
104+
--> $DIR/redundant_pattern_matching_option.rs:81:15
105105
|
106106
LL | while let Some(_) = Some(42) {}
107107
| ----------^^^^^^^----------- help: try this: `while Some(42).is_some()`
108108

109109
error: redundant pattern matching, consider using `is_none()`
110-
--> $DIR/redundant_pattern_matching_option.rs:89:15
110+
--> $DIR/redundant_pattern_matching_option.rs:83:15
111111
|
112112
LL | while let None = None::<()> {}
113113
| ----------^^^^------------- help: try this: `while None::<()>.is_none()`
114114

115115
error: redundant pattern matching, consider using `is_some()`
116-
--> $DIR/redundant_pattern_matching_option.rs:91:5
116+
--> $DIR/redundant_pattern_matching_option.rs:85:5
117117
|
118118
LL | / match Some(42) {
119119
LL | | Some(_) => true,
@@ -122,7 +122,7 @@ LL | | };
122122
| |_____^ help: try this: `Some(42).is_some()`
123123

124124
error: redundant pattern matching, consider using `is_none()`
125-
--> $DIR/redundant_pattern_matching_option.rs:96:5
125+
--> $DIR/redundant_pattern_matching_option.rs:90:5
126126
|
127127
LL | / match None::<()> {
128128
LL | | Some(_) => false,

‎tests/ui/redundant_pattern_matching_poll.fixed‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::all)]
4+
#![warn(clippy::redundant_pattern_matching)]
5+
#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
6+
7+
use std::task::Poll::{self, Pending, Ready};
8+
9+
fn main() {
10+
if Pending::<()>.is_pending() {}
11+
12+
if Ready(42).is_ready() {}
13+
14+
if Ready(42).is_ready() {
15+
foo();
16+
} else {
17+
bar();
18+
}
19+
20+
while Ready(42).is_ready() {}
21+
22+
while Ready(42).is_pending() {}
23+
24+
while Pending::<()>.is_pending() {}
25+
26+
if Pending::<i32>.is_pending() {}
27+
28+
if Ready(42).is_ready() {}
29+
30+
Ready(42).is_ready();
31+
32+
Pending::<()>.is_pending();
33+
34+
let _ = Pending::<()>.is_pending();
35+
36+
let poll = Ready(false);
37+
let x = if poll.is_ready() { true } else { false };
38+
takes_poll(x);
39+
40+
poll_const();
41+
42+
let _ = if gen_poll().is_ready() {
43+
1
44+
} else if gen_poll().is_pending() {
45+
2
46+
} else {
47+
3
48+
};
49+
}
50+
51+
fn gen_poll() -> Poll<()> {
52+
Pending
53+
}
54+
55+
fn takes_poll(_: bool) {}
56+
57+
fn foo() {}
58+
59+
fn bar() {}
60+
61+
const fn poll_const() {
62+
if Ready(42).is_ready() {}
63+
64+
if Pending::<()>.is_pending() {}
65+
66+
while Ready(42).is_ready() {}
67+
68+
while Pending::<()>.is_pending() {}
69+
70+
Ready(42).is_ready();
71+
72+
Pending::<()>.is_pending();
73+
}

0 commit comments

Comments
 (0)