about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2025-05-08 22:12:02 +0000
committerGitHub <noreply@github.com>2025-05-08 22:12:02 +0000
commitd8726caacc5240b322591fce776759543b833251 (patch)
tree72c835a696cc466003c5705d8738dc2a4323011a
parent33519b7c995f97ebaccd472a3578f928aca0e62b (diff)
parentb73d3b4384e65ab3d0d7af0e2d2f6c07ef1620aa (diff)
downloadrust-d8726caacc5240b322591fce776759543b833251.tar.gz
rust-d8726caacc5240b322591fce776759543b833251.zip
fix false negative for `unnecessary_unwrap` (#14758)
changelog: Fix [`unnecessary_unwrap`] false negative when any assignment
occurs in `if` branch (regardless of any variable).

Fixes: rust-lang/rust-clippy#14725
-rw-r--r--clippy_lints/src/unwrap.rs6
-rw-r--r--tests/ui/checked_unwrap/simple_conditionals.rs52
-rw-r--r--tests/ui/checked_unwrap/simple_conditionals.stderr37
3 files changed, 92 insertions, 3 deletions
diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs
index ba140788bb5..f152d62bf49 100644
--- a/clippy_lints/src/unwrap.rs
+++ b/clippy_lints/src/unwrap.rs
@@ -224,8 +224,10 @@ impl<'tcx> Delegate<'tcx> for MutationVisitor<'tcx> {
         }
     }
 
-    fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {
-        self.is_mutated = true;
+    fn mutate(&mut self, cat: &PlaceWithHirId<'tcx>, _: HirId) {
+        if is_potentially_local_place(self.local_id, &cat.place) {
+            self.is_mutated = true;
+        }
     }
 
     fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
diff --git a/tests/ui/checked_unwrap/simple_conditionals.rs b/tests/ui/checked_unwrap/simple_conditionals.rs
index 4101897d380..5589d8cc429 100644
--- a/tests/ui/checked_unwrap/simple_conditionals.rs
+++ b/tests/ui/checked_unwrap/simple_conditionals.rs
@@ -188,6 +188,58 @@ fn issue11371() {
     }
 }
 
+fn gen_option() -> Option<()> {
+    Some(())
+    // Or None
+}
+
+fn gen_result() -> Result<(), ()> {
+    Ok(())
+    // Or Err(())
+}
+
+fn issue14725() {
+    let option = Some(());
+
+    if option.is_some() {
+        let _ = option.as_ref().unwrap();
+        //~^ unnecessary_unwrap
+    } else {
+        let _ = option.as_ref().unwrap();
+        //~^ panicking_unwrap
+    }
+
+    let result = Ok::<(), ()>(());
+
+    if result.is_ok() {
+        let _y = 1;
+        result.as_ref().unwrap();
+        //~^ unnecessary_unwrap
+    } else {
+        let _y = 1;
+        result.as_ref().unwrap();
+        //~^ panicking_unwrap
+    }
+
+    let mut option = Some(());
+    if option.is_some() {
+        option = gen_option();
+        option.as_mut().unwrap();
+    } else {
+        option = gen_option();
+        option.as_mut().unwrap();
+    }
+
+    let mut result = Ok::<(), ()>(());
+    if result.is_ok() {
+        result = gen_result();
+        result.as_mut().unwrap();
+    } else {
+        result = gen_result();
+        result.as_mut().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 ad3c420270c..82a36aa5029 100644
--- a/tests/ui/checked_unwrap/simple_conditionals.stderr
+++ b/tests/ui/checked_unwrap/simple_conditionals.stderr
@@ -236,6 +236,41 @@ LL |     if result.is_ok() {
 LL |         result.as_mut().unwrap();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^
 
+error: called `unwrap` on `option` after checking its variant with `is_some`
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:205:17
+   |
+LL |     if option.is_some() {
+   |     ------------------- help: try: `if let Some(<item>) = &option`
+LL |         let _ = option.as_ref().unwrap();
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:208:17
+   |
+LL |     if option.is_some() {
+   |        ---------------- because of this check
+...
+LL |         let _ = option.as_ref().unwrap();
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: called `unwrap` on `result` after checking its variant with `is_ok`
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:216:9
+   |
+LL |     if result.is_ok() {
+   |     ----------------- help: try: `if let Ok(<item>) = &result`
+LL |         let _y = 1;
+LL |         result.as_ref().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this call to `unwrap()` will always panic
+  --> tests/ui/checked_unwrap/simple_conditionals.rs:220:9
+   |
+LL |     if result.is_ok() {
+   |        -------------- because of this check
+...
+LL |         result.as_ref().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
 error: creating a shared reference to mutable static
   --> tests/ui/checked_unwrap/simple_conditionals.rs:183:12
    |
@@ -246,5 +281,5 @@ LL |         if X.is_some() {
    = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives
    = note: `#[deny(static_mut_refs)]` on by default
 
-error: aborting due to 26 previous errors
+error: aborting due to 30 previous errors