Skip to content

Commit

Permalink
chore: use multipart_suggestion in significant_drop_tightening lint (#…
Browse files Browse the repository at this point in the history
…13823)

This addresses #13099 for
the significant_drop_tightening lint.

changelog: none
  • Loading branch information
xFrednet authored Dec 14, 2024
2 parents bfb87b9 + db7e453 commit 5ac1805
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 23 deletions.
12 changes: 3 additions & 9 deletions clippy_lints/src/significant_drop_tightening.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,10 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
snippet(cx, apa.last_bind_ident.span, ".."),
)
};
diag.span_suggestion_verbose(
apa.first_stmt_span,

diag.multipart_suggestion_verbose(
"merge the temporary construction with its single usage",
stmt,
Applicability::MaybeIncorrect,
);
diag.span_suggestion(
apa.last_stmt_span,
"remove separated single usage",
"",
vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())],
Applicability::MaybeIncorrect,
);
},
Expand Down
144 changes: 144 additions & 0 deletions tests/ui/significant_drop_tightening.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
#![warn(clippy::significant_drop_tightening)]

use std::sync::Mutex;

pub fn complex_return_triggers_the_lint() -> i32 {
fn foo() -> i32 {
1
}
let mutex = Mutex::new(1);
let lock = mutex.lock().unwrap();
let _ = *lock;
let _ = *lock;
drop(lock);
foo()
}

pub fn issue_10413() {
let mutex = Mutex::new(Some(1));
let opt = Some(1);
if opt.is_some() {
let lock = mutex.lock().unwrap();
let _ = *lock;
if opt.is_some() {
let _ = *lock;
}
}
}

pub fn issue_11128() {
use std::mem::drop as unlock;

struct Foo {
droppable: Option<Vec<i32>>,
mutex: Mutex<Vec<i32>>,
}

impl Drop for Foo {
fn drop(&mut self) {
if let Some(droppable) = self.droppable.take() {
let lock = self.mutex.lock().unwrap();
let idx_opt = lock.iter().copied().find(|el| Some(el) == droppable.first());
if let Some(idx) = idx_opt {
let local_droppable = vec![lock.first().copied().unwrap_or_default()];
unlock(lock);
drop(local_droppable);
}
}
}
}
}

pub fn issue_11160() -> bool {
let mutex = Mutex::new(1i32);
let lock = mutex.lock().unwrap();
let _ = lock.abs();
true
}

pub fn issue_11189() {
struct Number {
pub value: u32,
}

fn do_something() -> Result<(), ()> {
let number = Mutex::new(Number { value: 1 });
let number2 = Mutex::new(Number { value: 2 });
let number3 = Mutex::new(Number { value: 3 });
let mut lock = number.lock().unwrap();
let mut lock2 = number2.lock().unwrap();
let mut lock3 = number3.lock().unwrap();
lock.value += 1;
lock2.value += 1;
lock3.value += 1;
drop((lock, lock2, lock3));
Ok(())
}
}

pub fn path_return_can_be_ignored() -> i32 {
let mutex = Mutex::new(1);
let lock = mutex.lock().unwrap();
let rslt = *lock;
let _ = *lock;
rslt
}

pub fn post_bindings_can_be_ignored() {
let mutex = Mutex::new(1);
let lock = mutex.lock().unwrap();
let rslt = *lock;
let another = rslt;
let _ = another;
}

pub fn unnecessary_contention_with_multiple_owned_results() {
{
let mutex = Mutex::new(1i32);
let lock = mutex.lock().unwrap();
let _ = lock.abs();
let _ = lock.is_positive();
}

{
let mutex = Mutex::new(1i32);
let lock = mutex.lock().unwrap();
let rslt0 = lock.abs();
let rslt1 = lock.is_positive();
drop(lock);
do_heavy_computation_that_takes_time((rslt0, rslt1));
}
}

pub fn unnecessary_contention_with_single_owned_results() {
{
let mutex = Mutex::new(1i32);
let lock = mutex.lock().unwrap();
let _ = lock.abs();
}
{
let mutex = Mutex::new(vec![1i32]);
let mut lock = mutex.lock().unwrap();
lock.clear();
}

{
let mutex = Mutex::new(1i32);

let rslt0 = mutex.lock().unwrap().abs();

do_heavy_computation_that_takes_time(rslt0);
}
{
let mutex = Mutex::new(vec![1i32]);

mutex.lock().unwrap().clear();

do_heavy_computation_that_takes_time(());
}
}

// Marker used for illustration purposes.
pub fn do_heavy_computation_that_takes_time<T>(_: T) {}

fn main() {}
2 changes: 0 additions & 2 deletions tests/ui/significant_drop_tightening.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#![warn(clippy::significant_drop_tightening)]

//@no-rustfix: need to change the suggestion to a multipart suggestion

use std::sync::Mutex;

pub fn complex_return_triggers_the_lint() -> i32 {
Expand Down
18 changes: 6 additions & 12 deletions tests/ui/significant_drop_tightening.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: temporary with significant `Drop` can be early dropped
--> tests/ui/significant_drop_tightening.rs:12:9
--> tests/ui/significant_drop_tightening.rs:10:9
|
LL | pub fn complex_return_triggers_the_lint() -> i32 {
| __________________________________________________-
Expand All @@ -24,7 +24,7 @@ LL + drop(lock);
|

error: temporary with significant `Drop` can be early dropped
--> tests/ui/significant_drop_tightening.rs:106:13
--> tests/ui/significant_drop_tightening.rs:104:13
|
LL | / {
LL | | let mutex = Mutex::new(1i32);
Expand All @@ -44,7 +44,7 @@ LL + drop(lock);
|

error: temporary with significant `Drop` can be early dropped
--> tests/ui/significant_drop_tightening.rs:127:13
--> tests/ui/significant_drop_tightening.rs:125:13
|
LL | / {
LL | | let mutex = Mutex::new(1i32);
Expand All @@ -60,14 +60,11 @@ help: merge the temporary construction with its single usage
|
LL ~
LL + let rslt0 = mutex.lock().unwrap().abs();
|
help: remove separated single usage
|
LL - let rslt0 = lock.abs();
LL ~
|

error: temporary with significant `Drop` can be early dropped
--> tests/ui/significant_drop_tightening.rs:133:17
--> tests/ui/significant_drop_tightening.rs:131:17
|
LL | / {
LL | | let mutex = Mutex::new(vec![1i32]);
Expand All @@ -83,10 +80,7 @@ help: merge the temporary construction with its single usage
|
LL ~
LL + mutex.lock().unwrap().clear();
|
help: remove separated single usage
|
LL - lock.clear();
LL ~
|

error: aborting due to 4 previous errors
Expand Down

0 comments on commit 5ac1805

Please sign in to comment.