Extend unnecessary_unwrap to look for expect in addition to unwrap

Closes #7581
rust-lang · Sep 3, 2021 · 03d3131 · 03d3131
1 parent a8c2c7b
commit 03d3131
Show file tree

Hide file tree

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

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

diff --git a/tests/ui/checked_unwrap/simple_conditionals.stderr b/tests/ui/checked_unwrap/simple_conditionals.stderr
@@ -12,8  12,17 @@ note: the lint level is defined here
 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
-  --> $DIR/simple_conditionals.rs:41:9
   --> $DIR/simple_conditionals.rs:42:9
    |
 LL |     if x.is_some() {
    |        ----------- because of this check
@@ -27,16  36,25 @@ note: the lint level is defined here
 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
-  --> $DIR/simple_conditionals.rs:44:9
   --> $DIR/simple_conditionals.rs:46:9
    |
 LL |     if x.is_none() {
    |        ----------- because of this check
 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`
-  --> $DIR/simple_conditionals.rs:46:9
   --> $DIR/simple_conditionals.rs:48:9
    |
 LL |     if x.is_none() {
    |        ----------- the check is happening here
@@ -58,33  76,51 @@ LL |     m!(x);
    = 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`
-  --> $DIR/simple_conditionals.rs:54:9
   --> $DIR/simple_conditionals.rs:56:9
    |
 LL |     if x.is_ok() {
    |        --------- the check is happening here
 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
-  --> $DIR/simple_conditionals.rs:55:9
   --> $DIR/simple_conditionals.rs:58:9
    |
 LL |     if x.is_ok() {
    |        --------- because of this check
-LL |         x.unwrap(); // unnecessary
 ...
 LL |         x.unwrap_err(); // will 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() {
    |        --------- because of this check
 ...
 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`
-  --> $DIR/simple_conditionals.rs:58:9
   --> $DIR/simple_conditionals.rs:62:9
    |
 LL |     if x.is_ok() {
    |        --------- the check is happening here
@@ -93,15  129,15 @@ LL |         x.unwrap_err(); // unnecessary
    |         ^^^^^^^^^^^^^^
 
 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() {
    |        ---------- because of this check
 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`
-  --> $DIR/simple_conditionals.rs:62:9
   --> $DIR/simple_conditionals.rs:66:9
    |
 LL |     if x.is_err() {
    |        ---------- 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`
-  --> $DIR/simple_conditionals.rs:64:9
   --> $DIR/simple_conditionals.rs:68:9
    |
 LL |     if x.is_err() {
    |        ---------- the check is happening here
@@ -119,13  155,13 @@ LL |         x.unwrap(); // unnecessary
    |         ^^^^^^^^^^
 
 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() {
    |        ---------- because of this check
 ...
 LL |         x.unwrap_err(); // will panic
    |         ^^^^^^^^^^^^^^
 
-error: aborting due to 13 previous errors
 error: aborting due to 17 previous errors