rustc_on_unimplemented: introduce format specifiers as printing options for the annotated item.

Author: mejrs

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

@@ -396,9 +396,7 @@ pub(crate) fn parse_format_string(
         .map(|piece| match piece {
             RpfPiece::Lit(lit) => Piece::Lit(Symbol::intern(lit)),
             RpfPiece::NextArgument(arg) => {
-                warn_on_format_spec(&arg.format, &mut warnings, span, parser.is_source_literal);
-                let arg = parse_arg(&arg, mode, &mut warnings, span, parser.is_source_literal);
-                Piece::Arg(arg)
+                Piece::Arg(parse_arg(&arg, mode, &mut warnings, span, parser.is_source_literal))
             }
         })
         .collect();
@@ -415,26 +413,33 @@ fn parse_arg(
 ) -> FormatArg {
     let span = slice_span(input_span, arg.position_span.clone(), is_source_literal);
 
-    match arg.position {
+    let mut check_format = true;
+
+    let ret = match arg.position {
         // Something like "hello {name}"
         Position::ArgumentNamed(name) => match (mode, Symbol::intern(name)) {
             (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,
+            // `{This:ty}`
+            (Mode::RustcOnUnimplemented, sym::This) => match arg.format.ty {
+                "resolved" => {
+                    check_format = false;
+                    FormatArg::ThisResolved
+                }
+                "path" => {
+                    check_format = false;
+                    FormatArg::ThisPath
+                }
+                _ => FormatArg::This,
+            },
 
             // 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,
+                Mode::DiagnosticOnUnknown | Mode::DiagnosticOnMove | Mode::DiagnosticOnUnmatchArgs,
                 sym::This,
             ) => FormatArg::This,
 
@@ -471,11 +476,11 @@ fn parse_arg(
                     attr: mode.as_str(),
                     allowed: mode.allowed_format_arguments(),
                 });
-                return FormatArg::AsIs(Symbol::intern(&format!("{{{as_is}}}")));
+                FormatArg::AsIs(Symbol::intern(&format!("{{{as_is}}}")))
             }
         },
 
-        // `{:1}` and `{}` are ignored
+        // `{1}` and `{}` are ignored
         Position::ArgumentIs(idx) => {
             warnings.push(FormatWarning::IndexedArgument { span });
             FormatArg::AsIs(Symbol::intern(&format!("{{{idx}}}")))
@@ -484,7 +489,11 @@ fn parse_arg(
             warnings.push(FormatWarning::PositionalArgument { span });
             FormatArg::AsIs(sym::empty_braces)
         }
+    };
+    if check_format {
+        warn_on_format_spec(&arg.format, warnings, input_span, is_source_literal);
     }
+    ret
 }
 
 /// `#[rustc_on_unimplemented]` and `#[diagnostic::...]` don't actually do anything
@@ -495,12 +504,8 @@ fn warn_on_format_spec(
     input_span: Span,
     is_source_literal: bool,
 ) {
-    if spec.ty != "" {
-        let span = spec
-            .ty_span
-            .as_ref()
-            .map(|inner| slice_span(input_span, inner.clone(), is_source_literal))
-            .unwrap_or(input_span);
+    if let Some(ty_span) = &spec.ty_span {
+        let span = slice_span(input_span, ty_span.clone(), is_source_literal);
         warnings.push(FormatWarning::InvalidSpecifier { span })
     }
 }

compiler/rustc_hir/src/attrs/diagnostic.rs

@@ -147,11 +147,12 @@ impl FormatString {
                     };
                     ret.push_str(&slf);
                 }
+                Piece::Arg(FormatArg::This) => ret.push_str(&args.this),
 
                 // It's only `rustc_onunimplemented` from here
-                Piece::Arg(FormatArg::This) => ret.push_str(&args.this),
-                Piece::Arg(FormatArg::Trait) => {
-                    let _ = fmt::write(&mut ret, format_args!("{}", &args.this_sugared));
+                Piece::Arg(FormatArg::ThisPath) => ret.push_str(&args.this_path),
+                Piece::Arg(FormatArg::ThisResolved) => {
+                    let _ = fmt::write(&mut ret, format_args!("{}", &args.this_resolved));
                 }
                 Piece::Arg(FormatArg::ItemContext) => ret.push_str(args.item_context),
             }
@@ -197,7 +198,7 @@ impl FormatString {
 /// ```rust,ignore (just an example)
 /// FormatArgs {
 ///     this: "FromResidual",
-///     this_sugared: "FromResidual<Option<Infallible>>",
+///     this_resolved: "FromResidual<Option<Infallible>>",
 ///     item_context: "an async function",
 ///     generic_args: [("Self", "u32"), ("R", "Option<Infallible>")],
 /// }
@@ -206,7 +207,8 @@ impl FormatString {
 pub struct FormatArgs {
     /// The name of the item the attribute is on.
     pub this: String,
-    pub this_sugared: String = String::new(),
+    pub this_resolved: String = String::new(),
+    pub this_path: String = String::new(),
     pub item_context: &'static str = "",
     pub generic_args: Vec<(Symbol, String)> = Vec::new(),
 }
@@ -226,10 +228,12 @@ pub enum FormatArg {
     },
     // `{Self}`
     SelfUpper,
-    /// `{This}` or `{TraitName}`
+    /// `{This}` and `{This:name}`.
     This,
-    /// The sugared form of the trait
-    Trait,
+    /// The sugared form: `{This:sugared}`.
+    ThisResolved,
+    /// The full path: `{This:path}`.
+    ThisPath,
     /// what we're in, like a function, method, closure etc.
     ItemContext,
     /// What the user typed, if it doesn't match anything we can use.

compiler/rustc_parse_format/src/lib.rs

@@ -750,15 +750,15 @@ impl<'input> Parser<'input> {
         spec
     }
 
-    /// Always returns an empty `FormatSpec`
+    /// Always returns an empty `FormatSpec`, except for the `ty` and `ty_span` fields.
     fn diagnostic(&mut self) -> FormatSpec<'input> {
         let mut spec = FormatSpec::default();
 
-        let Some((Range { start, .. }, start_idx)) = self.consume_pos(':') else {
+        let Some((Range { start, .. }, _)) = self.consume_pos(':') else {
             return spec;
         };
 
-        spec.ty = self.string(start_idx);
+        spec.ty = self.string(self.input_vec_index);
         spec.ty_span = {
             let end = self.input_vec_index2range(self.input_vec_index).start;
             Some(start..end)

compiler/rustc_span/src/symbol.rs

@@ -317,7 +317,6 @@ symbols! {
         Target,
         This,
         TokenStream,
-        Trait,
         TrivialClone,
         Try,
         TryCaptureGeneric,

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

@@ -217,7 +217,9 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         }));
 
         let this = self.tcx.def_path_str(trait_pred.trait_ref.def_id);
-        let this_sugared = trait_pred.trait_ref.print_trait_sugared().to_string();
+        let this_resolved = trait_pred.trait_ref.print_trait_sugared().to_string();
+        let this_path =
+            ty::TraitRef::identity(self.tcx, def_id).print_only_trait_path().to_string();
 
         let filter_options =
             FilterOptions { self_types, from_desugaring, cause, crate_local, direct, generic_args };
@@ -249,7 +251,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             })
             .collect();
 
-        let format_args = FormatArgs { this, this_sugared, generic_args, item_context };
+        let format_args = FormatArgs { this, this_path, this_resolved, generic_args, item_context };
         (filter_options, format_args)
     }
 }

library/core/src/ops/function.rs

@@ -67,8 +67,8 @@ use crate::marker::Tuple;
         // SAFETY: tidy is not smart enough to tell that the below unsafe block is a string
         label = "call the function in a closure: `|| unsafe {{ /* code */ }}`"
     ),
-    message = "expected a `{Trait}` closure, found `{Self}`",
-    label = "expected an `{Trait}` closure, found `{Self}`"
+    message = "expected a `{This:resolved}` closure, found `{Self}`",
+    label = "expected an `{This:resolved}` closure, found `{Self}`"
 )]
 #[fundamental] // so that regex can rely that `&str: !FnMut`
 #[must_use = "closures are lazy and do nothing unless called"]
@@ -154,8 +154,8 @@ pub const trait Fn<Args: Tuple>: [const] FnMut<Args> {
         // SAFETY: tidy is not smart enough to tell that the below unsafe block is a string
         label = "call the function in a closure: `|| unsafe {{ /* code */ }}`"
     ),
-    message = "expected a `{Trait}` closure, found `{Self}`",
-    label = "expected an `{Trait}` closure, found `{Self}`"
+    message = "expected a `{This:resolved}` closure, found `{Self}`",
+    label = "expected an `{This:resolved}` closure, found `{Self}`"
 )]
 #[fundamental] // so that regex can rely that `&str: !FnMut`
 #[must_use = "closures are lazy and do nothing unless called"]
@@ -233,8 +233,8 @@ pub const trait FnMut<Args: Tuple>: FnOnce<Args> {
         // SAFETY: tidy is not smart enough to tell that the below unsafe block is a string
         label = "call the function in a closure: `|| unsafe {{ /* code */ }}`"
     ),
-    message = "expected a `{Trait}` closure, found `{Self}`",
-    label = "expected an `{Trait}` closure, found `{Self}`"
+    message = "expected a `{This:resolved}` closure, found `{Self}`",
+    label = "expected an `{This:resolved}` closure, found `{Self}`"
 )]
 #[fundamental] // so that regex can rely that `&str: !FnMut`
 #[must_use = "closures are lazy and do nothing unless called"]

src/doc/rustc-dev-guide/src/diagnostics.md

@@ -1018,12 +1018,13 @@ pub trait From<T>: Sized {
 ### Formatting
 
 The string literals are format strings that accept parameters wrapped in braces
-but positional and listed parameters and format specifiers are not accepted.
+but positional and listed parameters are not accepted.
 The following parameter names are valid:
 - `Self` and all generic parameters of the trait.
 - `This`: the name of the trait the attribute is on, without generics.
-- `Trait`: the name of the "sugared" trait.
-  See `TraitRefPrintSugared`.
+- `This:path`: the full path of the trait the attribute is on, with unresolved generics.
+- `This:resolved`: the full path of the trait the attribute is on, with resolved generics.
+Additionally, this will "sugar" the `Fn(...)` traits.
 - `ItemContext`: the kind of `hir::Node` we're in, things like `"an async block"`,
    `"a function"`, `"an async function"`, etc.
 

tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr

@@ -16,18 +16,18 @@ LL | #[diagnostic::on_unimplemented(message = "Test {}")]
    |
    = help: you can print empty braces by escaping them
 
-warning: format specifiers are not permitted in diagnostic attributes
-  --> $DIR/broken_format.rs:10:50
-   |
-LL | #[diagnostic::on_unimplemented(message = "Test {1:}")]
-   |                                                  ^ remove this format specifier
-
 warning: indexed format arguments are not permitted in diagnostic attributes
   --> $DIR/broken_format.rs:10:49
    |
 LL | #[diagnostic::on_unimplemented(message = "Test {1:}")]
    |                                                 ^ remove this format argument
 
+warning: format specifiers are not permitted in diagnostic attributes
+  --> $DIR/broken_format.rs:10:50
+   |
+LL | #[diagnostic::on_unimplemented(message = "Test {1:}")]
+   |                                                  ^ remove this format specifier
+
 warning: format specifiers are not permitted in diagnostic attributes
   --> $DIR/broken_format.rs:15:53
    |

tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs

@@ -35,13 +35,16 @@ impl Bar for i32 {}
     //~|WARN there is no parameter `r#struct` on trait `Baz`
     //~|WARN there is no parameter `r#enum` on trait `Baz`
     //~|WARN there is no parameter `union` on trait `Baz`
-    label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}"
+    label = "{float}{_Self}{crate_local}{This}{ItemContext}{This:path}{This:resolved}"
     //~^WARN there is no parameter `float` on trait `Baz`
     //~|WARN there is no parameter `_Self` on trait `Baz`
     //~|WARN there is no parameter `crate_local` on trait `Baz`
-    //~|WARN there is no parameter `Trait` on trait `Baz`
     //~|WARN there is no parameter `ItemContext` on trait `Baz`
     //~|WARN there is no parameter `This` on trait `Baz`
+    //~|WARN there is no parameter `This` on trait `Baz`
+    //~|WARN there is no parameter `This` on trait `Baz`
+    //~|WARN format specifiers are not permitted in diagnostic attributes
+    //~|WARN format specifiers are not permitted in diagnostic attributes
 )]
 trait Baz {}