Skip to content

Commit

Permalink
fix: correct suggestion for significant_drop_in_scrutinee in expressi…
Browse files Browse the repository at this point in the history
…ons (#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
  • Loading branch information
y21 authored Jan 21, 2025
2 parents 8f1b4bb + 23e602c commit d1b5fa2
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 3 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/matches/significant_drop_in_scrutinee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/significant_drop_in_scrutinee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
18 changes: 17 additions & 1 deletion tests/ui/significant_drop_in_scrutinee.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit d1b5fa2

Please sign in to comment.