Skip to content

Commit

Permalink
Add a lint to check needless Waker clones
Browse files Browse the repository at this point in the history
  • Loading branch information
a1phyr committed Oct 23, 2023
1 parent 56c8235 commit aa5d7b2
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5589,6 +5589,7 @@ Released 2018-09-13
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads
[`vtable_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons
[`waker_clone_and_wake`]: https://rust-lang.github.io/rust-clippy/master/index.html#waker_clone_and_wake
[`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop
[`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::USELESS_ASREF_INFO,
crate::methods::VEC_RESIZE_TO_ZERO_INFO,
crate::methods::VERBOSE_FILE_READS_INFO,
crate::methods::WAKER_CLONE_AND_WAKE_INFO,
crate::methods::WRONG_SELF_CONVENTION_INFO,
crate::methods::ZST_OFFSET_INFO,
crate::min_ident_chars::MIN_IDENT_CHARS_INFO,
Expand Down
27 changes: 27 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ mod useless_asref;
mod utils;
mod vec_resize_to_zero;
mod verbose_file_reads;
mod waker_clone_and_wake;
mod wrong_self_convention;
mod zst_offset;

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

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `clone().wake_by_ref()`
///
/// ### Why is this bad?
/// Cloning the waker is not necessary, `wake_by_ref()` enables the same operation
/// without extra cloning/dropping.
///
/// ### Example
/// ```rust,ignore
/// waker.clone().wake();
/// ```
/// Should be written
/// ```rust,ignore
/// waker.wake_by_ref();
/// ```
#[clippy::version = "1.75.0"]
pub WAKER_CLONE_AND_WAKE,
perf,
"cloning a `Waker` only to wake it"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -3777,6 +3800,7 @@ impl_lint_pass!(Methods => [
ITER_OUT_OF_BOUNDS,
PATH_ENDS_WITH_EXT,
REDUNDANT_AS_STR,
WAKER_CLONE_AND_WAKE,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4365,6 +4389,9 @@ impl Methods {
}
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
},
("wake", []) => {
waker_clone_and_wake::check(cx, expr, recv);
}
("write", []) => {
readonly_write_lock::check(cx, expr, recv);
}
Expand Down
34 changes: 34 additions & 0 deletions clippy_lints/src/methods/waker_clone_and_wake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{match_def_path, paths};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_span::sym;

use super::WAKER_CLONE_AND_WAKE;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) {
let ty = cx.typeck_results().expr_ty(recv).peel_refs();

if let Some(did) = ty.ty_adt_def() &&
match_def_path(cx, did.did(), &paths::WAKER) &&
let ExprKind::MethodCall(func, waker_ref, &[], _) = recv.kind &&
func.ident.name == sym::clone
{
let mut applicability = Applicability::MachineApplicable;

span_lint_and_sugg(
cx,
WAKER_CLONE_AND_WAKE,
expr.span,
"cloning a `Waker` only to wake it",
"replace with",
format!(
"{}.wake_by_ref()",
snippet_with_applicability(cx, waker_ref.span, "..", &mut applicability)
),
applicability,
);
}
}
1 change: 1 addition & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];
pub const INSTANT_NOW: [&str; 4] = ["std", "time", "Instant", "now"];
pub const VEC_IS_EMPTY: [&str; 4] = ["alloc", "vec", "Vec", "is_empty"];
pub const VEC_POP: [&str; 4] = ["alloc", "vec", "Vec", "pop"];
pub const WAKER: [&str; 4] = ["core", "task", "wake", "Waker"];
pub const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"];
pub const OPTION_EXPECT: [&str; 4] = ["core", "option", "Option", "expect"];
#[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/waker_clone_and_wake.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub fn wake(cx: &mut std::task::Context) {
cx.waker().wake_by_ref()
}

fn main() {}
5 changes: 5 additions & 0 deletions tests/ui/waker_clone_and_wake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub fn wake(cx: &mut std::task::Context) {
cx.waker().clone().wake()
}

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/waker_clone_and_wake.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: cloning a `Waker` only to wake it
--> $DIR/waker_clone_and_wake.rs:2:5
|
LL | cx.waker().clone().wake()
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `cx.waker().wake_by_ref()`
|
= note: `-D clippy::waker-clone-and-wake` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::waker_clone_and_wake)]`

error: aborting due to previous error

0 comments on commit aa5d7b2

Please sign in to comment.