Skip to content

Commit 03d3131

Browse files
committed
Extend unnecessary_unwrap to look for expect in addition to unwrap
Closes #7581
1 parent a8c2c7b commit 03d3131

File tree

3 files changed

+55
-15
lines changed

3 files changed

+55
-15
lines changed

clippy_lints/src/unwrap.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
172172
if_chain! {
173173
if let ExprKind::MethodCall(method_name, _, args, _) = expr.kind;
174174
if let ExprKind::Path(QPath::Resolved(None, path)) = args[0].kind;
175-
if [sym::unwrap, sym!(unwrap_err)].contains(&method_name.ident.name);
176-
let call_to_unwrap = method_name.ident.name == sym::unwrap;
175+
if [sym::unwrap, sym::expect, sym!(unwrap_err)].contains(&method_name.ident.name);
176+
let call_to_unwrap = [sym::unwrap, sym::expect].contains(&method_name.ident.name);
177177
if let Some(unwrappable) = self.unwrappables.iter()
178178
.find(|u| u.ident.res == path.res);
179179
// Span contexts should not differ with the conditional branch

tests/ui/checked_unwrap/simple_conditionals.rs

+4
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@ fn main() {
3737
let x = Some(());
3838
if x.is_some() {
3939
x.unwrap(); // unnecessary
40+
x.expect("an error message"); // unnecessary
4041
} else {
4142
x.unwrap(); // will panic
43+
x.expect("an error message"); // will panic
4244
}
4345
if x.is_none() {
4446
x.unwrap(); // will panic
@@ -52,9 +54,11 @@ fn main() {
5254
let mut x: Result<(), ()> = Ok(());
5355
if x.is_ok() {
5456
x.unwrap(); // unnecessary
57+
x.expect("an error message"); // unnecessary
5558
x.unwrap_err(); // will panic
5659
} else {
5760
x.unwrap(); // will panic
61+
x.expect("an error message"); // will panic
5862
x.unwrap_err(); // unnecessary
5963
}
6064
if x.is_err() {

tests/ui/checked_unwrap/simple_conditionals.stderr

+49-13
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,17 @@ note: the lint level is defined here
1212
LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
1414

15+
error: you checked before that `expect()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
16+
--> $DIR/simple_conditionals.rs:40:9
17+
|
18+
LL | if x.is_some() {
19+
| ----------- the check is happening here
20+
LL | x.unwrap(); // unnecessary
21+
LL | x.expect("an error message"); // unnecessary
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
1524
error: this call to `unwrap()` will always panic
16-
--> $DIR/simple_conditionals.rs:41:9
25+
--> $DIR/simple_conditionals.rs:42:9
1726
|
1827
LL | if x.is_some() {
1928
| ----------- because of this check
@@ -27,16 +36,25 @@ note: the lint level is defined here
2736
LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
2837
| ^^^^^^^^^^^^^^^^^^^^^^^^
2938

39+
error: this call to `expect()` will always panic
40+
--> $DIR/simple_conditionals.rs:43:9
41+
|
42+
LL | if x.is_some() {
43+
| ----------- because of this check
44+
...
45+
LL | x.expect("an error message"); // will panic
46+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
47+
3048
error: this call to `unwrap()` will always panic
31-
--> $DIR/simple_conditionals.rs:44:9
49+
--> $DIR/simple_conditionals.rs:46:9
3250
|
3351
LL | if x.is_none() {
3452
| ----------- because of this check
3553
LL | x.unwrap(); // will panic
3654
| ^^^^^^^^^^
3755

3856
error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
39-
--> $DIR/simple_conditionals.rs:46:9
57+
--> $DIR/simple_conditionals.rs:48:9
4058
|
4159
LL | if x.is_none() {
4260
| ----------- the check is happening here
@@ -58,33 +76,51 @@ LL | m!(x);
5876
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
5977

6078
error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
61-
--> $DIR/simple_conditionals.rs:54:9
79+
--> $DIR/simple_conditionals.rs:56:9
6280
|
6381
LL | if x.is_ok() {
6482
| --------- the check is happening here
6583
LL | x.unwrap(); // unnecessary
6684
| ^^^^^^^^^^
6785

86+
error: you checked before that `expect()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
87+
--> $DIR/simple_conditionals.rs:57:9
88+
|
89+
LL | if x.is_ok() {
90+
| --------- the check is happening here
91+
LL | x.unwrap(); // unnecessary
92+
LL | x.expect("an error message"); // unnecessary
93+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
94+
6895
error: this call to `unwrap_err()` will always panic
69-
--> $DIR/simple_conditionals.rs:55:9
96+
--> $DIR/simple_conditionals.rs:58:9
7097
|
7198
LL | if x.is_ok() {
7299
| --------- because of this check
73-
LL | x.unwrap(); // unnecessary
100+
...
74101
LL | x.unwrap_err(); // will panic
75102
| ^^^^^^^^^^^^^^
76103

77104
error: this call to `unwrap()` will always panic
78-
--> $DIR/simple_conditionals.rs:57:9
105+
--> $DIR/simple_conditionals.rs:60:9
79106
|
80107
LL | if x.is_ok() {
81108
| --------- because of this check
82109
...
83110
LL | x.unwrap(); // will panic
84111
| ^^^^^^^^^^
85112

113+
error: this call to `expect()` will always panic
114+
--> $DIR/simple_conditionals.rs:61:9
115+
|
116+
LL | if x.is_ok() {
117+
| --------- because of this check
118+
...
119+
LL | x.expect("an error message"); // will panic
120+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
121+
86122
error: you checked before that `unwrap_err()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
87-
--> $DIR/simple_conditionals.rs:58:9
123+
--> $DIR/simple_conditionals.rs:62:9
88124
|
89125
LL | if x.is_ok() {
90126
| --------- the check is happening here
@@ -93,15 +129,15 @@ LL | x.unwrap_err(); // unnecessary
93129
| ^^^^^^^^^^^^^^
94130

95131
error: this call to `unwrap()` will always panic
96-
--> $DIR/simple_conditionals.rs:61:9
132+
--> $DIR/simple_conditionals.rs:65:9
97133
|
98134
LL | if x.is_err() {
99135
| ---------- because of this check
100136
LL | x.unwrap(); // will panic
101137
| ^^^^^^^^^^
102138

103139
error: you checked before that `unwrap_err()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
104-
--> $DIR/simple_conditionals.rs:62:9
140+
--> $DIR/simple_conditionals.rs:66:9
105141
|
106142
LL | if x.is_err() {
107143
| ---------- the check is happening here
@@ -110,7 +146,7 @@ LL | x.unwrap_err(); // unnecessary
110146
| ^^^^^^^^^^^^^^
111147

112148
error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
113-
--> $DIR/simple_conditionals.rs:64:9
149+
--> $DIR/simple_conditionals.rs:68:9
114150
|
115151
LL | if x.is_err() {
116152
| ---------- the check is happening here
@@ -119,13 +155,13 @@ LL | x.unwrap(); // unnecessary
119155
| ^^^^^^^^^^
120156

121157
error: this call to `unwrap_err()` will always panic
122-
--> $DIR/simple_conditionals.rs:65:9
158+
--> $DIR/simple_conditionals.rs:69:9
123159
|
124160
LL | if x.is_err() {
125161
| ---------- because of this check
126162
...
127163
LL | x.unwrap_err(); // will panic
128164
| ^^^^^^^^^^^^^^
129165

130-
error: aborting due to 13 previous errors
166+
error: aborting due to 17 previous errors
131167

0 commit comments

Comments
 (0)