Rollup merge of rust-lang#155572 - mejrs:move_target_checks, r=JonathanBrouwer,GuillaumeGomez

Move diagnostic attribute target checks from check_attr

Move diagnostic attribute target checks into their targets. Part of rust-lang#131229 (comment)

This is much easier with `emit_dyn_lint` :) (thanks @GuillaumeGomez !)

I think there might be some opportunity to simplify all these `check_diagnostic_*` methods in `check_attr`. However there are some diagnostic attribute prs in flight and I'd like to wait for those to land first and then think about it. So that PR is not for today.

r? @JonathanBrouwer (or @GuillaumeGomez if you want)

Author: jhpratt

compiler/rustc_attr_parsing/src/attributes/diagnostic/do_not_recommend.rs

@@ -1,19 +1,25 @@
+use rustc_errors::Diagnostic;
 use rustc_feature::{AttributeTemplate, template};
+use rustc_hir::Target;
 use rustc_hir::attrs::AttributeKind;
 use rustc_hir::lints::AttributeLintKind;
-use rustc_session::lint::builtin::MALFORMED_DIAGNOSTIC_ATTRIBUTES;
+use rustc_session::lint::builtin::{
+    MALFORMED_DIAGNOSTIC_ATTRIBUTES, MISPLACED_DIAGNOSTIC_ATTRIBUTES,
+};
 use rustc_span::{Symbol, sym};
 
 use crate::attributes::{OnDuplicate, SingleAttributeParser};
 use crate::context::{AcceptContext, Stage};
+use crate::errors::IncorrectDoNotRecommendLocation;
 use crate::parser::ArgParser;
 use crate::target_checking::{ALL_TARGETS, AllowedTargets};
 
 pub(crate) struct DoNotRecommendParser;
 impl<S: Stage> SingleAttributeParser<S> for DoNotRecommendParser {
     const PATH: &[Symbol] = &[sym::diagnostic, sym::do_not_recommend];
     const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Warn;
-    const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS); // Checked in check_attr.
+    // "Allowed" on any target, noop on all but trait impls
+    const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS);
     const TEMPLATE: AttributeTemplate = template!(Word /*doesn't matter */);
 
     fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser) -> Option<AttributeKind> {
@@ -25,6 +31,19 @@ impl<S: Stage> SingleAttributeParser<S> for DoNotRecommendParser {
                 attr_span,
             );
         }
+
+        if !matches!(cx.target, Target::Impl { of_trait: true }) {
+            let target_span = cx.target_span;
+            cx.emit_dyn_lint(
+                MISPLACED_DIAGNOSTIC_ATTRIBUTES,
+                move |dcx, level| {
+                    IncorrectDoNotRecommendLocation { target_span }.into_diag(dcx, level)
+                },
+                attr_span,
+            );
+            return None;
+        }
+
         Some(AttributeKind::DoNotRecommend { attr_span })
     }
 }

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_const.rs

@@ -1,8 +1,10 @@
+use rustc_errors::Diagnostic;
 use rustc_hir::attrs::diagnostic::Directive;
+use rustc_session::lint::builtin::MISPLACED_DIAGNOSTIC_ATTRIBUTES;
 
 use crate::attributes::diagnostic::*;
 use crate::attributes::prelude::*;
-
+use crate::errors::DiagnosticOnConstOnlyForTraitImpls;
 #[derive(Default)]
 pub(crate) struct OnConstParser {
     span: Option<Span>,
@@ -21,6 +23,21 @@ impl<S: Stage> AttributeParser<S> for OnConstParser {
 
             let span = cx.attr_span;
             this.span = Some(span);
+
+            // FIXME(mejrs) no constness field on `Target`,
+            // so non-constness is still checked in check_attr.rs
+            if !matches!(cx.target, Target::Impl { of_trait: true }) {
+                let target_span = cx.target_span;
+                cx.emit_dyn_lint(
+                    MISPLACED_DIAGNOSTIC_ATTRIBUTES,
+                    move |dcx, level| {
+                        DiagnosticOnConstOnlyForTraitImpls { target_span }.into_diag(dcx, level)
+                    },
+                    span,
+                );
+                return;
+            }
+
             let mode = Mode::DiagnosticOnConst;
 
             let Some(items) = parse_list(cx, args, mode) else { return };
@@ -32,7 +49,8 @@ impl<S: Stage> AttributeParser<S> for OnConstParser {
         },
     )];
 
-    //FIXME Still checked in `check_attr.rs`
+    // "Allowed" on all targets; noop on anything but non-const trait impls;
+    // this linted on in parser.
     const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS);
 
     fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> {

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_move.rs

@@ -1,10 +1,13 @@
+use rustc_errors::Diagnostic;
 use rustc_feature::template;
 use rustc_hir::attrs::AttributeKind;
+use rustc_session::lint::builtin::MISPLACED_DIAGNOSTIC_ATTRIBUTES;
 use rustc_span::sym;
 
 use crate::attributes::diagnostic::*;
 use crate::attributes::prelude::*;
 use crate::context::{AcceptContext, Stage};
+use crate::errors::DiagnosticOnMoveOnlyForAdt;
 use crate::parser::ArgParser;
 use crate::target_checking::{ALL_TARGETS, AllowedTargets};
 
@@ -29,6 +32,15 @@ impl OnMoveParser {
         let span = cx.attr_span;
         self.span = Some(span);
 
+        if !matches!(cx.target, Target::Enum | Target::Struct | Target::Union) {
+            cx.emit_dyn_lint(
+                MISPLACED_DIAGNOSTIC_ATTRIBUTES,
+                move |dcx, level| DiagnosticOnMoveOnlyForAdt.into_diag(dcx, level),
+                span,
+            );
+            return;
+        }
+
         let Some(items) = parse_list(cx, args, mode) else { return };
 
         if let Some(directive) = parse_directive_items(cx, mode, items.mixed(), true) {
@@ -44,6 +56,8 @@ impl<S: Stage> AttributeParser<S> for OnMoveParser {
             this.parse(cx, args, Mode::DiagnosticOnMove);
         },
     )];
+
+    // "Allowed" for all targets but noop if used on not-adt.
     const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS);
 
     fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> {

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unimplemented.rs

@@ -1,7 +1,10 @@
+use rustc_errors::Diagnostic;
 use rustc_hir::attrs::diagnostic::Directive;
+use rustc_session::lint::builtin::MISPLACED_DIAGNOSTIC_ATTRIBUTES;
 
 use crate::attributes::diagnostic::*;
 use crate::attributes::prelude::*;
+use crate::errors::DiagnosticOnUnimplementedOnlyForTraits;
 
 #[derive(Default)]
 pub(crate) struct OnUnimplementedParser {
@@ -19,11 +22,12 @@ impl OnUnimplementedParser {
         let span = cx.attr_span;
         self.span = Some(span);
 
-        // If target is not a trait, returning early will make `finalize` emit a
-        // `AttributeKind::OnUnimplemented {span, directive: None }`, to prevent it being
-        // accidentally used on non-trait items like trait aliases.
         if !matches!(cx.target, Target::Trait) {
-            // Lint later emitted in check_attr
+            cx.emit_dyn_lint(
+                MISPLACED_DIAGNOSTIC_ATTRIBUTES,
+                move |dcx, level| DiagnosticOnUnimplementedOnlyForTraits.into_diag(dcx, level),
+                span,
+            );
             return;
         }
 

compiler/rustc_attr_parsing/src/attributes/diagnostic/on_unknown.rs

@@ -1,7 +1,11 @@
+use rustc_errors::Diagnostic;
 use rustc_hir::attrs::diagnostic::Directive;
+use rustc_session::lint::builtin::MISPLACED_DIAGNOSTIC_ATTRIBUTES;
 
+use crate::ShouldEmit;
 use crate::attributes::diagnostic::*;
 use crate::attributes::prelude::*;
+use crate::errors::DiagnosticOnUnknownOnlyForImports;
 
 #[derive(Default)]
 pub(crate) struct OnUnknownParser {
@@ -25,6 +29,22 @@ impl OnUnknownParser {
         let span = cx.attr_span;
         self.span = Some(span);
 
+        // At early parsing we get passed `Target::Crate` regardless of the item we're on.
+        // Only do target checking if we're late.
+        let early = matches!(cx.stage.should_emit(), ShouldEmit::Nothing);
+
+        if !early && !matches!(cx.target, Target::Use) {
+            let target_span = cx.target_span;
+            cx.emit_dyn_lint(
+                MISPLACED_DIAGNOSTIC_ATTRIBUTES,
+                move |dcx, level| {
+                    DiagnosticOnUnknownOnlyForImports { target_span }.into_diag(dcx, level)
+                },
+                span,
+            );
+            return;
+        }
+
         let Some(items) = parse_list(cx, args, mode) else { return };
 
         if let Some(directive) = parse_directive_items(cx, mode, items.mixed(), true) {
@@ -41,7 +61,7 @@ impl<S: Stage> AttributeParser<S> for OnUnknownParser {
             this.parse(cx, args, Mode::DiagnosticOnUnknown);
         },
     )];
-    //FIXME attribute is not parsed for non-use statements but diagnostics are issued in `check_attr.rs`
+    // "Allowed" for all targets, but noop for all but use statements.
     const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS);
 
     fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> {

compiler/rustc_attr_parsing/src/errors.rs

@@ -252,3 +252,32 @@ pub(crate) struct DocUnknownAny {
 #[derive(Diagnostic)]
 #[diag("expected boolean for `#[doc(auto_cfg = ...)]`")]
 pub(crate) struct DocAutoCfgWrongLiteral;
+
+#[derive(Diagnostic)]
+#[diag("`#[diagnostic::on_const]` can only be applied to non-const trait implementations")]
+pub(crate) struct DiagnosticOnConstOnlyForTraitImpls {
+    #[label("not a trait implementation")]
+    pub target_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag("`#[diagnostic::on_move]` can only be applied to enums, structs or unions")]
+pub(crate) struct DiagnosticOnMoveOnlyForAdt;
+
+#[derive(Diagnostic)]
+#[diag("`#[diagnostic::on_unimplemented]` can only be applied to trait definitions")]
+pub(crate) struct DiagnosticOnUnimplementedOnlyForTraits;
+
+#[derive(Diagnostic)]
+#[diag("`#[diagnostic::on_unknown]` can only be applied to `use` statements")]
+pub(crate) struct DiagnosticOnUnknownOnlyForImports {
+    #[label("not an import")]
+    pub target_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag("`#[diagnostic::do_not_recommend]` can only be placed on trait implementations")]
+pub(crate) struct IncorrectDoNotRecommendLocation {
+    #[label("not a trait implementation")]
+    pub target_span: Span,
+}