Permit {This} in diagnostic attribute format literals

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

@@ -6,7 +6,6 @@ use rustc_hir::attrs::diagnostic::{
     Directive, FilterFormatString, Flag, FormatArg, FormatString, LitOrArg, Name, NameValue,
     OnUnimplementedCondition, Piece, Predicate,
 };
-use rustc_hir::lints::FormatWarning;
 use rustc_macros::Diagnostic;
 use rustc_parse_format::{
     Argument, FormatSpec, ParseError, ParseMode, Parser, Piece as RpfPiece, Position,
@@ -19,9 +18,8 @@ use thin_vec::{ThinVec, thin_vec};
 
 use crate::context::{AcceptContext, Stage};
 use crate::errors::{
-    DisallowedPlaceholder, DisallowedPositionalArgument, IgnoredDiagnosticOption,
-    InvalidFormatSpecifier, MalFormedDiagnosticAttributeLint, MissingOptionsForDiagnosticAttribute,
-    NonMetaItemDiagnosticAttribute, WrappedParserError,
+    FormatWarning, IgnoredDiagnosticOption, MalFormedDiagnosticAttributeLint,
+    MissingOptionsForDiagnosticAttribute, NonMetaItemDiagnosticAttribute, WrappedParserError,
 };
 use crate::parser::{ArgParser, MetaItemListParser, MetaItemOrLitParser, MetaItemParser};
 
@@ -88,6 +86,29 @@ impl Mode {
             Self::DiagnosticOnUnmatchArgs => DEFAULT,
         }
     }
+
+    fn allowed_format_arguments(&self) -> &'static str {
+        match self {
+            Self::RustcOnUnimplemented => {
+                "see <https://rustc-dev-guide.rust-lang.org/diagnostics.html#rustc_on_unimplemented> for allowed format arguments"
+            }
+            Self::DiagnosticOnUnimplemented => {
+                "only `Self` and generics of the trait are allowed as a format argument"
+            }
+            Self::DiagnosticOnConst => {
+                "only `Self` and generics of the implementation are allowed as a format argument"
+            }
+            Self::DiagnosticOnMove => {
+                "only `This`, `Self` and generics of the type are allowed as a format argument"
+            }
+            Self::DiagnosticOnUnknown => {
+                "only `This` is allowed as a format argument, referring to the failed import"
+            }
+            Self::DiagnosticOnUnmatchArgs => {
+                "only `This` is allowed as a format argument, referring to the macro's name"
+            }
+        }
+    }
 }
 
 fn merge_directives<S: Stage>(
@@ -257,22 +278,13 @@ fn parse_directive_items<'p, S: Stage>(
             match parse_format_string(input.name, snippet, input.span, mode) {
                 Ok((f, warnings)) => {
                     for warning in warnings {
-                        let (FormatWarning::InvalidSpecifier { span, .. }
-                        | FormatWarning::PositionalArgument { span, .. }
-                        | FormatWarning::DisallowedPlaceholder { span }) = warning;
+                        let (FormatWarning::InvalidSpecifier { span }
+                        | FormatWarning::PositionalArgument { span }
+                        | FormatWarning::IndexedArgument { span }
+                        | FormatWarning::DisallowedPlaceholder { span, .. }) = warning;
                         cx.emit_dyn_lint(
                             MALFORMED_DIAGNOSTIC_FORMAT_LITERALS,
-                            move |dcx, level| match warning {
-                                FormatWarning::PositionalArgument { .. } => {
-                                    DisallowedPositionalArgument.into_diag(dcx, level)
-                                }
-                                FormatWarning::InvalidSpecifier { .. } => {
-                                    InvalidFormatSpecifier.into_diag(dcx, level)
-                                }
-                                FormatWarning::DisallowedPlaceholder { .. } => {
-                                    DisallowedPlaceholder.into_diag(dcx, level)
-                                }
-                            },
+                            move |dcx, level| warning.into_diag(dcx, level),
                             span,
                         );
                     }
@@ -422,38 +434,74 @@ fn parse_arg(
     is_source_literal: bool,
 ) -> FormatArg {
     let span = slice_span(input_span, arg.position_span.clone(), is_source_literal);
-    if matches!(mode, Mode::DiagnosticOnUnknown) {
-        warnings.push(FormatWarning::DisallowedPlaceholder { span });
-        return FormatArg::AsIs(sym::empty_braces);
-    }
 
     match arg.position {
         // Something like "hello {name}"
         Position::ArgumentNamed(name) => match (mode, Symbol::intern(name)) {
-            // Only `#[rustc_on_unimplemented]` can use these
-            (Mode::RustcOnUnimplemented { .. }, sym::ItemContext) => FormatArg::ItemContext,
-            (Mode::RustcOnUnimplemented { .. } | Mode::DiagnosticOnUnmatchArgs, sym::This) => {
-                FormatArg::This
+            (Mode::RustcOnUnimplemented, sym::ItemContext) => FormatArg::ItemContext,
+
+            // Like `{This}`, but sugared.
+            // FIXME(mejrs) maybe rename/rework this or something
+            // if we want to apply this to other attrs?
+            (Mode::RustcOnUnimplemented, sym::Trait) => FormatArg::Trait,
+
+            // Some diagnostic attributes can use `{This}` to refer to the annotated item.
+            // For those that don't, we continue and maybe use it as a generic parameter.
+            //
+            // FIXME(mejrs) `DiagnosticOnUnimplemented` is intentionally not here;
+            // that requires lang approval which is best kept for a standalone PR.
+            (
+                Mode::RustcOnUnimplemented
+                | Mode::DiagnosticOnUnknown
+                | Mode::DiagnosticOnMove
+                | Mode::DiagnosticOnUnmatchArgs,
+                sym::This,
+            ) => FormatArg::This,
+
+            // `{Self}`; the self type.
+            // - For trait declaration attributes that's the type that does not implement it.
+            // - for trait impl attributes, the implemented for type.
+            // - For ADT attributes, that's the type (which will be identical to `{This}`)
+            // - For everything else it doesn't make sense.
+            (
+                Mode::RustcOnUnimplemented
+                | Mode::DiagnosticOnUnimplemented
+                | Mode::DiagnosticOnMove
+                | Mode::DiagnosticOnConst,
+                kw::SelfUpper,
+            ) => FormatArg::SelfUpper,
+
+            // Generic parameters.
+            // FIXME(mejrs) unfortunately, all the "special" symbols above might fall through,
+            // but at this time we are not aware of what generic parameters the trait actually has.
+            // If we find `ItemContext` or something we have to assume that's a generic parameter.
+            // We lint against that in `check_attr.rs` though.
+            (
+                Mode::RustcOnUnimplemented
+                | Mode::DiagnosticOnUnimplemented
+                | Mode::DiagnosticOnMove
+                | Mode::DiagnosticOnConst,
+                generic_param,
+            ) => FormatArg::GenericParam { generic_param, span },
+
+            // Generics are explicitly not allowed, we print those back as is.
+            (Mode::DiagnosticOnUnknown | Mode::DiagnosticOnUnmatchArgs, as_is) => {
+                warnings.push(FormatWarning::DisallowedPlaceholder {
+                    span,
+                    attr: mode.as_str(),
+                    allowed: mode.allowed_format_arguments(),
+                });
+                return FormatArg::AsIs(Symbol::intern(&format!("{{{as_is}}}")));
             }
-            (Mode::RustcOnUnimplemented { .. }, sym::Trait) => FormatArg::Trait,
-            // Any attribute can use these
-            (_, kw::SelfUpper) => FormatArg::SelfUpper,
-            (_, generic_param) => FormatArg::GenericParam { generic_param, span },
         },
 
         // `{:1}` and `{}` are ignored
         Position::ArgumentIs(idx) => {
-            warnings.push(FormatWarning::PositionalArgument {
-                span,
-                help: format!("use `{{{idx}}}` to print a number in braces"),
-            });
+            warnings.push(FormatWarning::IndexedArgument { span });
             FormatArg::AsIs(Symbol::intern(&format!("{{{idx}}}")))
         }
         Position::ArgumentImplicitlyIs(_) => {
-            warnings.push(FormatWarning::PositionalArgument {
-                span,
-                help: String::from("use `{{}}` to print empty braces"),
-            });
+            warnings.push(FormatWarning::PositionalArgument { span });
             FormatArg::AsIs(sym::empty_braces)
         }
     }
@@ -473,7 +521,7 @@ fn warn_on_format_spec(
             .as_ref()
             .map(|inner| slice_span(input_span, inner.clone(), is_source_literal))
             .unwrap_or(input_span);
-        warnings.push(FormatWarning::InvalidSpecifier { span, name: spec.ty.into() })
+        warnings.push(FormatWarning::InvalidSpecifier { span })
     }
 }
 

compiler/rustc_attr_parsing/src/errors.rs

@@ -357,23 +357,6 @@ pub(crate) struct MalFormedDiagnosticAttributeLint {
     pub span: Span,
 }
 
-#[derive(Diagnostic)]
-#[diag("positional format arguments are not allowed here")]
-#[help(
-    "only named format arguments with the name of one of the generic types are allowed in this context"
-)]
-pub(crate) struct DisallowedPositionalArgument;
-
-#[derive(Diagnostic)]
-#[diag("format arguments are not allowed here")]
-#[help("consider removing this format argument")]
-pub(crate) struct DisallowedPlaceholder;
-
-#[derive(Diagnostic)]
-#[diag("invalid format specifier")]
-#[help("no format specifier are supported in this position")]
-pub(crate) struct InvalidFormatSpecifier;
-
 #[derive(Diagnostic)]
 #[diag("{$description}")]
 pub(crate) struct WrappedParserError<'a> {
@@ -407,3 +390,34 @@ pub(crate) struct MissingOptionsForDiagnosticAttribute {
     "only literals are allowed as values for the `message`, `note` and `label` options. These options must be separated by a comma"
 )]
 pub(crate) struct NonMetaItemDiagnosticAttribute;
+
+#[derive(Diagnostic, Clone, Copy)]
+pub(crate) enum FormatWarning {
+    #[diag("positional arguments are not permitted in diagnostic attributes")]
+    #[help("you can print empty braces by escaping them")]
+    PositionalArgument {
+        #[label("remove this format argument")]
+        span: Span,
+    },
+
+    #[diag("indexed format arguments are not permitted in diagnostic attributes")]
+    IndexedArgument {
+        #[label("remove this format argument")]
+        span: Span,
+    },
+
+    #[diag("format specifiers are not permitted in diagnostic attributes")]
+    InvalidSpecifier {
+        #[label("remove this format specifier")]
+        span: Span,
+    },
+
+    #[diag("this format argument is not allowed in `#[{$attr}]`")]
+    #[note("{$allowed}")]
+    DisallowedPlaceholder {
+        #[label("remove this format argument")]
+        span: Span,
+        attr: &'static str,
+        allowed: &'static str,
+    },
+}

compiler/rustc_hir/src/lints.rs

@@ -1,8 +1,8 @@
 use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_error_messages::MultiSpan;
 use rustc_errors::{Diag, DiagCtxtHandle, Level};
+pub use rustc_lint_defs::AttributeLintKind;
 use rustc_lint_defs::LintId;
-pub use rustc_lint_defs::{AttributeLintKind, FormatWarning};
 
 use crate::HirId;
 

compiler/rustc_lint_defs/src/lib.rs

@@ -658,13 +658,6 @@ pub enum AttributeLintKind {
     UnexpectedCfgValue((Symbol, Span), Option<(Symbol, Span)>),
 }
 
-#[derive(Debug, Clone, HashStable_Generic)]
-pub enum FormatWarning {
-    PositionalArgument { span: Span, help: String },
-    InvalidSpecifier { name: String, span: Span },
-    DisallowedPlaceholder { span: Span },
-}
-
 pub type RegisteredTools = FxIndexSet<Ident>;
 
 /// Declares a static item of type `&'static Lint`.

compiler/rustc_passes/src/check_attr.rs

@@ -201,9 +201,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                 },
                 Attribute::Parsed(AttributeKind::OnUnimplemented{directive,..}) => {self.check_diagnostic_on_unimplemented(hir_id, directive.as_deref())},
                 Attribute::Parsed(AttributeKind::OnConst{span, ..}) => {self.check_diagnostic_on_const(*span, hir_id, target, item)},
-                Attribute::Parsed(AttributeKind::OnUnmatchArgs { directive, .. }) => {
-                    self.check_diagnostic_on_unmatch_args(hir_id, directive.as_deref())
-                },
                 Attribute::Parsed(AttributeKind::OnMove { directive , .. }) => {
                     self.check_diagnostic_on_move(hir_id, directive.as_deref())
                 },
@@ -256,6 +253,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                     | AttributeKind::NoMangle(..)
                     | AttributeKind::NoStd { .. }
                     | AttributeKind::OnUnknown { .. }
+                    | AttributeKind::OnUnmatchArgs { .. }
                     | AttributeKind::Optimize(..)
                     | AttributeKind::PanicRuntime
                     | AttributeKind::PatchableFunctionEntry { .. }
@@ -562,20 +560,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         // ...whose generics would that be, anyway? The traits' or the impls'?
     }
 
-    /// Checks use of generic formatting parameters in `#[diagnostic::on_unmatch_args]`.
-    fn check_diagnostic_on_unmatch_args(&self, hir_id: HirId, directive: Option<&Directive>) {
-        if let Some(directive) = directive {
-            directive.visit_params(&mut |argument_name, span| {
-                self.tcx.emit_node_span_lint(
-                    MALFORMED_DIAGNOSTIC_FORMAT_LITERALS,
-                    hir_id,
-                    span,
-                    errors::OnUnmatchArgsMalformedFormatLiterals { name: argument_name },
-                )
-            });
-        }
-    }
-
     /// Checks use of generic formatting parameters in `#[diagnostic::on_move]`
     fn check_diagnostic_on_move(&self, hir_id: HirId, directive: Option<&Directive>) {
         if let Some(directive) = directive {

compiler/rustc_passes/src/errors.rs

@@ -1303,10 +1303,3 @@ pub(crate) struct UnknownFormatParameterForOnUnimplementedAttr {
 pub(crate) struct OnMoveMalformedFormatLiterals {
     pub name: Symbol,
 }
-
-#[derive(Diagnostic)]
-#[diag("unknown parameter `{$name}`")]
-#[help(r#"use {"`{This}`"} to refer to the macro name"#)]
-pub(crate) struct OnUnmatchArgsMalformedFormatLiterals {
-    pub name: Symbol,
-}

compiler/rustc_resolve/src/imports.rs

@@ -2,6 +2,7 @@
 
 use std::mem;
 
+use itertools::Itertools;
 use rustc_ast::{Item, NodeId};
 use rustc_attr_parsing::AttributeParser;
 use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
@@ -879,8 +880,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         let (message, label, notes) =
             // Feature gating for `on_unknown_attr` happens initialization of the field
             if let Some(directive) = errors[0].1.on_unknown_attr.as_ref().map(|a| &a.directive) {
+                let this = errors.iter().map(|(_import, err)| {
+
+                    // Is this unwrap_or reachable?
+                    err.segment.unwrap_or(kw::Underscore)
+                }).join(", ");
+
                 let args = FormatArgs {
-                    this: paths.join(", "),
+                    this,
                     // Unused
                     this_sugared: String::new(),
                     // Unused

compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs

@@ -4,7 +4,6 @@ use rustc_hir as hir;
 use rustc_hir::attrs::diagnostic::{ConditionOptions, CustomDiagnostic, FormatArgs};
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::find_attr;
-pub use rustc_hir::lints::FormatWarning;
 use rustc_middle::ty::print::PrintTraitRefExt;
 use rustc_middle::ty::{self, GenericParamDef, GenericParamDefKind};
 use rustc_span::Symbol;

tests/ui/diagnostic_namespace/multiline_spans.rs

@@ -31,17 +31,17 @@ pub trait MultiLine4 {}
 #[diagnostic::on_unimplemented(message = "here is a big \
                                          multiline string \
                                          {Self:+}")]
-//~^ ERROR invalid format specifier [malformed_diagnostic_format_literals]
+//~^ ERROR format specifiers are not permitted in diagnostic attributes [malformed_diagnostic_format_literals]
 pub trait MultiLineFmt {}
 
 #[diagnostic::on_unimplemented(message = "here is a big \
                                          multiline string {Self:X}")]
-//~^ ERROR invalid format specifier [malformed_diagnostic_format_literals]
+//~^ ERROR format specifiers are not permitted in diagnostic attributes [malformed_diagnostic_format_literals]
 pub trait MultiLineFmt2 {}
 
 #[diagnostic::on_unimplemented(message = "here is a big \
     multiline string {Self:#}")]
-//~^ ERROR invalid format specifier [malformed_diagnostic_format_literals]
+//~^ ERROR format specifiers are not permitted in diagnostic attributes [malformed_diagnostic_format_literals]
 pub trait MultiLineFmt3 {}
 
 
@@ -51,5 +51,5 @@ pub trait MultiLineFmt3 {}
                                 \
                                                 \
     multiline string {Self:?}")]
-//~^ ERROR invalid format specifier [malformed_diagnostic_format_literals]
+//~^ ERROR format specifiers are not permitted in diagnostic attributes [malformed_diagnostic_format_literals]
 pub trait MultiLineFmt4 {}