impl manual_noop_waker lint (#16687)

*[View all
comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16687)*

Fixes #16639

### Description

This PR introduces a new lint manual\_noop\_waker that detects manual,
empty implementations of the std::task::Wake trait. These can be
replaced with std::task::Waker::noop(), which is more performant (avoids
Arc allocation) and cleaner.

### Example

```rust
struct MyWaker;
impl Wake for MyWaker {
    fn wake(self: Arc<Self>) {}
    fn wake_by_ref(self: &Arc<Self>) {}
}
````
Can be replaced with:

```rust
let waker = Waker::noop();

````

* \[x\] I've followed the [contribution
guidelines](https://github.com/rust-lang/rust-clippy/blob/master/CONTRIBUTING.md)

*   \[x\] I've added UI tests for the new lint

*   \[x\] I've run `cargo dev update_lints`

changelog: `[manual_noop_waker]`: Added a lint to detect manual no-op
Wake implementations.

Author: samueltardieu

CHANGELOG.md

@@ -6797,6 +6797,7 @@ Released 2018-09-13
 [`manual_midpoint`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_midpoint
 [`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
+[`manual_noop_waker`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_noop_waker
 [`manual_ok_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_err
 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
 [`manual_option_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_as_slice

clippy_lints/src/declared_lints.rs

@@ -311,6 +311,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
+    crate::manual_noop_waker::MANUAL_NOOP_WAKER_INFO,
     crate::manual_option_as_slice::MANUAL_OPTION_AS_SLICE_INFO,
     crate::manual_pop_if::MANUAL_POP_IF_INFO,
     crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO,

clippy_lints/src/lib.rs

@@ -209,6 +209,7 @@ mod manual_is_power_of_two;
 mod manual_let_else;
 mod manual_main_separator_str;
 mod manual_non_exhaustive;
+mod manual_noop_waker;
 mod manual_option_as_slice;
 mod manual_pop_if;
 mod manual_range_patterns;
@@ -865,6 +866,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))),
         Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)),
         Box::new(move |tcx| Box::new(manual_pop_if::ManualPopIf::new(tcx, conf))),
+        Box::new(|_| Box::new(manual_noop_waker::ManualNoopWaker)),
         // add late passes here, used by `cargo dev new_lint`
     ];
     store.late_passes.extend(late_lints);

clippy_lints/src/manual_noop_waker.rs

@@ -0,0 +1,71 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::{is_empty_block, sym};
+use rustc_hir::{ImplItemKind, Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual implementations of `std::task::Wake` that are empty.
+    ///
+    /// ### Why is this bad?
+    /// `Waker::noop()` provides a more performant and cleaner way to create a
+    /// waker that does nothing, avoiding unnecessary `Arc` allocations and
+    /// reference count increments.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # use std::sync::Arc;
+    /// # use std::task::Wake;
+    /// struct MyWaker;
+    /// impl Wake for MyWaker {
+    ///     fn wake(self: Arc<Self>) {}
+    ///     fn wake_by_ref(self: &Arc<Self>) {}
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// use std::task::Waker;
+    /// let waker = Waker::noop();
+    /// ```
+    #[clippy::version = "1.96.0"]
+    pub MANUAL_NOOP_WAKER,
+    complexity,
+    "manual implementations of noop wakers can be simplified using Waker::noop()"
+}
+
+declare_lint_pass!(ManualNoopWaker => [MANUAL_NOOP_WAKER]);
+
+impl<'tcx> LateLintPass<'tcx> for ManualNoopWaker {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if let ItemKind::Impl(imp) = item.kind
+            && let Some(trait_ref) = imp.of_trait
+            && let Some(trait_id) = trait_ref.trait_ref.trait_def_id()
+            && cx.tcx.is_diagnostic_item(sym::Wake, trait_id)
+        {
+            for impl_item_ref in imp.items {
+                let impl_item = cx
+                    .tcx
+                    .hir_node_by_def_id(impl_item_ref.owner_id.def_id)
+                    .expect_impl_item();
+
+                if let ImplItemKind::Fn(_, body_id) = &impl_item.kind {
+                    let body = cx.tcx.hir_body(*body_id);
+                    if !is_empty_block(body.value) {
+                        return;
+                    }
+                }
+            }
+
+            span_lint_and_help(
+                cx,
+                MANUAL_NOOP_WAKER,
+                trait_ref.trait_ref.path.span,
+                "manual implementation of a no-op waker",
+                None,
+                "use `std::task::Waker::noop()` instead",
+            );
+        }
+    }
+}

clippy_lints/src/unit_types/unit_arg.rs

@@ -4,7 +4,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::{SpanRangeExt, indent_of, reindent_multiline};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::expr_type_is_certain;
-use clippy_utils::{is_expr_default, is_from_proc_macro};
+use clippy_utils::{is_empty_block, is_expr_default, is_from_proc_macro};
 use rustc_errors::Applicability;
 use rustc_hir::{Block, Expr, ExprKind, MatchSource, Node, StmtKind};
 use rustc_lint::LateContext;
@@ -205,20 +205,6 @@ fn is_block_with_no_expr(expr: &Expr<'_>) -> bool {
     matches!(expr.kind, ExprKind::Block(Block { expr: None, .. }, _))
 }
 
-fn is_empty_block(expr: &Expr<'_>) -> bool {
-    matches!(
-        expr.kind,
-        ExprKind::Block(
-            Block {
-                stmts: [],
-                expr: None,
-                ..
-            },
-            _,
-        )
-    )
-}
-
 fn fmt_stmts_and_call(
     cx: &LateContext<'_>,
     call_expr: &Expr<'_>,

clippy_utils/src/lib.rs

@@ -311,6 +311,21 @@ pub fn as_some_expr<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Optio
     }
 }
 
+/// Check if the given `Expr` is an empty block (i.e. `{}`) or not.
+pub fn is_empty_block(expr: &Expr<'_>) -> bool {
+    matches!(
+        expr.kind,
+        ExprKind::Block(
+            Block {
+                stmts: [],
+                expr: None,
+                ..
+            },
+            _,
+        )
+    )
+}
+
 /// Checks if `expr` is an empty block or an empty tuple.
 pub fn is_unit_expr(expr: &Expr<'_>) -> bool {
     matches!(

clippy_utils/src/paths.rs

@@ -133,6 +133,8 @@ path_macros! {
     macro_path: PathNS::Macro,
 }
 
+// Paths in the standard library missing a diagnostic item
+
 // Paths in external crates
 pub static FUTURES_IO_ASYNCREADEXT: PathLookup = type_path!(futures_util::AsyncReadExt);
 pub static FUTURES_IO_ASYNCWRITEEXT: PathLookup = type_path!(futures_util::AsyncWriteExt);

clippy_utils/src/sym.rs

@@ -122,6 +122,7 @@ generate! {
     V6,
     VecDeque,
     Visitor,
+    Wake,
     Waker,
     Weak,
     Wrapping,

tests/ui/manual_noop_waker.rs

@@ -0,0 +1,40 @@
+#![warn(clippy::manual_noop_waker)]
+use std::sync::Arc;
+use std::task::Wake;
+
+struct PartialWaker;
+impl Wake for PartialWaker {
+    //~^ ERROR: manual implementation of a no-op waker
+    fn wake(self: Arc<Self>) {}
+}
+
+struct MyWakerPartial;
+impl Wake for MyWakerPartial {
+    //~^ manual_noop_waker
+    fn wake(self: Arc<Self>) {}
+    // wake_by_ref not implemented, uses default
+}
+
+trait CustomWake {
+    fn wake(self);
+}
+
+impl CustomWake for () {
+    fn wake(self) {}
+}
+
+mod custom_module {
+    use std::sync::Arc;
+
+    // Custom Wake trait that should NOT trigger the lint
+    pub trait Wake {
+        fn wake(self: Arc<Self>);
+        fn wake_by_ref(self: &Arc<Self>);
+    }
+
+    pub struct CustomWaker;
+    impl Wake for CustomWaker {
+        fn wake(self: Arc<Self>) {}
+        fn wake_by_ref(self: &Arc<Self>) {}
+    }
+}

tests/ui/manual_noop_waker.stderr

@@ -0,0 +1,20 @@
+error: manual implementation of a no-op waker
+  --> tests/ui/manual_noop_waker.rs:6:6
+   |
+LL | impl Wake for PartialWaker {
+   |      ^^^^
+   |
+   = help: use `std::task::Waker::noop()` instead
+   = note: `-D clippy::manual-noop-waker` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_noop_waker)]`
+
+error: manual implementation of a no-op waker
+  --> tests/ui/manual_noop_waker.rs:12:6
+   |
+LL | impl Wake for MyWakerPartial {
+   |      ^^^^
+   |
+   = help: use `std::task::Waker::noop()` instead
+
+error: aborting due to 2 previous errors
+