Auto merge of #11387 - y21:issue11371, r=blyxyas

[`unnecessary_unwrap`]: lint on `.as_ref().unwrap()`

Closes #11371

This turned out to be a little more code than I originally thought, because the lint also makes sure to not lint if the user tries to mutate the option:
```rs
if option.is_some() {
  option = None;
  option.unwrap(); // don't lint here
}
```
... which means that even if we taught this lint to recognize `.as_mut()`, it would *still* not lint because that would count as a mutation. So we need to allow `.as_mut()` calls but reject other kinds of mutations.
Unfortunately it doesn't look like this is possible with `is_potentially_mutated` (seeing what kind of mutation happened).
This replaces it with a custom little visitor that does basically what it did before, but also allows `.as_mut()`.

changelog: [`unnecessary_unwrap`]: lint on `.as_ref().unwrap()`

2 files changed, 120 insertions, 1 deletions

diff --git a/tests/ui/checked_unwrap/simple_conditionals.rs b/tests/ui/checked_unwrap/simple_conditionals.rs
index e82e7bcb06c..02f80cc52ac 100644
--- a/tests/ui/checked_unwrap/simple_conditionals.rs
+++ b/tests/ui/checked_unwrap/simple_conditionals.rs
@@ -128,6 +128,57 @@ fn main() {
     assert!(x.is_ok(), "{:?}", x.unwrap_err());
 }
 
+fn issue11371() {
+    let option = Some(());
+
+    if option.is_some() {
+        option.as_ref().unwrap();
+        //~^ ERROR: called `unwrap` on `option` after checking its variant with `is_some`
+    } else {
+        option.as_ref().unwrap();
+        //~^ ERROR: this call to `unwrap()` will always panic
+    }
+
+    let result = Ok::<(), ()>(());
+
+    if result.is_ok() {
+        result.as_ref().unwrap();
+        //~^ ERROR: called `unwrap` on `result` after checking its variant with `is_ok`
+    } else {
+        result.as_ref().unwrap();
+        //~^ ERROR: this call to `unwrap()` will always panic
+    }
+
+    let mut option = Some(());
+    if option.is_some() {
+        option.as_mut().unwrap();
+        //~^ ERROR: called `unwrap` on `option` after checking its variant with `is_some`
+    } else {
+        option.as_mut().unwrap();
+        //~^ ERROR: this call to `unwrap()` will always panic
+    }
+
+    let mut result = Ok::<(), ()>(());
+    if result.is_ok() {
+        result.as_mut().unwrap();
+        //~^ ERROR: called `unwrap` on `result` after checking its variant with `is_ok`
+    } else {
+        result.as_mut().unwrap();
+        //~^ ERROR: this call to `unwrap()` will always panic
+    }
+
+    // This should not lint. Statics are, at the time of writing, not linted on anyway,
+    // but if at some point they are supported by this lint, it should correctly see that
+    // `X` is being mutated and not suggest `if let Some(..) = X {}`
+    static mut X: Option<i32> = Some(123);
+    unsafe {
+        if X.is_some() {
+            X = None;
+            X.unwrap();
+        }
+    }
+}
+
 fn check_expect() {
     let x = Some(());
     if x.is_some() {
diff --git a/tests/ui/checked_unwrap/simple_conditionals.stderr b/tests/ui/checked_unwrap/simple_conditionals.stderr
index ed603581ecd..a5afbba7317 100644
--- a/tests/ui/checked_unwrap/simple_conditionals.stderr
+++ b/tests/ui/checked_unwrap/simple_conditionals.stderr
@@ -168,5 +168,73 @@ LL |     if x.is_err() {
 LL |         x.unwrap_err();
    |         ^^^^^^^^^^^^^^
 
-error: aborting due to 17 previous errors
+error: called `unwrap` on `option` after checking its variant with `is_some`
+  --> $DIR/simple_conditionals.rs:135:9
+   |
+LL |     if option.is_some() {
+   |     ------------------- help: try: `if let Some(..) = &option`
+LL |         option.as_ref().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> $DIR/simple_conditionals.rs:138:9
+   |
+LL |     if option.is_some() {
+   |        ---------------- because of this check
+...
+LL |         option.as_ref().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: called `unwrap` on `result` after checking its variant with `is_ok`
+  --> $DIR/simple_conditionals.rs:145:9
+   |
+LL |     if result.is_ok() {
+   |     ----------------- help: try: `if let Ok(..) = &result`
+LL |         result.as_ref().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> $DIR/simple_conditionals.rs:148:9
+   |
+LL |     if result.is_ok() {
+   |        -------------- because of this check
+...
+LL |         result.as_ref().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: called `unwrap` on `option` after checking its variant with `is_some`
+  --> $DIR/simple_conditionals.rs:154:9
+   |
+LL |     if option.is_some() {
+   |     ------------------- help: try: `if let Some(..) = &mut option`
+LL |         option.as_mut().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> $DIR/simple_conditionals.rs:157:9
+   |
+LL |     if option.is_some() {
+   |        ---------------- because of this check
+...
+LL |         option.as_mut().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: called `unwrap` on `result` after checking its variant with `is_ok`
+  --> $DIR/simple_conditionals.rs:163:9
+   |
+LL |     if result.is_ok() {
+   |     ----------------- help: try: `if let Ok(..) = &mut result`
+LL |         result.as_mut().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> $DIR/simple_conditionals.rs:166:9
+   |
+LL |     if result.is_ok() {
+   |        -------------- because of this check
+...
+LL |         result.as_mut().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 25 previous errors