Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
71 changes: 71 additions & 0 deletions clippy_lints/src/manual_noop_waker.rs
Original file line number Diff line number Diff line change
@@ -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",
);
}
}
}
16 changes: 1 addition & 15 deletions clippy_lints/src/unit_types/unit_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<'_>,
Expand Down
15 changes: 15 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
2 changes: 2 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/sym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ generate! {
V6,
VecDeque,
Visitor,
Wake,
Waker,
Weak,
Wrapping,
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/manual_noop_waker.rs
Original file line number Diff line number Diff line change
@@ -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>) {}
}
}
20 changes: 20 additions & 0 deletions tests/ui/manual_noop_waker.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading