Rollup merge of #94580 - xFrednet:55112-only-reason-in-lint-attr, r=lcnr

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
2022-03-08 11:04:52 +01:00 · 2022-03-08 11:04:52 +01:00 · 83ed227a8f
commit 83ed227a8f
parent aec535f805 700ec66aed
6 changed files with 133 additions and 41 deletions
--- a/compiler/rustc_lint/src/levels.rs
+++ b/compiler/rustc_lint/src/levels.rs
@ -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;
            }

@ -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(
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/compiler/rustc_passes/src/check_attr.rs
@ -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};
@ -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 {
@ -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> {
--- a/src/test/ui/empty/empty-attributes.rs
+++ b/src/test/ui/empty/empty-attributes.rs
@ -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
--- a/src/test/ui/empty/empty-attributes.stderr
+++ b/src/test/ui/empty/empty-attributes.stderr
@ -1,18 +1,18 @@
 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
@ -20,7 +20,7 @@ LL | #[target_feature()]
   = 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
@ -28,7 +28,15 @@ LL | #![allow()]
   = 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
@ -36,7 +44,7 @@ LL | #![warn()]
   = 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
@ -44,7 +52,7 @@ LL | #![deny()]
   = 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
@ -52,12 +60,12 @@ LL | #![forbid()]
   = 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

--- a/src/test/ui/lint/rfc-2383-lint-reason/lint-attribute-only-with-reason.rs
+++ b/src/test/ui/lint/rfc-2383-lint-reason/lint-attribute-only-with-reason.rs
@ -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() {}
--- a/src/test/ui/lint/rfc-2383-lint-reason/lint-attribute-only-with-reason.stderr
+++ b/src/test/ui/lint/rfc-2383-lint-reason/lint-attribute-only-with-reason.stderr
@ -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
+