Skip to content

Commit 2f0f4dd

Browse files
committed
Auto merge of #11698 - a1phyr:waker_clone_and_wake, r=y21
Add `waker_clone_and_wake` lint to check needless `Waker` clones Check for patterns of `waker.clone().wake()` and replace them with `waker.wake_by_ref()`. An alternative name could be `waker_clone_then_wake` changelog: [ `waker_clone_wake`]: new lint
2 parents 392b255 + ebf6667 commit 2f0f4dd

File tree

8 files changed

+137
-0
lines changed

8 files changed

+137
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5589,6 +5589,7 @@ Released 2018-09-13
55895589
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
55905590
[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads
55915591
[`vtable_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons
5592+
[`waker_clone_wake`]: https://rust-lang.github.io/rust-clippy/master/index.html#waker_clone_wake
55925593
[`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition
55935594
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop
55945595
[`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
443443
crate::methods::USELESS_ASREF_INFO,
444444
crate::methods::VEC_RESIZE_TO_ZERO_INFO,
445445
crate::methods::VERBOSE_FILE_READS_INFO,
446+
crate::methods::WAKER_CLONE_WAKE_INFO,
446447
crate::methods::WRONG_SELF_CONVENTION_INFO,
447448
crate::methods::ZST_OFFSET_INFO,
448449
crate::min_ident_chars::MIN_IDENT_CHARS_INFO,

clippy_lints/src/methods/mod.rs

+27
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ mod useless_asref;
112112
mod utils;
113113
mod vec_resize_to_zero;
114114
mod verbose_file_reads;
115+
mod waker_clone_wake;
115116
mod wrong_self_convention;
116117
mod zst_offset;
117118

@@ -3632,6 +3633,28 @@ declare_clippy_lint! {
36323633
"`as_str` used to call a method on `str` that is also available on `String`"
36333634
}
36343635

3636+
declare_clippy_lint! {
3637+
/// ### What it does
3638+
/// Checks for usage of `waker.clone().wake()`
3639+
///
3640+
/// ### Why is this bad?
3641+
/// Cloning the waker is not necessary, `wake_by_ref()` enables the same operation
3642+
/// without extra cloning/dropping.
3643+
///
3644+
/// ### Example
3645+
/// ```rust,ignore
3646+
/// waker.clone().wake();
3647+
/// ```
3648+
/// Should be written
3649+
/// ```rust,ignore
3650+
/// waker.wake_by_ref();
3651+
/// ```
3652+
#[clippy::version = "1.75.0"]
3653+
pub WAKER_CLONE_WAKE,
3654+
perf,
3655+
"cloning a `Waker` only to wake it"
3656+
}
3657+
36353658
pub struct Methods {
36363659
avoid_breaking_exported_api: bool,
36373660
msrv: Msrv,
@@ -3777,6 +3800,7 @@ impl_lint_pass!(Methods => [
37773800
ITER_OUT_OF_BOUNDS,
37783801
PATH_ENDS_WITH_EXT,
37793802
REDUNDANT_AS_STR,
3803+
WAKER_CLONE_WAKE,
37803804
]);
37813805

37823806
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4365,6 +4389,9 @@ impl Methods {
43654389
}
43664390
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
43674391
},
4392+
("wake", []) => {
4393+
waker_clone_wake::check(cx, expr, recv);
4394+
}
43684395
("write", []) => {
43694396
readonly_write_lock::check(cx, expr, recv);
43704397
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_applicability;
3+
use clippy_utils::{is_trait_method, match_def_path, paths};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{Expr, ExprKind};
6+
use rustc_lint::LateContext;
7+
use rustc_span::sym;
8+
9+
use super::WAKER_CLONE_WAKE;
10+
11+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) {
12+
let ty = cx.typeck_results().expr_ty(recv);
13+
14+
if let Some(did) = ty.ty_adt_def()
15+
&& match_def_path(cx, did.did(), &paths::WAKER)
16+
&& let ExprKind::MethodCall(_, waker_ref, &[], _) = recv.kind
17+
&& is_trait_method(cx, recv, sym::Clone)
18+
{
19+
let mut applicability = Applicability::MachineApplicable;
20+
let snippet = snippet_with_applicability(cx, waker_ref.span.source_callsite(), "..", &mut applicability);
21+
22+
span_lint_and_sugg(
23+
cx,
24+
WAKER_CLONE_WAKE,
25+
expr.span,
26+
"cloning a `Waker` only to wake it",
27+
"replace with",
28+
format!("{snippet}.wake_by_ref()"),
29+
applicability,
30+
);
31+
}
32+
}

clippy_utils/src/paths.rs

+1
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];
112112
pub const INSTANT_NOW: [&str; 4] = ["std", "time", "Instant", "now"];
113113
pub const VEC_IS_EMPTY: [&str; 4] = ["alloc", "vec", "Vec", "is_empty"];
114114
pub const VEC_POP: [&str; 4] = ["alloc", "vec", "Vec", "pop"];
115+
pub const WAKER: [&str; 4] = ["core", "task", "wake", "Waker"];
115116
pub const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"];
116117
pub const OPTION_EXPECT: [&str; 4] = ["core", "option", "Option", "expect"];
117118
#[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so

tests/ui/waker_clone_wake.fixed

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#[derive(Clone)]
2+
pub struct Custom;
3+
4+
impl Custom {
5+
pub fn wake(self) {}
6+
}
7+
8+
macro_rules! mac {
9+
($cx:ident) => {
10+
$cx.waker()
11+
};
12+
}
13+
14+
pub fn wake(cx: &mut std::task::Context) {
15+
cx.waker().wake_by_ref();
16+
17+
mac!(cx).wake_by_ref();
18+
}
19+
20+
pub fn no_lint(cx: &mut std::task::Context, c: &Custom) {
21+
c.clone().wake();
22+
23+
let w = cx.waker().clone();
24+
w.wake();
25+
26+
cx.waker().clone().wake_by_ref();
27+
}
28+
29+
fn main() {}

tests/ui/waker_clone_wake.rs

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#[derive(Clone)]
2+
pub struct Custom;
3+
4+
impl Custom {
5+
pub fn wake(self) {}
6+
}
7+
8+
macro_rules! mac {
9+
($cx:ident) => {
10+
$cx.waker()
11+
};
12+
}
13+
14+
pub fn wake(cx: &mut std::task::Context) {
15+
cx.waker().clone().wake();
16+
17+
mac!(cx).clone().wake();
18+
}
19+
20+
pub fn no_lint(cx: &mut std::task::Context, c: &Custom) {
21+
c.clone().wake();
22+
23+
let w = cx.waker().clone();
24+
w.wake();
25+
26+
cx.waker().clone().wake_by_ref();
27+
}
28+
29+
fn main() {}

tests/ui/waker_clone_wake.stderr

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error: cloning a `Waker` only to wake it
2+
--> $DIR/waker_clone_wake.rs:15:5
3+
|
4+
LL | cx.waker().clone().wake();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `cx.waker().wake_by_ref()`
6+
|
7+
= note: `-D clippy::waker-clone-wake` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::waker_clone_wake)]`
9+
10+
error: cloning a `Waker` only to wake it
11+
--> $DIR/waker_clone_wake.rs:17:5
12+
|
13+
LL | mac!(cx).clone().wake();
14+
| ^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `mac!(cx).wake_by_ref()`
15+
16+
error: aborting due to 2 previous errors
17+

0 commit comments

Comments
 (0)