Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-23 21:23:56 +00:00
Code Issues Projects Releases Packages Wiki Activity

Extend unnecessary_unwrap to look for expect in addition to unwrap

Browse source 
Closes #7581
This commit is contained in:
Jake Goulding 2021-08-18 09:56:25 -04:00
parent a8c2c7b712
commit 03d3131cb4
3 changed files with 55 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
clippy_lints/src/unwrap.rs
View file

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

4
tests/ui/checked_unwrap/simple_conditionals.rs
View file

@ -37,8 +37,10 @@ fn main() {
let x = Some(()); let x = Some(());
if x.is_some() { if x.is_some() {
x.unwrap(); // unnecessary x.unwrap(); // unnecessary
x.expect("an error message"); // unnecessary
} else { } else {
x.unwrap(); // will panic x.unwrap(); // will panic
x.expect("an error message"); // will panic
} }
if x.is_none() { if x.is_none() {
x.unwrap(); // will panic x.unwrap(); // will panic
@ -52,9 +54,11 @@ fn main() {
let mut x: Result<(), ()> = Ok(()); let mut x: Result<(), ()> = Ok(());
if x.is_ok() { if x.is_ok() {
x.unwrap(); // unnecessary x.unwrap(); // unnecessary
x.expect("an error message"); // unnecessary
x.unwrap_err(); // will panic x.unwrap_err(); // will panic
} else { } else {
x.unwrap(); // will panic x.unwrap(); // will panic
x.expect("an error message"); // will panic
x.unwrap_err(); // unnecessary x.unwrap_err(); // unnecessary
} }
if x.is_err() { if x.is_err() {

62
tests/ui/checked_unwrap/simple_conditionals.stderr
View file

@ -12,8 +12,17 @@ note: the lint level is defined here
LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)] LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^
error: you checked before that `expect()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
--> $DIR/simple_conditionals.rs:40:9
|
LL | if x.is_some() {
| ----------- the check is happening here
LL | x.unwrap(); // unnecessary
LL | x.expect("an error message"); // unnecessary
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this call to `unwrap()` will always panic error: this call to `unwrap()` will always panic
--> $DIR/simple_conditionals.rs:41:9 --> $DIR/simple_conditionals.rs:42:9
| |
LL | if x.is_some() { LL | if x.is_some() {
| ----------- because of this check | ----------- because of this check
@ -27,8 +36,17 @@ note: the lint level is defined here
LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)] LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^
error: this call to `expect()` will always panic
--> $DIR/simple_conditionals.rs:43:9
|
LL | if x.is_some() {
| ----------- because of this check
...
LL | x.expect("an error message"); // will panic
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this call to `unwrap()` will always panic error: this call to `unwrap()` will always panic
--> $DIR/simple_conditionals.rs:44:9 --> $DIR/simple_conditionals.rs:46:9
| |
LL | if x.is_none() { LL | if x.is_none() {
| ----------- because of this check | ----------- because of this check
@ -36,7 +54,7 @@ LL | x.unwrap(); // will panic
| ^^^^^^^^^^ | ^^^^^^^^^^
error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match` error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
--> $DIR/simple_conditionals.rs:46:9 --> $DIR/simple_conditionals.rs:48:9
| |
LL | if x.is_none() { LL | if x.is_none() {
| ----------- the check is happening here | ----------- the check is happening here
@ -58,24 +76,33 @@ LL | m!(x);
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info) = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match` error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
--> $DIR/simple_conditionals.rs:54:9 --> $DIR/simple_conditionals.rs:56:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- the check is happening here | --------- the check is happening here
LL | x.unwrap(); // unnecessary LL | x.unwrap(); // unnecessary
| ^^^^^^^^^^ | ^^^^^^^^^^
error: you checked before that `expect()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
--> $DIR/simple_conditionals.rs:57:9
|
LL | if x.is_ok() {
| --------- the check is happening here
LL | x.unwrap(); // unnecessary
LL | x.expect("an error message"); // unnecessary
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this call to `unwrap_err()` will always panic error: this call to `unwrap_err()` will always panic
--> $DIR/simple_conditionals.rs:55:9 --> $DIR/simple_conditionals.rs:58:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- because of this check | --------- because of this check
LL | x.unwrap(); // unnecessary ...
LL | x.unwrap_err(); // will panic LL | x.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: this call to `unwrap()` will always panic error: this call to `unwrap()` will always panic
--> $DIR/simple_conditionals.rs:57:9 --> $DIR/simple_conditionals.rs:60:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- because of this check | --------- because of this check
@ -83,8 +110,17 @@ LL | if x.is_ok() {
LL | x.unwrap(); // will panic LL | x.unwrap(); // will panic
| ^^^^^^^^^^ | ^^^^^^^^^^
error: this call to `expect()` will always panic
--> $DIR/simple_conditionals.rs:61:9
|
LL | if x.is_ok() {
| --------- because of this check
...
LL | x.expect("an error message"); // will panic
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: you checked before that `unwrap_err()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match` error: you checked before that `unwrap_err()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
--> $DIR/simple_conditionals.rs:58:9 --> $DIR/simple_conditionals.rs:62:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- the check is happening here | --------- the check is happening here
@ -93,7 +129,7 @@ LL | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: this call to `unwrap()` will always panic error: this call to `unwrap()` will always panic
--> $DIR/simple_conditionals.rs:61:9 --> $DIR/simple_conditionals.rs:65:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- because of this check | ---------- because of this check
@ -101,7 +137,7 @@ LL | x.unwrap(); // will panic
| ^^^^^^^^^^ | ^^^^^^^^^^
error: you checked before that `unwrap_err()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match` error: you checked before that `unwrap_err()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
--> $DIR/simple_conditionals.rs:62:9 --> $DIR/simple_conditionals.rs:66:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- the check is happening here | ---------- the check is happening here
@ -110,7 +146,7 @@ LL | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match` error: you checked before that `unwrap()` cannot fail, instead of checking and unwrapping, it's better to use `if let` or `match`
--> $DIR/simple_conditionals.rs:64:9 --> $DIR/simple_conditionals.rs:68:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- the check is happening here | ---------- the check is happening here
@ -119,7 +155,7 @@ LL | x.unwrap(); // unnecessary
| ^^^^^^^^^^ | ^^^^^^^^^^
error: this call to `unwrap_err()` will always panic error: this call to `unwrap_err()` will always panic
--> $DIR/simple_conditionals.rs:65:9 --> $DIR/simple_conditionals.rs:69:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- because of this check | ---------- because of this check
@ -127,5 +163,5 @@ LL | if x.is_err() {
LL | x.unwrap_err(); // will panic LL | x.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: aborting due to 13 previous errors error: aborting due to 17 previous errors