diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f666caf306f..678d95542c8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7089,6 +7089,7 @@ Released 2018-09-13 [`unstable_as_mut_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_mut_slice [`unstable_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_slice [`unused_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_async +[`unused_async_trait_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_async_trait_impl [`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect [`unused_enumerate_index`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_enumerate_index [`unused_format_specs`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_format_specs diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 87d75234ebc0..c67db2ef7389 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -768,6 +768,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO, crate::unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME_INFO, crate::unused_async::UNUSED_ASYNC_INFO, + crate::unused_async_trait_impl::UNUSED_ASYNC_TRAIT_IMPL_INFO, crate::unused_io_amount::UNUSED_IO_AMOUNT_INFO, crate::unused_peekable::UNUSED_PEEKABLE_INFO, crate::unused_result_ok::UNUSED_RESULT_OK_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 40487fe48f22..80cfa5da6487 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -375,6 +375,7 @@ mod unneeded_struct_pattern; mod unnested_or_patterns; mod unsafe_removed_from_name; mod unused_async; +mod unused_async_trait_impl; mod unused_io_amount; mod unused_peekable; mod unused_result_ok; @@ -852,6 +853,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(|_| Box::new(volatile_composites::VolatileComposites)), Box::new(|_| Box::::default()), Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))), + Box::new(|_| Box::new(unused_async_trait_impl::UnusedAsyncTraitImpl)), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/clippy_lints/src/unused_async_trait_impl.rs b/clippy_lints/src/unused_async_trait_impl.rs new file mode 100644 index 000000000000..f4263bd46ee2 --- /dev/null +++ b/clippy_lints/src/unused_async_trait_impl.rs @@ -0,0 +1,184 @@ +use clippy_utils::diagnostics::span_lint_hir_and_then; +use clippy_utils::is_def_id_trait_method; +use clippy_utils::source::SpanRangeExt; +use clippy_utils::usage::is_todo_unimplemented_stub; +use rustc_errors::Applicability; +use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn}; +use rustc_hir::{ + Body, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, Defaultness, Expr, ExprKind, FnDecl, IsAsync, Node, + TraitItem, YieldSource, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::nested_filter; +use rustc_session::impl_lint_pass; +use rustc_span::Span; +use rustc_span::def_id::LocalDefId; + +declare_clippy_lint! { + /// ### What it does + /// Checks for trait function implementations that are declared `async` but have no `.await`s inside of them. + /// + /// ### Why is this bad? + /// Async functions with no async code create computational overhead. + /// Even though the trait requires the function to return a future, + /// returning a `core::future::ready` with the result is more efficient. + /// + /// ### Example + /// ```no_run + /// trait AsyncTrait { + /// async fn get_random_number() -> i64; + /// } + /// + /// impl AsyncTrait for () { + /// async fn get_random_number() -> i64 { + /// 4 // Chosen by fair dice roll. Guaranteed to be random. + /// } + /// } + /// ``` + /// + /// Use instead: + /// ```no_run + /// trait AsyncTrait { + /// async fn get_random_number() -> i64; + /// } + /// + /// impl AsyncTrait for () { + /// fn get_random_number() -> impl Future { + /// core::future::ready(4) // Chosen by fair dice roll. Guaranteed to be random. + /// } + /// } + /// ``` + /// + /// ### Note + /// An `async` block generates code that defers execution until the Future is polled. + /// When using `core::future::ready` the code is executed immediately. + #[clippy::version = "1.94.0"] + pub UNUSED_ASYNC_TRAIT_IMPL, + pedantic, + "finds async trait impl functions with no await statements" +} + +pub struct UnusedAsyncTraitImpl; + +impl_lint_pass!(UnusedAsyncTraitImpl => [UNUSED_ASYNC_TRAIT_IMPL]); + +struct AsyncFnVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + found_await: bool, + async_depth: usize, +} + +impl<'tcx> Visitor<'tcx> for AsyncFnVisitor<'_, 'tcx> { + type NestedFilter = nested_filter::OnlyBodies; + + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { + if let ExprKind::Yield(_, YieldSource::Await { .. }) = ex.kind + && self.async_depth == 1 + { + self.found_await = true; + } + + let is_async_block = matches!( + ex.kind, + ExprKind::Closure(Closure { + kind: ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)), + .. + }) + ); + + if is_async_block { + self.async_depth += 1; + } + + walk_expr(self, ex); + + if is_async_block { + self.async_depth -= 1; + } + } + + fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt { + self.cx.tcx + } +} + +impl<'tcx> LateLintPass<'tcx> for UnusedAsyncTraitImpl { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + fn_kind: FnKind<'tcx>, + fn_decl: &'tcx FnDecl<'tcx>, + body: &Body<'tcx>, + span: Span, + def_id: LocalDefId, + ) { + if let IsAsync::Async(async_span) = fn_kind.asyncness() + && !span.from_expansion() + && is_def_id_trait_method(cx, def_id) + && !is_default_trait_impl(cx, def_id) + && !async_fn_contains_todo_unimplemented_macro(cx, body) + { + let mut visitor = AsyncFnVisitor { + cx, + found_await: false, + async_depth: 0, + }; + walk_fn(&mut visitor, fn_kind, fn_decl, body.id(), def_id); + if !visitor.found_await { + span_lint_hir_and_then( + cx, + UNUSED_ASYNC_TRAIT_IMPL, + cx.tcx.local_def_id_to_hir_id(def_id), + span, + "unused `async` for async trait impl function with no await statements", + |diag| { + if let Some(output_src) = fn_decl.output.span().get_source_text(cx) + && let Some(body_src) = body.value.span.get_source_text(cx) + { + let output_str = output_src.as_str(); + let body_str = body_src.as_str(); + + let sugg = vec![ + (async_span, String::new()), + (fn_decl.output.span(), format!("impl Future")), + (body.value.span, format!("{{ core::future::ready({body_str}) }}")), + ]; + + diag.help("a Future can be constructed from the return value with `core::future::ready`"); + diag.multipart_suggestion( + format!("consider removing the `async` from this function and returning `impl Future` instead"), + sugg, + Applicability::MachineApplicable + ); + } + }, + ); + } + } + } +} + +fn is_default_trait_impl(cx: &LateContext<'_>, def_id: LocalDefId) -> bool { + matches!( + cx.tcx.hir_node_by_def_id(def_id), + Node::TraitItem(TraitItem { + defaultness: Defaultness::Default { .. }, + .. + }) + ) +} + +fn async_fn_contains_todo_unimplemented_macro(cx: &LateContext<'_>, body: &Body<'_>) -> bool { + if let ExprKind::Closure(closure) = body.value.kind + && let ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)) = closure.kind + && let body = cx.tcx.hir_body(closure.body) + && let ExprKind::Block(block, _) = body.value.kind + && block.stmts.is_empty() + && let Some(expr) = block.expr + && let ExprKind::DropTemps(inner) = expr.kind + { + return is_todo_unimplemented_stub(cx, inner); + } + + false +} diff --git a/tests/ui/unused_async_trait_impl.fixed b/tests/ui/unused_async_trait_impl.fixed new file mode 100644 index 000000000000..ea6ba1e57562 --- /dev/null +++ b/tests/ui/unused_async_trait_impl.fixed @@ -0,0 +1,26 @@ +#![warn(clippy::unused_async_trait_impl)] + +trait HasAsyncMethod { + async fn do_something() -> u32; +} + +struct Inefficient; +struct Efficient; + +impl HasAsyncMethod for Inefficient { + fn do_something() -> impl Future { core::future::ready({ + //~^ unused_async_trait_impl + 1 + }) } +} + +impl HasAsyncMethod for Efficient { + fn do_something() -> impl Future { + core::future::ready(1) + } +} + +fn main() { + Inefficient::do_something(); + Efficient::do_something(); +} diff --git a/tests/ui/unused_async_trait_impl.rs b/tests/ui/unused_async_trait_impl.rs new file mode 100644 index 000000000000..4d476778f777 --- /dev/null +++ b/tests/ui/unused_async_trait_impl.rs @@ -0,0 +1,26 @@ +#![warn(clippy::unused_async_trait_impl)] + +trait HasAsyncMethod { + async fn do_something() -> u32; +} + +struct Inefficient; +struct Efficient; + +impl HasAsyncMethod for Inefficient { + async fn do_something() -> u32 { + //~^ unused_async_trait_impl + 1 + } +} + +impl HasAsyncMethod for Efficient { + fn do_something() -> impl Future { + core::future::ready(1) + } +} + +fn main() { + Inefficient::do_something(); + Efficient::do_something(); +} diff --git a/tests/ui/unused_async_trait_impl.stderr b/tests/ui/unused_async_trait_impl.stderr new file mode 100644 index 000000000000..a20241d2f4a4 --- /dev/null +++ b/tests/ui/unused_async_trait_impl.stderr @@ -0,0 +1,22 @@ +error: unused `async` for async trait impl function with no await statements + --> tests/ui/unused_async_trait_impl.rs:11:5 + | +LL | / async fn do_something() -> u32 { +LL | | +LL | | 1 +LL | | } + | |_____^ + | + = help: a Future can be constructed from the return value with `core::future::ready` + = note: `-D clippy::unused-async-trait-impl` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unused_async_trait_impl)]` +help: consider removing the `async` from this function and returning `impl Future` instead + | +LL ~ fn do_something() -> impl Future { core::future::ready({ +LL + +LL + 1 +LL + }) } + | + +error: aborting due to 1 previous error +