impl manual_noop_waker lint

[jaroslawroszyk]

View all comments

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

struct MyWaker;
impl Wake for MyWaker {
    fn wake(self: Arc<Self>) {}
    fn wake_by_ref(self: &Arc<Self>) {}
}

Can be replaced with:

let waker = Waker::noop();

• I've followed the contribution guidelines

• I've added UI tests for the new lint

• I've run cargo dev update_lints

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

[rustbot]

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: 7 candidates
• 7 candidates expanded to 7 candidates
• Random selection from Jarcho, dswij, llogiq, samueltardieu

[samueltardieu]

This will falsely trigger the lint:

trait Wake {
    fn wake(self);
}

impl Wake for () {
    fn wake(self) {}
}

See the Clippy book for good practices. Sorry if this is not the case, but the lint looks vibe-coded by a LLM which is not very efficient.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[jaroslawroszyk]

This will falsely trigger the lint:

trait Wake {
    fn wake(self);
}

impl Wake for () {
    fn wake(self) {}
}

See the Clippy book for good practices. Sorry if this is not the case, but the lint looks vibe-coded by a LLM which is not very efficient.

View changes since this review

It's not vibe coded. I followed example and on that I'll do implementation. It's my first pr in this repo.

[jaroslawroszyk]

@rustbot ready

[samueltardieu]

You haven't addressed the false positives (see #16687 (review))

View changes since this review

[jaroslawroszyk]

You haven't addressed the false positives (see #16687 (review))

View changes since this review

Did you see?
tests/ui/manual_noop_waker.rs
and stderr of that?

[jaroslawroszyk]

@rustbot ready

[jaroslawroszyk]

@rustbot ready

[samueltardieu]

We're getting to the final touches.

View changes since this review

[jaroslawroszyk]

@rustbot ready

[samueltardieu]

Thanks.