From 37b83660bc2501f6e90cb48689304aed73229967 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Fri, 21 Jul 2023 00:50:01 +0200 Subject: [PATCH 1/2] [`unused_async`]: don't lint if paths reference async fn without call --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/unused_async.rs | 103 +++++++++++++++++++++++++------ tests/ui/unused_async.rs | 17 +++++ tests/ui/unused_async.stderr | 34 ++++++---- 4 files changed, 123 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1609eb396b43..e6f6212508ef 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -911,7 +911,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move |_| Box::new(if_then_some_else_none::IfThenSomeElseNone::new(msrv()))); store.register_late_pass(|_| Box::new(bool_assert_comparison::BoolAssertComparison)); store.register_early_pass(move || Box::new(module_style::ModStyle)); - store.register_late_pass(|_| Box::new(unused_async::UnusedAsync)); + store.register_late_pass(|_| Box::::default()); let disallowed_types = conf.disallowed_types.clone(); store.register_late_pass(move |_| Box::new(disallowed_types::DisallowedTypes::new(disallowed_types.clone()))); let import_renames = conf.enforced_import_renames.clone(); diff --git a/clippy_lints/src/unused_async.rs b/clippy_lints/src/unused_async.rs index 5e42cf7e4f3f..5c8c6e589141 100644 --- a/clippy_lints/src/unused_async.rs +++ b/clippy_lints/src/unused_async.rs @@ -1,11 +1,13 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_def_id_trait_method; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir::def::DefKind; use rustc_hir::intravisit::{walk_body, walk_expr, walk_fn, FnKind, Visitor}; -use rustc_hir::{Body, Expr, ExprKind, FnDecl, YieldSource}; +use rustc_hir::{Body, Expr, ExprKind, FnDecl, Node, YieldSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::def_id::LocalDefId; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::def_id::{LocalDefId, LocalDefIdSet}; use rustc_span::Span; declare_clippy_lint! { @@ -38,7 +40,23 @@ declare_clippy_lint! { "finds async functions with no await statements" } -declare_lint_pass!(UnusedAsync => [UNUSED_ASYNC]); +#[derive(Default)] +pub struct UnusedAsync { + /// Keeps track of async functions used as values (i.e. path expressions to async functions that + /// are not immediately called) + async_fns_as_value: LocalDefIdSet, + /// Functions with unused `async`, linted post-crate after we've found all uses of local async + /// functions + unused_async_fns: FxHashMap, +} + +#[derive(Copy, Clone)] +struct UnusedAsyncFn { + fn_span: Span, + await_in_async_block: Option, +} + +impl_lint_pass!(UnusedAsync => [UNUSED_ASYNC]); struct AsyncFnVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, @@ -101,24 +119,71 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync { }; walk_fn(&mut visitor, fn_kind, fn_decl, body.id(), def_id); if !visitor.found_await { - span_lint_and_then( - cx, - UNUSED_ASYNC, - span, - "unused `async` for function with no await statements", - |diag| { - diag.help("consider removing the `async` from this function"); - - if let Some(span) = visitor.await_in_async_block { - diag.span_note( - span, - "`await` used in an async block, which does not require \ - the enclosing function to be `async`", - ); - } + // Don't lint just yet, but store the necessary information for later. + // The actual linting happens in `check_crate_post`, once we've found all + // uses of local async functions that do require asyncness to pass typeck + self.unused_async_fns.insert( + def_id, + UnusedAsyncFn { + await_in_async_block: visitor.await_in_async_block, + fn_span: span, }, ); } } } + + fn check_path(&mut self, cx: &LateContext<'tcx>, path: &rustc_hir::Path<'tcx>, hir_id: rustc_hir::HirId) { + fn is_node_func_call(node: Node<'_>, expected_receiver: Span) -> bool { + matches!( + node, + Node::Expr(Expr { + kind: ExprKind::Call(Expr { span, .. }, _) | ExprKind::MethodCall(_, Expr { span, .. }, ..), + .. + }) if *span == expected_receiver + ) + } + + // Find paths to local async functions that aren't immediately called. + // E.g. `async fn f() {}; let x = f;` + // Depending on how `x` is used, f's asyncness might be required despite not having any `await` + // statements, so don't lint at all if there are any such paths. + if let Some(def_id) = path.res.opt_def_id() + && let Some(local_def_id) = def_id.as_local() + && let Some(DefKind::Fn) = cx.tcx.opt_def_kind(def_id) + && cx.tcx.asyncness(def_id).is_async() + && !is_node_func_call(cx.tcx.hir().get_parent(hir_id), path.span) + { + self.async_fns_as_value.insert(local_def_id); + } + } + + // After collecting all unused `async` and problematic paths to such functions, + // lint those unused ones that didn't have any path expressions to them. + fn check_crate_post(&mut self, cx: &LateContext<'tcx>) { + let iter = self + .unused_async_fns + .iter() + .filter_map(|(did, item)| (!self.async_fns_as_value.contains(did)).then_some(item)); + + for fun in iter { + span_lint_and_then( + cx, + UNUSED_ASYNC, + fun.fn_span, + "unused `async` for function with no await statements", + |diag| { + diag.help("consider removing the `async` from this function"); + + if let Some(span) = fun.await_in_async_block { + diag.span_note( + span, + "`await` used in an async block, which does not require \ + the enclosing function to be `async`", + ); + } + }, + ); + } + } } diff --git a/tests/ui/unused_async.rs b/tests/ui/unused_async.rs index 69e46ab47362..1d188025e418 100644 --- a/tests/ui/unused_async.rs +++ b/tests/ui/unused_async.rs @@ -37,6 +37,23 @@ mod issue10459 { } } +mod issue9695 { + use std::future::Future; + + async fn f() {} + async fn f2() {} + async fn f3() {} + + fn needs_async_fn>(_: fn() -> F) {} + + fn test() { + let x = f; + needs_async_fn(x); // async needed in f + needs_async_fn(f2); // async needed in f2 + f3(); // async not needed in f3 + } +} + async fn foo() -> i32 { 4 } diff --git a/tests/ui/unused_async.stderr b/tests/ui/unused_async.stderr index ffae8366b88c..035dffa7f13c 100644 --- a/tests/ui/unused_async.stderr +++ b/tests/ui/unused_async.stderr @@ -1,3 +1,22 @@ +error: unused `async` for function with no await statements + --> $DIR/unused_async.rs:45:5 + | +LL | async fn f3() {} + | ^^^^^^^^^^^^^^^^ + | + = help: consider removing the `async` from this function + = note: `-D clippy::unused-async` implied by `-D warnings` + +error: unused `async` for function with no await statements + --> $DIR/unused_async.rs:57:1 + | +LL | / async fn foo() -> i32 { +LL | | 4 +LL | | } + | |_^ + | + = help: consider removing the `async` from this function + error: unused `async` for function with no await statements --> $DIR/unused_async.rs:13:5 | @@ -14,20 +33,9 @@ note: `await` used in an async block, which does not require the enclosing funct | LL | ready(()).await; | ^^^^^ - = note: `-D clippy::unused-async` implied by `-D warnings` - -error: unused `async` for function with no await statements - --> $DIR/unused_async.rs:40:1 - | -LL | / async fn foo() -> i32 { -LL | | 4 -LL | | } - | |_^ - | - = help: consider removing the `async` from this function error: unused `async` for function with no await statements - --> $DIR/unused_async.rs:51:5 + --> $DIR/unused_async.rs:68:5 | LL | / async fn unused(&self) -> i32 { LL | | 1 @@ -36,5 +44,5 @@ LL | | } | = help: consider removing the `async` from this function -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors From 482d5fafc9ffc7edf9236ea186883ee0346e7938 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 22 Jul 2023 14:33:36 +0200 Subject: [PATCH 2/2] replace HashMap with Vec, use span_lint_hir_and_then --- clippy_lints/src/unused_async.rs | 21 +++++++++---------- tests/ui/unused_async.stderr | 36 ++++++++++++++++---------------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/clippy_lints/src/unused_async.rs b/clippy_lints/src/unused_async.rs index 5c8c6e589141..bc7c3897a6e8 100644 --- a/clippy_lints/src/unused_async.rs +++ b/clippy_lints/src/unused_async.rs @@ -1,6 +1,5 @@ -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::is_def_id_trait_method; -use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::DefKind; use rustc_hir::intravisit::{walk_body, walk_expr, walk_fn, FnKind, Visitor}; use rustc_hir::{Body, Expr, ExprKind, FnDecl, Node, YieldSource}; @@ -47,11 +46,12 @@ pub struct UnusedAsync { async_fns_as_value: LocalDefIdSet, /// Functions with unused `async`, linted post-crate after we've found all uses of local async /// functions - unused_async_fns: FxHashMap, + unused_async_fns: Vec, } #[derive(Copy, Clone)] struct UnusedAsyncFn { + def_id: LocalDefId, fn_span: Span, await_in_async_block: Option, } @@ -122,13 +122,11 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync { // Don't lint just yet, but store the necessary information for later. // The actual linting happens in `check_crate_post`, once we've found all // uses of local async functions that do require asyncness to pass typeck - self.unused_async_fns.insert( + self.unused_async_fns.push(UnusedAsyncFn { + await_in_async_block: visitor.await_in_async_block, + fn_span: span, def_id, - UnusedAsyncFn { - await_in_async_block: visitor.await_in_async_block, - fn_span: span, - }, - ); + }); } } } @@ -164,12 +162,13 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync { let iter = self .unused_async_fns .iter() - .filter_map(|(did, item)| (!self.async_fns_as_value.contains(did)).then_some(item)); + .filter(|UnusedAsyncFn { def_id, .. }| (!self.async_fns_as_value.contains(def_id))); for fun in iter { - span_lint_and_then( + span_lint_hir_and_then( cx, UNUSED_ASYNC, + cx.tcx.local_def_id_to_hir_id(fun.def_id), fun.fn_span, "unused `async` for function with no await statements", |diag| { diff --git a/tests/ui/unused_async.stderr b/tests/ui/unused_async.stderr index 035dffa7f13c..8d9b72c4886c 100644 --- a/tests/ui/unused_async.stderr +++ b/tests/ui/unused_async.stderr @@ -1,3 +1,21 @@ +error: unused `async` for function with no await statements + --> $DIR/unused_async.rs:13:5 + | +LL | / async fn async_block_await() { +LL | | async { +LL | | ready(()).await; +LL | | }; +LL | | } + | |_____^ + | + = help: consider removing the `async` from this function +note: `await` used in an async block, which does not require the enclosing function to be `async` + --> $DIR/unused_async.rs:15:23 + | +LL | ready(()).await; + | ^^^^^ + = note: `-D clippy::unused-async` implied by `-D warnings` + error: unused `async` for function with no await statements --> $DIR/unused_async.rs:45:5 | @@ -5,7 +23,6 @@ LL | async fn f3() {} | ^^^^^^^^^^^^^^^^ | = help: consider removing the `async` from this function - = note: `-D clippy::unused-async` implied by `-D warnings` error: unused `async` for function with no await statements --> $DIR/unused_async.rs:57:1 @@ -17,23 +34,6 @@ LL | | } | = help: consider removing the `async` from this function -error: unused `async` for function with no await statements - --> $DIR/unused_async.rs:13:5 - | -LL | / async fn async_block_await() { -LL | | async { -LL | | ready(()).await; -LL | | }; -LL | | } - | |_____^ - | - = help: consider removing the `async` from this function -note: `await` used in an async block, which does not require the enclosing function to be `async` - --> $DIR/unused_async.rs:15:23 - | -LL | ready(()).await; - | ^^^^^ - error: unused `async` for function with no await statements --> $DIR/unused_async.rs:68:5 |