about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2022-03-08 11:04:52 +0100
committerGitHub <noreply@github.com>2022-03-08 11:04:52 +0100
commit83ed227a8f66faccade2fa9affc51e50a0087a5b (patch)
treee42af7f3d6b86009cf38a7e4c5e1c39707731e23
parentaec535f8051c7cf00b1e373a05893b2cb580eeac (diff)
parent700ec66aed90f446a2d09b6fc96599b1f8099057 (diff)
downloadrust-83ed227a8f66faccade2fa9affc51e50a0087a5b.tar.gz
rust-83ed227a8f66faccade2fa9affc51e50a0087a5b.zip
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 :upside_down_face:

cc: #55112
-rw-r--r--compiler/rustc_lint/src/levels.rs4
-rw-r--r--compiler/rustc_passes/src/check_attr.rs80
-rw-r--r--src/test/ui/empty/empty-attributes.rs3
-rw-r--r--src/test/ui/empty/empty-attributes.stderr26
-rw-r--r--src/test/ui/lint/rfc-2383-lint-reason/lint-attribute-only-with-reason.rs14
-rw-r--r--src/test/ui/lint/rfc-2383-lint-reason/lint-attribute-only-with-reason.stderr47
6 files changed, 133 insertions, 41 deletions
diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs
index f46f74fa45f..bbfbf61f486 100644
--- 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(
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index d94ad7ba71a..22020015550 100644
--- 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> {
diff --git a/src/test/ui/empty/empty-attributes.rs b/src/test/ui/empty/empty-attributes.rs
index 7e9b05587b0..d319227b217 100644
--- 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
diff --git a/src/test/ui/empty/empty-attributes.stderr b/src/test/ui/empty/empty-attributes.stderr
index e0798e4f0c6..8653eaf5ccd 100644
--- 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
 
diff --git 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
new file mode 100644
index 00000000000..bafdea96e08
--- /dev/null
+++ 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() {}
diff --git 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
new file mode 100644
index 00000000000..3bf8137dc6e
--- /dev/null
+++ 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
+