Add unused_async_trait_impl lint

CHANGELOG.md

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

@@ -782,6 +782,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::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/unused_async.rs

@@ -1,11 +1,15 @@
-use clippy_utils::diagnostics::span_lint_hir_and_then;
+use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
 use clippy_utils::is_def_id_trait_method;
+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::def::DefKind;
 use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn};
 use rustc_hir::{
-    Body, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, Defaultness, Expr, ExprKind, FnDecl, HirId, Node,
-    TraitItem, YieldSource,
+    BlockCheckMode, Body, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, Defaultness, Expr, ExprKind,
+    FnDecl, HirId, ImplItem, ImplItemKind, IsAsync, Node, TraitItem, YieldSource,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter;
@@ -43,7 +47,52 @@ declare_clippy_lint! {
     "finds async functions with no await statements"
 }
 
-impl_lint_pass!(UnusedAsync => [UNUSED_ASYNC]);
+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.97.0"]
+    pub UNUSED_ASYNC_TRAIT_IMPL,
+    pedantic,
+    "finds async trait impl functions with no await statements"
+}
+
+impl_lint_pass!(UnusedAsync => [UNUSED_ASYNC, UNUSED_ASYNC_TRAIT_IMPL]);
 
 #[derive(Default)]
 pub struct UnusedAsync {
@@ -194,6 +243,105 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
             );
         }
     }
+
+    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,
+                await_in_async_block: None,
+                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_and_then(
+                    cx,
+                    UNUSED_ASYNC_TRAIT_IMPL,
+                    impl_item.span,
+                    "unused `async` for async trait impl function with no `.await` statements",
+                    |diag| {
+                        // The suggestion might be incorrect. The future changes from awaiting for the first poll to
+                        // evaluate the expression, to immediately evaluate the expression.
+                        let mut app = Applicability::MaybeIncorrect;
+
+                        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);
+
+                        let body_snippet = 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
+                            && let Some(expr) = block.expr
+                            && let ExprKind::DropTemps(inner) = expr.kind
+                            && let ExprKind::Block(block, _) = inner.kind
+                            && block.stmts.is_empty()
+                            && block.rules == BlockCheckMode::DefaultBlock
+                        {
+                            // Async body is just a simple expression: strip any curly braces.
+                            strip_block_braces(&snippet_with_applicability(cx, body.value.span, "_", &mut app))
+                                .to_string()
+                        } else {
+                            // Fetch body snippet and truncate excess indentation. Like this:
+                            // {
+                            //     4
+                            // }
+                            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!(
+                            "`{builtin_crate}::future::ready` creates a `Future` which returns the value immediately when `poll`ed"
+                        ));
+                        diag.multipart_suggestion(
+                                    format!("consider removing the `async` from this function and returning `impl Future<Output = {signature_snippet}>` instead"),
+                                    sugg,
+                                    app
+                                );
+                    },
+                );
+            }
+        }
+    }
 }
 
 fn is_default_trait_impl(cx: &LateContext<'_>, def_id: LocalDefId) -> bool {
@@ -219,3 +367,12 @@ fn async_fn_contains_todo_unimplemented_macro(cx: &LateContext<'_>, body: &Body<
 
     false
 }
+
+fn strip_block_braces(block: &str) -> &str {
+    block
+        .trim()
+        .strip_prefix("{")
+        .and_then(|block| block.strip_suffix("}"))
+        .unwrap_or(block)
+        .trim()
+}

tests/ui/unused_async_trait_impl.fixed

@@ -0,0 +1,77 @@
+#![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!
+}