user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

correct suggestion for must_use_unit when there are multiple attributes

Browse source
This commit is contained in:
lapla-cogito 2024-12-15 20:28:38 +09:00
parent 62407104fb
commit e7c27c6774
No known key found for this signature in database
GPG key ID: B39C71D9F127FF9F
4 changed files with 75 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

58
clippy_lints/src/functions/must_use.rs
View file

@ -29,7 +29,7 @@ pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>
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, sig);
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
} else if is_public && !is_proc_macro(attrs) && !attrs.iter().any(|a| a.has_name(sym::no_mangle)) {
check_must_use_candidate(
cx,
@ -51,7 +51,7 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp
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, sig);
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, 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,
@ -74,7 +74,7 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr
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, sig);
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, 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) {
@ -92,6 +92,7 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr
}
}
#[allow(clippy::too_many_arguments)]
fn check_needless_must_use(
cx: &LateContext<'_>,
decl: &hir::FnDecl<'_>,
@ -99,21 +100,54 @@ fn check_needless_must_use(
item_span: Span,
fn_header_span: Span,
attr: &Attribute,
attrs: &[Attribute],
sig: &FnSig<'_>,
) {
if in_external_macro(cx.sess(), item_span) {
return;
}
if returns_unit(decl) {
span_lint_and_then(
cx,
MUST_USE_UNIT,
fn_header_span,
"this unit-returning function has a `#[must_use]` attribute",
|diag| {
diag.span_suggestion(attr.span, "remove the attribute", "", Applicability::MachineApplicable);
},
);
if attrs.len() == 1 {
span_lint_and_then(
cx,
MUST_USE_UNIT,
fn_header_span,
"this unit-returning function has a `#[must_use]` attribute",
|diag| {
diag.span_suggestion(attr.span, "remove the attribute", "", Applicability::MachineApplicable);
},
);
} else {
// When there are multiple attributes, it is not sufficient to simply make `must_use` empty, see
// issue #12320.
span_lint_and_then(
cx,
MUST_USE_UNIT,
fn_header_span,
"this unit-returning function has a `#[must_use]` attribute",
|diag| {
let mut attrs_without_must_use = attrs.to_vec();
attrs_without_must_use.retain(|a| a.id != attr.id);
let sugg_str = attrs_without_must_use
.iter()
.map(|a| {
if a.value_str().is_none() {
return a.name_or_empty().to_string();
}
format!("{} = \"{}\"", a.name_or_empty(), a.value_str().unwrap())
})
.collect::<Vec<_>>()
.join(", ");
diag.span_suggestion(
attrs[0].span.with_hi(attrs[attrs.len() - 1].span.hi()),
"change these attributes to",
sugg_str,
Applicability::MachineApplicable,
);
},
);
}
} 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() {

6
tests/ui/must_use_unit.fixed
View file

@ -23,3 +23,9 @@ fn main() {
fn foo() {}
);
}
#[cfg_attr(all(), deprecated)]
fn issue_12320() {}
#[cfg_attr(all(), deprecated, doc = "foo")]
fn issue_12320_2() {}

6
tests/ui/must_use_unit.rs
View file

@ -26,3 +26,9 @@ fn main() {
fn foo() {}
);
}
#[cfg_attr(all(), must_use, deprecated)]
fn issue_12320() {}
#[cfg_attr(all(), deprecated, doc = "foo", must_use)]
fn issue_12320_2() {}

18
tests/ui/must_use_unit.stderr
View file

@ -25,5 +25,21 @@ LL | #[must_use = "With note"]
LL | pub fn must_use_with_note() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 3 previous errors
error: this unit-returning function has a `#[must_use]` attribute
--> tests/ui/must_use_unit.rs:31:1
|
LL | #[cfg_attr(all(), must_use, deprecated)]
| -------------------- help: change these attributes to: `deprecated`
LL | fn issue_12320() {}
| ^^^^^^^^^^^^^^^^
error: this unit-returning function has a `#[must_use]` attribute
--> tests/ui/must_use_unit.rs:34:1
|
LL | #[cfg_attr(all(), deprecated, doc = "foo", must_use)]
| --------------------------------- help: change these attributes to: `deprecated, doc = "foo"`
LL | fn issue_12320_2() {}
| ^^^^^^^^^^^^^^^^^^
error: aborting due to 5 previous errors