From a43bfefd19a3af040b6cbc5cc607814d9250689a Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 27 Jun 2023 22:00:00 +0200 Subject: [PATCH 1/2] [`unused_async`]: don't lint on async trait impls --- clippy_lints/src/unused_async.rs | 18 ++++++++++++++++-- tests/ui/unused_async.rs | 14 ++++++++++++++ tests/ui/unused_async.stderr | 8 ++++---- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/unused_async.rs b/clippy_lints/src/unused_async.rs index 117dda09222..21153973dff 100644 --- a/clippy_lints/src/unused_async.rs +++ b/clippy_lints/src/unused_async.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; 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, ItemKind, Node, YieldSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -81,6 +81,20 @@ impl<'a, 'tcx> Visitor<'tcx> for AsyncFnVisitor<'a, 'tcx> { } } +/// Checks if the `def_id` belongs to a function and that function is part of a trait impl, +/// in which case we shouldn't lint because `async` is part of the trait definition and therefore +/// can't be removed. +fn is_in_trait_impl(cx: &LateContext<'_>, def_id: LocalDefId) -> bool { + if let Some(hir_id) = cx.tcx.opt_local_def_id_to_hir_id(def_id) + && let Node::Item(item) = cx.tcx.hir().get_parent(hir_id) + && let ItemKind::Impl(imp) = item.kind + { + imp.of_trait.is_some() + } else { + false + } +} + impl<'tcx> LateLintPass<'tcx> for UnusedAsync { fn check_fn( &mut self, @@ -91,7 +105,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync { span: Span, def_id: LocalDefId, ) { - if !span.from_expansion() && fn_kind.asyncness().is_async() { + if !span.from_expansion() && fn_kind.asyncness().is_async() && !is_in_trait_impl(cx, def_id) { let mut visitor = AsyncFnVisitor { cx, found_await: false, diff --git a/tests/ui/unused_async.rs b/tests/ui/unused_async.rs index bfaa5dadfa5..69e46ab4736 100644 --- a/tests/ui/unused_async.rs +++ b/tests/ui/unused_async.rs @@ -1,4 +1,6 @@ #![warn(clippy::unused_async)] +#![feature(async_fn_in_trait)] +#![allow(incomplete_features)] use std::future::Future; use std::pin::Pin; @@ -23,6 +25,18 @@ mod issue10800 { } } +mod issue10459 { + trait HasAsyncMethod { + async fn do_something() -> u32; + } + + impl HasAsyncMethod for () { + async fn do_something() -> u32 { + 1 + } + } +} + async fn foo() -> i32 { 4 } diff --git a/tests/ui/unused_async.stderr b/tests/ui/unused_async.stderr index 8ac2066a6b2..ffae8366b88 100644 --- a/tests/ui/unused_async.stderr +++ b/tests/ui/unused_async.stderr @@ -1,5 +1,5 @@ error: unused `async` for function with no await statements - --> $DIR/unused_async.rs:11:5 + --> $DIR/unused_async.rs:13:5 | LL | / async fn async_block_await() { LL | | async { @@ -10,14 +10,14 @@ 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:13:23 + --> $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:26:1 + --> $DIR/unused_async.rs:40:1 | LL | / async fn foo() -> i32 { LL | | 4 @@ -27,7 +27,7 @@ LL | | } = help: consider removing the `async` from this function error: unused `async` for function with no await statements - --> $DIR/unused_async.rs:37:5 + --> $DIR/unused_async.rs:51:5 | LL | / async fn unused(&self) -> i32 { LL | | 1 From b713cd5945501c07990e9845eed7a5e612d1c135 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 28 Jun 2023 11:35:10 +0200 Subject: [PATCH 2/2] move `is_in_trait_method` to utils and rename --- clippy_lints/src/unused_async.rs | 19 +++---------------- clippy_utils/src/lib.rs | 12 ++++++++++++ 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/unused_async.rs b/clippy_lints/src/unused_async.rs index 21153973dff..5e42cf7e4f3 100644 --- a/clippy_lints/src/unused_async.rs +++ b/clippy_lints/src/unused_async.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_def_id_trait_method; use rustc_hir::intravisit::{walk_body, walk_expr, walk_fn, FnKind, Visitor}; -use rustc_hir::{Body, Expr, ExprKind, FnDecl, ItemKind, Node, YieldSource}; +use rustc_hir::{Body, Expr, ExprKind, FnDecl, YieldSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -81,20 +82,6 @@ impl<'a, 'tcx> Visitor<'tcx> for AsyncFnVisitor<'a, 'tcx> { } } -/// Checks if the `def_id` belongs to a function and that function is part of a trait impl, -/// in which case we shouldn't lint because `async` is part of the trait definition and therefore -/// can't be removed. -fn is_in_trait_impl(cx: &LateContext<'_>, def_id: LocalDefId) -> bool { - if let Some(hir_id) = cx.tcx.opt_local_def_id_to_hir_id(def_id) - && let Node::Item(item) = cx.tcx.hir().get_parent(hir_id) - && let ItemKind::Impl(imp) = item.kind - { - imp.of_trait.is_some() - } else { - false - } -} - impl<'tcx> LateLintPass<'tcx> for UnusedAsync { fn check_fn( &mut self, @@ -105,7 +92,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync { span: Span, def_id: LocalDefId, ) { - if !span.from_expansion() && fn_kind.asyncness().is_async() && !is_in_trait_impl(cx, def_id) { + if !span.from_expansion() && fn_kind.asyncness().is_async() && !is_def_id_trait_method(cx, def_id) { let mut visitor = AsyncFnVisitor { cx, found_await: false, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 078dcef1927..727b59f1f43 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -327,6 +327,18 @@ pub fn is_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) .map_or(false, |did| is_diag_trait_item(cx, did, diag_item)) } +/// Checks if the `def_id` belongs to a function that is part of a trait impl. +pub fn is_def_id_trait_method(cx: &LateContext<'_>, def_id: LocalDefId) -> bool { + if let Some(hir_id) = cx.tcx.opt_local_def_id_to_hir_id(def_id) + && let Node::Item(item) = cx.tcx.hir().get_parent(hir_id) + && let ItemKind::Impl(imp) = item.kind + { + imp.of_trait.is_some() + } else { + false + } +} + /// Checks if the given expression is a path referring an item on the trait /// that is marked with the given diagnostic item. ///