Skip to content

Commit

Permalink
Auto merge of #13471 - y21:issue13466-fp, r=llogiq
Browse files Browse the repository at this point in the history
Remove method call receiver special casing in `unused_async` lint

Fixes the false positive mentioned in #13466 (comment).

The false negative in the OP would be nice to fix too, but I'd rather do that in a separate PR because it's much more involved

Before this change, the `unused_async` lint would check if the async fn is also used anywhere and avoid linting if so. The exception is if the async function is immediately called, because the returned future handling can be easily removed (and also if we don't have some exceptions then the lint wouldn't trigger anywhere) *or* if it's a method call receiver.

I'm not exactly sure why I implemented that special casing for method call receivers in #11200, but it doesn't make much sense in hindsight imo. Especially given that method calls are essentially equivalent to function calls with the receiver as the first argument, which was the primary motivation for not linting in the first place (async fn passed to another function, like `axum::get(handler)` where handler has to be an async fn).

changelog: none
  • Loading branch information
bors committed Sep 28, 2024
2 parents b367d34 + d24a631 commit 897f0e4
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 15 deletions.
23 changes: 10 additions & 13 deletions clippy_lints/src/unused_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::is_def_id_trait_method;
use rustc_hir::def::DefKind;
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn};
use rustc_hir::{Body, Expr, ExprKind, FnDecl, Node, YieldSource};
use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId, Node, YieldSource};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::nested_filter;
use rustc_session::impl_lint_pass;
Expand Down Expand Up @@ -137,17 +137,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
}
}

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
)
}

fn check_path(&mut self, cx: &LateContext<'tcx>, path: &rustc_hir::Path<'tcx>, hir_id: HirId) {
// 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`
Expand All @@ -156,7 +146,14 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
&& let Some(local_def_id) = def_id.as_local()
&& cx.tcx.def_kind(def_id) == DefKind::Fn
&& cx.tcx.asyncness(def_id).is_async()
&& !is_node_func_call(cx.tcx.parent_hir_node(hir_id), path.span)
&& let parent = cx.tcx.parent_hir_node(hir_id)
&& !matches!(
parent,
Node::Expr(Expr {
kind: ExprKind::Call(Expr { span, .. }, _),
..
}) if *span == path.span
)
{
self.async_fns_as_value.insert(local_def_id);
}
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/unused_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,22 @@ mod issue9695 {
}
}

mod issue13466 {
use std::future::Future;

struct Wrap<F>(F);
impl<F> From<F> for Wrap<F> {
fn from(f: F) -> Self {
Self(f)
}
}
fn takes_fut<F: Fn() -> Fut, Fut: Future>(_: Wrap<F>) {}
async fn unused_async() {}
fn fp() {
takes_fut(unused_async.into());
}
}

async fn foo() -> i32 {
//~^ ERROR: unused `async` for function with no await statements
4
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/unused_async.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ LL | async fn f3() {}
= help: consider removing the `async` from this function

error: unused `async` for function with no await statements
--> tests/ui/unused_async.rs:58:1
--> tests/ui/unused_async.rs:74:1
|
LL | / async fn foo() -> i32 {
LL | |
Expand All @@ -38,7 +38,7 @@ LL | | }
= help: consider removing the `async` from this function

error: unused `async` for function with no await statements
--> tests/ui/unused_async.rs:70:5
--> tests/ui/unused_async.rs:86:5
|
LL | / async fn unused(&self) -> i32 {
LL | |
Expand Down

0 comments on commit 897f0e4

Please sign in to comment.