Add unused_async_trait_impl lint

CHANGELOG.md

@@ -7091,6 +7091,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

clippy_lints/src/declared_lints.rs

@@ -770,6 +770,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,

clippy_lints/src/lib.rs

@@ -379,6 +379,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;
@@ -861,6 +862,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
         Box::new(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)),
         Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, 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);

clippy_lints/src/unused_async_trait_impl.rs

@@ -0,0 +1,198 @@
+use clippy_utils::diagnostics::span_lint_hir_and_then;
+use clippy_utils::source::{
+    HasSession, indent_of, reindent_multiline, snippet_block_with_applicability, snippet_with_applicability,
+};
+use clippy_utils::usage::is_todo_unimplemented_stub;
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::{Visitor, walk_expr};
+use rustc_hir::{
+    Body, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, Expr, ExprKind, ImplItem, ImplItemKind, IsAsync,
+    YieldSource,
+};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::hir::nested_filter;
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for trait method 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 method to return a future,
+    /// returning a `core::future::ready` with the result is more efficient
+    /// as it reduces the number of states in the Future state machine by at least one.
+    ///
+    /// Note that the behaviour is slightly different when using `core::future::ready`,
+    /// as the value is computed immediately and stored in a future for later retrieval at the first (and only valid) call to `poll`.
+    /// An `async` block generates code that completely defers the computation of this value until the Future is polled.
+    ///
+    /// ### 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<Output = i64> {
+    ///         core::future::ready(4) // Chosen by fair dice roll. Guaranteed to be random.
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.94.0"]
+    pub UNUSED_ASYNC_TRAIT_IMPL,
+    pedantic,
+    "finds async trait impl functions with no await statements"
+}
+
+declare_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_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
+        if let ImplItemKind::Fn(ref sig, body_id) = impl_item.kind
+            && let IsAsync::Async(async_span) = sig.header.asyncness
+            && let body = cx.tcx.hir_body(body_id)
+            && !async_fn_contains_todo_unimplemented_macro(cx, body)
+        {
+            let mut visitor = AsyncFnVisitor {
+                cx,
+                found_await: false,
+                async_depth: 0,
+            };
+            visitor.visit_nested_body(body_id);
+
+            if !visitor.found_await
+                && let Some(builtin_crate) = clippy_utils::std_or_core(cx)
+            {
+                span_lint_hir_and_then(
+                    cx,
+                    UNUSED_ASYNC_TRAIT_IMPL,
+                    cx.tcx.local_def_id_to_hir_id(impl_item.owner_id.def_id),
+                    impl_item.span,
+                    "unused `async` for async trait impl function with no await statements",
+                    |diag| {
+                        let mut app = Applicability::MachineApplicable;
+
+                        let async_span = cx.sess().source_map().span_extend_while_whitespace(async_span);
+
+                        let signature_snippet = snippet_with_applicability(cx, sig.decl.output.span(), "_", &mut app);
+
+                        // Fetch body snippet and truncate excess indentation. Like this:
+                        // {
+                        //     4
+                        // }
+                        let body_snippet = snippet_block_with_applicability(cx, body.value.span, "_", None, &mut app);
+                        // Wrap body snippet in `std::future::ready(...)` and indent everything by one level, like this:
+                        //     core::future::ready({
+                        //         4
+                        //     })
+                        let new_body_inner_snippet: String = reindent_multiline(
+                            &format!("{builtin_crate}::future::ready({body_snippet})"),
+                            false,
+                            Some(4),
+                        );
+
+                        let sugg = vec![
+                            (async_span, String::new()),
+                            (
+                                sig.decl.output.span(),
+                                format!("impl Future<Output = {signature_snippet}>"),
+                            ),
+                            (
+                                body.value.span,
+                                // Wrap the entire snippet in fresh curly braces and indent everything except the first
+                                // line by the indentation level of the original body snippet, like this:
+                                // {
+                                // <indent>     core::future::ready({
+                                // <indent>         4
+                                // <indent>     }
+                                // <indent> }
+                                reindent_multiline(
+                                    &format!("{{\n{new_body_inner_snippet}\n}}"),
+                                    true,
+                                    indent_of(cx, body.value.span),
+                                ),
+                            ),
+                        ];
+                        diag.help(format!(
+                            "a Future can be constructed from the return value with `{builtin_crate}::future::ready`"
+                        ));
+                        diag.multipart_suggestion(
+                                    format!("consider removing the `async` from this function and returning `impl Future<Output = {signature_snippet}>` instead"),
+                                    sugg,
+                                    app
+                                );
+                    },
+                );
+            }
+        }
+    }
+}
+
+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
+}

tests/ui/unused_async_trait_impl.fixed

@@ -0,0 +1,81 @@
+#![warn(clippy::unused_async_trait_impl)]
+
+trait HasAsyncMethod {
+    async fn do_something() -> u32;
+}
+
+struct Inefficient;
+struct Efficient;
+struct Stub;
+
+impl HasAsyncMethod for Inefficient {
+    fn do_something() -> impl Future<Output = u32> {
+        std::future::ready({
+            //~^ unused_async_trait_impl
+            1
+        })
+    }
+}
+
+impl HasAsyncMethod for Efficient {
+    fn do_something() -> impl Future<Output = u32> {
+        std::future::ready(1)
+    }
+}
+
+impl HasAsyncMethod for Stub {
+    async fn do_something() -> u32 {
+        todo!() // Do not emit the lint in this case.
+    }
+}
+
+// Test to check if the identation of the various snippets goes as intended.
+mod indented {
+    struct Indented;
+
+    impl crate::HasAsyncMethod for Indented {
+        fn do_something() -> impl Future<Output = u32> {
+            std::future::ready({
+                //~^ unused_async_trait_impl
+                let mut x = 0;
+                for y in 0..64 {
+                    x = (x + 1) * y;
+                }
+
+                let fake_fut = async {
+                    if x == 0 {
+                        panic!("Fake example");
+                    }
+                };
+
+                x
+            })
+        }
+    }
+
+    struct Complex<T>(std::marker::PhantomData<T>);
+
+    impl<T> crate::HasAsyncMethod for Complex<T>
+    where
+        T: Sized,
+    {
+        fn do_something() -> impl Future<Output = u32> {
+            std::future::ready({
+                //~^ unused_async_trait_impl
+                5
+            })
+        }
+    }
+}
+
+trait HasDefaultAsyncMethod {
+    // The lint should not suggest a change for trait fn's as changing that decl
+    // implies a less restrictive Future type.
+    async fn do_something() -> u32 {
+        0
+    }
+}
+
+impl HasDefaultAsyncMethod for Stub {
+    // Nothing!
+}

tests/ui/unused_async_trait_impl.rs

@@ -0,0 +1,75 @@
+#![warn(clippy::unused_async_trait_impl)]
+
+trait HasAsyncMethod {
+    async fn do_something() -> u32;
+}
+
+struct Inefficient;
+struct Efficient;
+struct Stub;
+
+impl HasAsyncMethod for Inefficient {
+    async fn do_something() -> u32 {
+        //~^ unused_async_trait_impl
+        1
+    }
+}
+
+impl HasAsyncMethod for Efficient {
+    fn do_something() -> impl Future<Output = u32> {
+        std::future::ready(1)
+    }
+}
+
+impl HasAsyncMethod for Stub {
+    async fn do_something() -> u32 {
+        todo!() // Do not emit the lint in this case.
+    }
+}
+
+// Test to check if the identation of the various snippets goes as intended.
+mod indented {
+    struct Indented;
+
+    impl crate::HasAsyncMethod for Indented {
+        async fn do_something() -> u32 {
+            //~^ unused_async_trait_impl
+            let mut x = 0;
+            for y in 0..64 {
+                x = (x + 1) * y;
+            }
+
+            let fake_fut = async {
+                if x == 0 {
+                    panic!("Fake example");
+                }
+            };
+
+            x
+        }
+    }
+
+    struct Complex<T>(std::marker::PhantomData<T>);
+
+    impl<T> crate::HasAsyncMethod for Complex<T>
+    where
+        T: Sized,
+    {
+        async fn do_something() -> u32 {
+            //~^ unused_async_trait_impl
+            5
+        }
+    }
+}
+
+trait HasDefaultAsyncMethod {
+    // The lint should not suggest a change for trait fn's as changing that decl
+    // implies a less restrictive Future type.
+    async fn do_something() -> u32 {
+        0
+    }
+}
+
+impl HasDefaultAsyncMethod for Stub {
+    // Nothing!
+}