about summary refs log tree commit diff
diff options
context:
space:
mode:
authorTimo <30553356+y21@users.noreply.github.com>2025-01-21 02:28:23 +0000
committerGitHub <noreply@github.com>2025-01-21 02:28:23 +0000
commitd1b5fa2416f48abc4e5f54df040a3aed864c88cf (patch)
tree2248fd291ae54532fe894c5ce70a15f7aad48f6a
parent8f1b4bb87a9c028391f779c83dd4cecb615bd67b (diff)
parent23e602cd94efecf2136a29e77d335aedf9ffea74 (diff)
downloadrust-d1b5fa2416f48abc4e5f54df040a3aed864c88cf.tar.gz
rust-d1b5fa2416f48abc4e5f54df040a3aed864c88cf.zip
fix: correct suggestion for significant_drop_in_scrutinee in expressions (#14019)
This PR fixes an issue with the `significant_drop_in_scrutinee`, where
the lint generates invalid Rust syntax when suggesting fixes for match
expressions that are part of larger expressions, such as in assignment
contexts. For example:

```rust
    let mutex = Mutex::new(State {});
    let _ = match mutex.lock().unwrap().foo() {
        true => 0,
        false => 1,
    };
```
would suggest:
```rust
let _ = let value = mutex.lock().unwrap().foo();
match value {
```
With this PR, it now suggests:
```rust
let value = mutex.lock().unwrap().foo();
let _ = match value {
```

closes: #13986

changelog: [`significant_drop_in_scrutinee`] Fix incorrect suggestion
for `significant_drop_in_scrutinee` lint in expression context
-rw-r--r--clippy_lints/src/matches/significant_drop_in_scrutinee.rs4
-rw-r--r--tests/ui/significant_drop_in_scrutinee.rs14
-rw-r--r--tests/ui/significant_drop_in_scrutinee.stderr18
3 files changed, 33 insertions, 3 deletions
diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs
index 9d8e0a69433..35f2e780d2e 100644
--- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs
+++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs
@@ -2,7 +2,7 @@ use std::ops::ControlFlow;
 
 use crate::FxHashSet;
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::source::{indent_of, snippet};
+use clippy_utils::source::{first_line_of_span, indent_of, snippet};
 use clippy_utils::ty::{for_each_top_level_late_bound_region, is_copy};
 use clippy_utils::{get_attr, is_lint_allowed};
 use itertools::Itertools;
@@ -152,7 +152,7 @@ fn set_suggestion<'tcx>(diag: &mut Diag<'_, ()>, cx: &LateContext<'tcx>, expr: &
     diag.multipart_suggestion(
         suggestion_message,
         vec![
-            (expr.span.shrink_to_lo(), replacement),
+            (first_line_of_span(cx, expr.span).shrink_to_lo(), replacement),
             (found.found_span, scrutinee_replacement),
         ],
         Applicability::MaybeIncorrect,
diff --git a/tests/ui/significant_drop_in_scrutinee.rs b/tests/ui/significant_drop_in_scrutinee.rs
index e9ebd23beac..39d550398d7 100644
--- a/tests/ui/significant_drop_in_scrutinee.rs
+++ b/tests/ui/significant_drop_in_scrutinee.rs
@@ -850,4 +850,18 @@ async fn should_not_trigger_lint_in_async_expansion(mutex: Mutex<i32>) -> i32 {
     }
 }
 
+fn should_trigger_lint_in_match_expr() {
+    let mutex = Mutex::new(State {});
+
+    // Should trigger lint because the lifetime of the temporary MutexGuard is surprising because it
+    // is preserved until the end of the match, but there is no clear indication that this is the
+    // case.
+    let _ = match mutex.lock().unwrap().foo() {
+        //~^ ERROR: temporary with significant `Drop` in `match` scrutinee will live until the
+        //~| NOTE: this might lead to deadlocks or other unexpected behavior
+        true => 0,
+        false => 1,
+    };
+}
+
 fn main() {}
diff --git a/tests/ui/significant_drop_in_scrutinee.stderr b/tests/ui/significant_drop_in_scrutinee.stderr
index 23e38948ec5..f99d862aa6b 100644
--- a/tests/ui/significant_drop_in_scrutinee.stderr
+++ b/tests/ui/significant_drop_in_scrutinee.stderr
@@ -584,5 +584,21 @@ LL ~     let value = *foo_async(&mutex).await.unwrap();
 LL ~     match value {
    |
 
-error: aborting due to 30 previous errors
+error: temporary with significant `Drop` in `match` scrutinee will live until the end of the `match` expression
+  --> tests/ui/significant_drop_in_scrutinee.rs:859:19
+   |
+LL |     let _ = match mutex.lock().unwrap().foo() {
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     };
+   |      - temporary lives until here
+   |
+   = note: this might lead to deadlocks or other unexpected behavior
+help: try moving the temporary above the match
+   |
+LL ~     let value = mutex.lock().unwrap().foo();
+LL ~     let _ = match value {
+   |
+
+error: aborting due to 31 previous errors