Skip to content

Commit

Permalink
Rollup merge of #94580 - xFrednet:55112-only-reason-in-lint-attr, r=lcnr
Browse files Browse the repository at this point in the history
Emit `unused_attributes` if a level attr only has a reason

Fixes a comment from `compiler/rustc_lint/src/levels.rs`. Lint level attributes that only contain a reason will also trigger the `unused_attribute` lint. The lint now also checks for the `expect` lint level.

That's it, have a great rest of the day for everyone reasoning this 🙃

cc: #55112
  • Loading branch information
matthiaskrgr authored Mar 8, 2022
2 parents aec535f + 700ec66 commit 83ed227
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 41 deletions.
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

0 comments on commit 83ed227

Please sign in to comment.