From e2742a0ea24b4c596514021d0c94ff5a78ff0757 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Mon, 3 Apr 2023 01:27:53 +0200 Subject: [PATCH 1/3] Fix `double_must_use` for async functions --- clippy_lints/src/functions/must_use.rs | 7 ++++--- tests/ui/double_must_use.rs | 6 ++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/functions/must_use.rs b/clippy_lints/src/functions/must_use.rs index 1e9e826631c3..56acd31b1d2d 100644 --- a/clippy_lints/src/functions/must_use.rs +++ b/clippy_lints/src/functions/must_use.rs @@ -23,7 +23,8 @@ use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT}; pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { let attrs = cx.tcx.hir().attrs(item.hir_id()); let attr = cx.tcx.get_attr(item.owner_id, sym::must_use); - if let hir::ItemKind::Fn(ref sig, _generics, ref body_id) = item.kind { + if let hir::ItemKind::Fn(ref sig, _generics, ref body_id) = item.kind && !sig.header.is_async() /* (#10486) */ { + let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id); let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); if let Some(attr) = attr { @@ -43,7 +44,7 @@ pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_> } pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) { - if let hir::ImplItemKind::Fn(ref sig, ref body_id) = item.kind { + if let hir::ImplItemKind::Fn(ref sig, ref body_id) = item.kind && !sig.header.is_async() /* (#10486) */ { let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id); let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); let attrs = cx.tcx.hir().attrs(item.hir_id()); @@ -65,7 +66,7 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp } pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { - if let hir::TraitItemKind::Fn(ref sig, ref eid) = item.kind { + if let hir::TraitItemKind::Fn(ref sig, ref eid) = item.kind && !sig.header.is_async() /* (#10486) */ { let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id); let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); diff --git a/tests/ui/double_must_use.rs b/tests/ui/double_must_use.rs index 05e087b08bc1..a4cd8630de59 100644 --- a/tests/ui/double_must_use.rs +++ b/tests/ui/double_must_use.rs @@ -21,6 +21,12 @@ pub fn must_use_with_note() -> Result<(), ()> { unimplemented!(); } +// vvvv Should not lint (#10486) +#[must_use] +async fn async_must_use() -> usize { + unimplemented!(); +} + fn main() { must_use_result(); must_use_tuple(); From d60274355894c0e0768b034ab538aa0625fc2a91 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Mon, 3 Apr 2023 15:15:43 +0200 Subject: [PATCH 2/3] only focus on `double_must_use` + Add `Result<(), ()>` test --- clippy_lints/src/functions/must_use.rs | 17 +++++++++-------- tests/ui/double_must_use.rs | 5 +++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/functions/must_use.rs b/clippy_lints/src/functions/must_use.rs index 56acd31b1d2d..ab68d7a3726f 100644 --- a/clippy_lints/src/functions/must_use.rs +++ b/clippy_lints/src/functions/must_use.rs @@ -1,3 +1,4 @@ +use hir::FnSig; use rustc_ast::ast::Attribute; use rustc_errors::Applicability; use rustc_hir::def_id::DefIdSet; @@ -23,12 +24,11 @@ use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT}; pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { let attrs = cx.tcx.hir().attrs(item.hir_id()); let attr = cx.tcx.get_attr(item.owner_id, sym::must_use); - if let hir::ItemKind::Fn(ref sig, _generics, ref body_id) = item.kind && !sig.header.is_async() /* (#10486) */ { - + if let hir::ItemKind::Fn(ref sig, _generics, ref body_id) = item.kind { let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id); let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); if let Some(attr) = attr { - check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr); + check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, sig); } else if is_public && !is_proc_macro(attrs) && !attrs.iter().any(|a| a.has_name(sym::no_mangle)) { check_must_use_candidate( cx, @@ -44,13 +44,13 @@ pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_> } pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) { - if let hir::ImplItemKind::Fn(ref sig, ref body_id) = item.kind && !sig.header.is_async() /* (#10486) */ { + if let hir::ImplItemKind::Fn(ref sig, ref body_id) = item.kind { let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id); let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); let attrs = cx.tcx.hir().attrs(item.hir_id()); let attr = cx.tcx.get_attr(item.owner_id, sym::must_use); if let Some(attr) = attr { - check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr); + check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, sig); } else if is_public && !is_proc_macro(attrs) && trait_ref_of_method(cx, item.owner_id.def_id).is_none() { check_must_use_candidate( cx, @@ -66,14 +66,14 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp } pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { - if let hir::TraitItemKind::Fn(ref sig, ref eid) = item.kind && !sig.header.is_async() /* (#10486) */ { + if let hir::TraitItemKind::Fn(ref sig, ref eid) = item.kind { let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id); let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); let attrs = cx.tcx.hir().attrs(item.hir_id()); let attr = cx.tcx.get_attr(item.owner_id, sym::must_use); if let Some(attr) = attr { - check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr); + check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, sig); } else if let hir::TraitFn::Provided(eid) = *eid { let body = cx.tcx.hir().body(eid); if attr.is_none() && is_public && !is_proc_macro(attrs) { @@ -98,6 +98,7 @@ fn check_needless_must_use( item_span: Span, fn_header_span: Span, attr: &Attribute, + sig: &FnSig<'_>, ) { if in_external_macro(cx.sess(), item_span) { return; @@ -112,7 +113,7 @@ fn check_needless_must_use( diag.span_suggestion(attr.span, "remove the attribute", "", Applicability::MachineApplicable); }, ); - } else if attr.value_str().is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) { + } else if attr.value_str().is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) && !sig.header.is_async() { span_lint_and_help( cx, DOUBLE_MUST_USE, diff --git a/tests/ui/double_must_use.rs b/tests/ui/double_must_use.rs index a4cd8630de59..26a387b3cf04 100644 --- a/tests/ui/double_must_use.rs +++ b/tests/ui/double_must_use.rs @@ -27,6 +27,11 @@ async fn async_must_use() -> usize { unimplemented!(); } +#[must_use] +async fn async_must_use_result() -> Result<(), ()> { + Ok(()) +} + fn main() { must_use_result(); must_use_tuple(); From a37eb4dfc97cdcbd177ccc0fda1909014b145635 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Mon, 3 Apr 2023 16:01:30 +0200 Subject: [PATCH 3/3] Fix false negative on `Result<(), ()>` --- clippy_lints/src/functions/must_use.rs | 12 +++++++++++- tests/ui/double_must_use.stderr | 10 +++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/functions/must_use.rs b/clippy_lints/src/functions/must_use.rs index ab68d7a3726f..d0ad26282642 100644 --- a/clippy_lints/src/functions/must_use.rs +++ b/clippy_lints/src/functions/must_use.rs @@ -3,6 +3,7 @@ use rustc_ast::ast::Attribute; use rustc_errors::Applicability; use rustc_hir::def_id::DefIdSet; use rustc_hir::{self as hir, def::Res, QPath}; +use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LintContext}; use rustc_middle::{ lint::in_external_macro, @@ -113,7 +114,16 @@ fn check_needless_must_use( diag.span_suggestion(attr.span, "remove the attribute", "", Applicability::MachineApplicable); }, ); - } else if attr.value_str().is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) && !sig.header.is_async() { + } else if attr.value_str().is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) { + // Ignore async functions unless Future::Output type is a must_use type + if sig.header.is_async() { + let infcx = cx.tcx.infer_ctxt().build(); + if let Some(future_ty) = infcx.get_impl_future_output_ty(return_ty(cx, item_id)) + && !is_must_use_ty(cx, future_ty) { + return; + } + } + span_lint_and_help( cx, DOUBLE_MUST_USE, diff --git a/tests/ui/double_must_use.stderr b/tests/ui/double_must_use.stderr index 3d34557a881b..49ab2ea3e12b 100644 --- a/tests/ui/double_must_use.stderr +++ b/tests/ui/double_must_use.stderr @@ -23,5 +23,13 @@ LL | pub fn must_use_array() -> [Result<(), ()>; 1] { | = help: either add some descriptive text or remove the attribute -error: aborting due to 3 previous errors +error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` + --> $DIR/double_must_use.rs:31:1 + | +LL | async fn async_must_use_result() -> Result<(), ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: either add some descriptive text or remove the attribute + +error: aborting due to 4 previous errors