about summary refs log tree commit diff
diff options
context:
space:
mode:
authorBenoît du Garreau <bdgdlm@outlook.com>2023-10-23 14:41:10 +0200
committerBenoît du Garreau <bdgdlm@outlook.com>2023-10-24 09:58:23 +0200
commitf8790963d9dd9fd4fd22456eef1cbfe2fb8cb12a (patch)
treec857981eb44bef2510359fcf6d3b5b277f775df0
parent56c8235c99d0138d839b009319416732218dd7ca (diff)
downloadrust-f8790963d9dd9fd4fd22456eef1cbfe2fb8cb12a.tar.gz
rust-f8790963d9dd9fd4fd22456eef1cbfe2fb8cb12a.zip
Add a lint to check needless `Waker` clones
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/methods/mod.rs27
-rw-r--r--clippy_lints/src/methods/waker_clone_wake.rs34
-rw-r--r--clippy_utils/src/paths.rs1
-rw-r--r--tests/ui/waker_clone_wake.fixed22
-rw-r--r--tests/ui/waker_clone_wake.rs22
-rw-r--r--tests/ui/waker_clone_wake.stderr11
8 files changed, 119 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 26c9877dbff..b28075d8eb3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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_wake`]: https://rust-lang.github.io/rust-clippy/master/index.html#waker_clone_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
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 77438b27f90..9de4a8598b4 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -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_WAKE_INFO,
     crate::methods::WRONG_SELF_CONVENTION_INFO,
     crate::methods::ZST_OFFSET_INFO,
     crate::min_ident_chars::MIN_IDENT_CHARS_INFO,
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index a935aea5075..9c8d2711c3d 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -112,6 +112,7 @@ mod useless_asref;
 mod utils;
 mod vec_resize_to_zero;
 mod verbose_file_reads;
+mod waker_clone_wake;
 mod wrong_self_convention;
 mod zst_offset;
 
@@ -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 `waker.clone().wake()`
+    ///
+    /// ### 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_WAKE,
+    perf,
+    "cloning a `Waker` only to wake it"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3777,6 +3800,7 @@ impl_lint_pass!(Methods => [
     ITER_OUT_OF_BOUNDS,
     PATH_ENDS_WITH_EXT,
     REDUNDANT_AS_STR,
+    WAKER_CLONE_WAKE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4365,6 +4389,9 @@ impl Methods {
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
                 },
+                ("wake", []) => {
+                    waker_clone_wake::check(cx, expr, recv);
+                }
                 ("write", []) => {
                     readonly_write_lock::check(cx, expr, recv);
                 }
diff --git a/clippy_lints/src/methods/waker_clone_wake.rs b/clippy_lints/src/methods/waker_clone_wake.rs
new file mode 100644
index 00000000000..db13266db80
--- /dev/null
+++ b/clippy_lints/src/methods/waker_clone_wake.rs
@@ -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_WAKE;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) {
+    let ty = cx.typeck_results().expr_ty(recv);
+
+    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_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,
+        );
+    }
+}
diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs
index 4a20399e364..2cf7e2cde96 100644
--- a/clippy_utils/src/paths.rs
+++ b/clippy_utils/src/paths.rs
@@ -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
diff --git a/tests/ui/waker_clone_wake.fixed b/tests/ui/waker_clone_wake.fixed
new file mode 100644
index 00000000000..2df52f57d65
--- /dev/null
+++ b/tests/ui/waker_clone_wake.fixed
@@ -0,0 +1,22 @@
+#[derive(Clone)]
+pub struct Custom;
+
+impl Custom {
+    pub fn wake(self) {}
+}
+
+pub fn wake(cx: &mut std::task::Context) {
+    cx.waker().wake_by_ref();
+
+    // We don't do that for now
+    let w = cx.waker().clone();
+    w.wake();
+
+    cx.waker().clone().wake_by_ref();
+}
+
+pub fn no_lint(c: &Custom) {
+    c.clone().wake()
+}
+
+fn main() {}
diff --git a/tests/ui/waker_clone_wake.rs b/tests/ui/waker_clone_wake.rs
new file mode 100644
index 00000000000..4fe354b0ef1
--- /dev/null
+++ b/tests/ui/waker_clone_wake.rs
@@ -0,0 +1,22 @@
+#[derive(Clone)]
+pub struct Custom;
+
+impl Custom {
+    pub fn wake(self) {}
+}
+
+pub fn wake(cx: &mut std::task::Context) {
+    cx.waker().clone().wake();
+
+    // We don't do that for now
+    let w = cx.waker().clone();
+    w.wake();
+
+    cx.waker().clone().wake_by_ref();
+}
+
+pub fn no_lint(c: &Custom) {
+    c.clone().wake()
+}
+
+fn main() {}
diff --git a/tests/ui/waker_clone_wake.stderr b/tests/ui/waker_clone_wake.stderr
new file mode 100644
index 00000000000..426a577e620
--- /dev/null
+++ b/tests/ui/waker_clone_wake.stderr
@@ -0,0 +1,11 @@
+error: cloning a `Waker` only to wake it
+  --> $DIR/waker_clone_wake.rs:9:5
+   |
+LL |     cx.waker().clone().wake();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `cx.waker().wake_by_ref()`
+   |
+   = note: `-D clippy::waker-clone-wake` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::waker_clone_wake)]`
+
+error: aborting due to previous error
+