Permit {This} in diagnostic attribute format literals

Author: mejrs

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

@@ -1,12 +1,12 @@
 use std::ops::Range;
 
-use rustc_errors::E0232;
+use rustc_errors::{Diagnostic, E0232};
 use rustc_hir::AttrPath;
 use rustc_hir::attrs::diagnostic::{
     Directive, FilterFormatString, Flag, FormatArg, FormatString, LitOrArg, Name, NameValue,
     OnUnimplementedCondition, Piece, Predicate,
 };
-use rustc_hir::lints::{AttributeLintKind, FormatWarning};
+use rustc_hir::lints::AttributeLintKind;
 use rustc_macros::Diagnostic;
 use rustc_parse_format::{
     Argument, FormatSpec, ParseError, ParseMode, Parser, Piece as RpfPiece, Position,
@@ -18,6 +18,7 @@ use rustc_span::{Ident, InnerSpan, Span, Symbol, kw, sym};
 use thin_vec::{ThinVec, thin_vec};
 
 use crate::context::{AcceptContext, Stage};
+use crate::errors::FormatWarning;
 use crate::parser::{ArgParser, MetaItemListParser, MetaItemOrLitParser, MetaItemParser};
 
 pub(crate) mod do_not_recommend;
@@ -83,6 +84,26 @@ 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 | Self::DiagnosticOnUnmatchArgs => {
+                "only `This` is allowed as a format argument"
+            }
+        }
+    }
 }
 
 fn merge_directives<S: Stage>(
@@ -242,12 +263,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;
-                        cx.emit_lint(
+                        let (FormatWarning::InvalidSpecifier { span }
+                        | FormatWarning::PositionalArgument { span }
+                        | FormatWarning::IndexedArgument { span }
+                        | FormatWarning::DisallowedPlaceholder { span, .. }) = warning;
+                        cx.emit_dyn_lint(
                             MALFORMED_DIAGNOSTIC_FORMAT_LITERALS,
-                            AttributeLintKind::MalformedDiagnosticFormat { warning },
+                            move |dcx, level| warning.into_diag(dcx, level),
                             span,
                         );
                     }
@@ -394,38 +416,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)
         }
     }
@@ -445,7 +503,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

@@ -325,3 +325,34 @@ pub(crate) struct IncorrectDoNotRecommendLocation {
     #[label("not a trait implementation")]
     pub target_span: Span,
 }
+
+#[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/src/early/diagnostics.rs

@@ -2,7 +2,7 @@ use std::any::Any;
 
 use rustc_data_structures::sync::DynSend;
 use rustc_errors::{Diag, DiagCtxtHandle, Diagnostic, Level};
-use rustc_hir::lints::{AttributeLintKind, FormatWarning};
+use rustc_hir::lints::AttributeLintKind;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::Session;
 
@@ -52,17 +52,6 @@ impl<'a> Diagnostic<'a, ()> for DecorateAttrLint<'_, '_, '_> {
                 lints::MalFormedDiagnosticAttributeLint { attribute, options, span }
                     .into_diag(dcx, level)
             }
-            AttributeLintKind::MalformedDiagnosticFormat { warning } => match warning {
-                FormatWarning::PositionalArgument { .. } => {
-                    lints::DisallowedPositionalArgument.into_diag(dcx, level)
-                }
-                FormatWarning::InvalidSpecifier { .. } => {
-                    lints::InvalidFormatSpecifier.into_diag(dcx, level)
-                }
-                FormatWarning::DisallowedPlaceholder { .. } => {
-                    lints::DisallowedPlaceholder.into_diag(dcx, level)
-                }
-            },
             AttributeLintKind::DiagnosticWrappedParserError { description, label, span } => {
                 lints::WrappedParserError { description, label, span: *span }.into_diag(dcx, level)
             }

compiler/rustc_lint/src/lints.rs

@@ -3303,23 +3303,6 @@ pub(crate) struct ExpectedNoArgs;
 )]
 pub(crate) struct ExpectedNameValue;
 
-#[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> {

compiler/rustc_lint_defs/src/lib.rs

@@ -660,20 +660,12 @@ pub enum AttributeLintKind {
     ExpectedNoArgs,
     ExpectedNameValue,
     MalFormedDiagnosticAttribute { attribute: &'static str, options: &'static str, span: Span },
-    MalformedDiagnosticFormat { warning: FormatWarning },
     DiagnosticWrappedParserError { description: String, label: String, span: Span },
     IgnoredDiagnosticOption { option_name: Symbol, first_span: Span, later_span: Span },
     MissingOptionsForDiagnosticAttribute { attribute: &'static str, options: &'static str },
     NonMetaItemDiagnosticAttribute,
 }
 
-#[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_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,12 @@ 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)| {
+                    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 {}

tests/ui/diagnostic_namespace/multiline_spans.stderr

@@ -36,37 +36,29 @@ LL |     multiline string {unknown}")]
    |
    = help: expect either a generic argument name or `{Self}` as format argument
 
-error: invalid format specifier
+error: format specifiers are not permitted in diagnostic attributes
   --> $DIR/multiline_spans.rs:33:47
    |
 LL | ...                   {Self:+}")]
-   |                            ^^
-   |
-   = help: no format specifier are supported in this position
+   |                            ^^ remove this format specifier
 
-error: invalid format specifier
+error: format specifiers are not permitted in diagnostic attributes
   --> $DIR/multiline_spans.rs:38:64
    |
 LL | ...                   multiline string {Self:X}")]
-   |                                             ^^
-   |
-   = help: no format specifier are supported in this position
+   |                                             ^^ remove this format specifier
 
-error: invalid format specifier
+error: format specifiers are not permitted in diagnostic attributes
   --> $DIR/multiline_spans.rs:43:27
    |
 LL |     multiline string {Self:#}")]
-   |                           ^^
-   |
-   = help: no format specifier are supported in this position
+   |                           ^^ remove this format specifier
 
-error: invalid format specifier
+error: format specifiers are not permitted in diagnostic attributes
   --> $DIR/multiline_spans.rs:53:27
    |
 LL |     multiline string {Self:?}")]
-   |                           ^^
-   |
-   = help: no format specifier are supported in this position
+   |                           ^^ remove this format specifier
 
 error: aborting due to 8 previous errors