Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit unused_attributes if a level attr only has a reason #94580

Merged
merged 1 commit into from
Mar 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ impl<'s> LintLevelsBuilder<'s> {
};

if metas.is_empty() {
// FIXME (#55112): issue unused-attributes lint for `#[level()]`
// This emits the unused_attributes lint for `#[level()]`
continue;
}

Expand All @@ -271,8 +271,6 @@ impl<'s> LintLevelsBuilder<'s> {
ast::MetaItemKind::Word => {} // actual lint names handled later
ast::MetaItemKind::NameValue(ref name_value) => {
if item.path == sym::reason {
// FIXME (#55112): issue unused-attributes lint if we thereby
// don't have any lint names (`#[level(reason = "foo")]`)
if let ast::LitKind::Str(rationale, _) = name_value.kind {
if !self.sess.features_untracked().lint_reasons {
feature_err(
Expand Down
80 changes: 51 additions & 29 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! conflicts between multiple such attributes attached to the same
//! item.

use rustc_ast::{ast, AttrStyle, Attribute, Lit, LitKind, NestedMetaItem};
use rustc_ast::{ast, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{pluralize, struct_span_err, Applicability};
use rustc_feature::{AttributeDuplicates, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
Expand Down Expand Up @@ -178,34 +178,7 @@ impl CheckAttrVisitor<'_> {
check_duplicates(self.tcx, attr, hir_id, *duplicates, &mut seen);
}

// Warn on useless empty attributes.
if matches!(
attr.name_or_empty(),
sym::macro_use
| sym::allow
| sym::warn
| sym::deny
| sym::forbid
| sym::feature
| sym::repr
| sym::target_feature
) && attr.meta_item_list().map_or(false, |list| list.is_empty())
{
self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
lint.build("unused attribute")
.span_suggestion(
attr.span,
"remove this attribute",
String::new(),
Applicability::MachineApplicable,
)
.note(&format!(
"attribute `{}` with an empty list has no effect",
attr.name_or_empty()
))
.emit();
});
}
self.check_unused_attribute(hir_id, attr)
}

if !is_valid {
Expand Down Expand Up @@ -1969,6 +1942,55 @@ impl CheckAttrVisitor<'_> {
});
}
}

fn check_unused_attribute(&self, hir_id: HirId, attr: &Attribute) {
// Warn on useless empty attributes.
let note = if matches!(
attr.name_or_empty(),
sym::macro_use
| sym::allow
| sym::expect
| sym::warn
| sym::deny
| sym::forbid
| sym::feature
| sym::repr
| sym::target_feature
) && attr.meta_item_list().map_or(false, |list| list.is_empty())
{
format!(
"attribute `{}` with an empty list has no effect",
attr.name_or_empty()
)
} else if matches!(
attr.name_or_empty(),
sym::allow | sym::warn | sym::deny | sym::forbid | sym::expect
) && let Some(meta) = attr.meta_item_list()
&& meta.len() == 1
&& let Some(item) = meta[0].meta_item()
&& let MetaItemKind::NameValue(_) = &item.kind
&& item.path == sym::reason
{
format!(
"attribute `{}` without any lints has no effect",
attr.name_or_empty()
)
} else {
return;
};

self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
lint.build("unused attribute")
.span_suggestion(
attr.span,
"remove this attribute",
String::new(),
Applicability::MachineApplicable,
)
.note(&note)
.emit();
});
}
}

impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/empty/empty-attributes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#![feature(lint_reasons)]

#![deny(unused_attributes)]
#![allow()] //~ ERROR unused attribute
#![expect()] //~ ERROR unused attribute
#![warn()] //~ ERROR unused attribute
#![deny()] //~ ERROR unused attribute
#![forbid()] //~ ERROR unused attribute
Expand Down
26 changes: 17 additions & 9 deletions src/test/ui/empty/empty-attributes.stderr
Original file line number Diff line number Diff line change
@@ -1,63 +1,71 @@
error: unused attribute
--> $DIR/empty-attributes.rs:8:1
--> $DIR/empty-attributes.rs:11:1
|
LL | #[repr()]
| ^^^^^^^^^ help: remove this attribute
|
note: the lint level is defined here
--> $DIR/empty-attributes.rs:1:9
--> $DIR/empty-attributes.rs:3:9
|
LL | #![deny(unused_attributes)]
| ^^^^^^^^^^^^^^^^^
= note: attribute `repr` with an empty list has no effect

error: unused attribute
--> $DIR/empty-attributes.rs:11:1
--> $DIR/empty-attributes.rs:14:1
|
LL | #[target_feature()]
| ^^^^^^^^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `target_feature` with an empty list has no effect

error: unused attribute
--> $DIR/empty-attributes.rs:2:1
--> $DIR/empty-attributes.rs:4:1
|
LL | #![allow()]
| ^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `allow` with an empty list has no effect

error: unused attribute
--> $DIR/empty-attributes.rs:3:1
--> $DIR/empty-attributes.rs:5:1
|
LL | #![expect()]
| ^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `expect` with an empty list has no effect

error: unused attribute
--> $DIR/empty-attributes.rs:6:1
|
LL | #![warn()]
| ^^^^^^^^^^ help: remove this attribute
|
= note: attribute `warn` with an empty list has no effect

error: unused attribute
--> $DIR/empty-attributes.rs:4:1
--> $DIR/empty-attributes.rs:7:1
|
LL | #![deny()]
| ^^^^^^^^^^ help: remove this attribute
|
= note: attribute `deny` with an empty list has no effect

error: unused attribute
--> $DIR/empty-attributes.rs:5:1
--> $DIR/empty-attributes.rs:8:1
|
LL | #![forbid()]
| ^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `forbid` with an empty list has no effect

error: unused attribute
--> $DIR/empty-attributes.rs:6:1
--> $DIR/empty-attributes.rs:9:1
|
LL | #![feature()]
| ^^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `feature` with an empty list has no effect

error: aborting due to 7 previous errors
error: aborting due to 8 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(lint_reasons)]

#![deny(unused_attributes)]

#[allow(reason = "I want to allow something")]//~ ERROR unused attribute
#[expect(reason = "I don't know what I'm waiting for")]//~ ERROR unused attribute
#[warn(reason = "This should be warn by default")]//~ ERROR unused attribute
#[deny(reason = "All listed lints are denied")]//~ ERROR unused attribute
#[forbid(reason = "Just some reason")]//~ ERROR unused attribute

#[allow(clippy::box_collection, reason = "This is still valid")]
#[warn(dead_code, reason = "This is also reasonable")]

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error: unused attribute
--> $DIR/lint-attribute-only-with-reason.rs:5:1
|
LL | #[allow(reason = "I want to allow something")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
|
note: the lint level is defined here
--> $DIR/lint-attribute-only-with-reason.rs:3:9
|
LL | #![deny(unused_attributes)]
| ^^^^^^^^^^^^^^^^^
= note: attribute `allow` without any lints has no effect

error: unused attribute
--> $DIR/lint-attribute-only-with-reason.rs:6:1
|
LL | #[expect(reason = "I don't know what I'm waiting for")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `expect` without any lints has no effect

error: unused attribute
--> $DIR/lint-attribute-only-with-reason.rs:7:1
|
LL | #[warn(reason = "This should be warn by default")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `warn` without any lints has no effect

error: unused attribute
--> $DIR/lint-attribute-only-with-reason.rs:8:1
|
LL | #[deny(reason = "All listed lints are denied")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `deny` without any lints has no effect

error: unused attribute
--> $DIR/lint-attribute-only-with-reason.rs:9:1
|
LL | #[forbid(reason = "Just some reason")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
|
= note: attribute `forbid` without any lints has no effect

error: aborting due to 5 previous errors