Rollup merge of #141474 - mejrs:diagnostic_mode, r=compiler-errors

Add `ParseMode::Diagnostic` and fix multiline spans in diagnostic attribute lints

Best viewed commit by commit.

The first commit is a test, the commits following that are small refactors to `rustc_parse_format`. Originally I wanted to do a much larger change (doing these smaller fixes first would have that made easier to review), but ended up doing something else instead.

An observable change from this is that the diagnostic attribute no longer tries to parse align/fill/width/etc parameters. For an example (see also test changes), a string like `"{Self:!}"` no longer says "missing '}'", instead it says that format parameters are not allowed. It'll now also format the string as if the user wrote just `"{Self}"`

Author: matthiaskrgr

compiler/rustc_parse_format/src/lib.rs

@@ -553,3 +553,45 @@ fn asm_concat() {
     assert_eq!(parser.by_ref().collect::<Vec<Piece<'static>>>(), &[Lit(asm)]);
     assert_eq!(parser.line_spans, &[]);
 }
+
+#[test]
+fn diagnostic_format_flags() {
+    let lit = "{thing:blah}";
+    let mut parser = Parser::new(lit, None, None, false, ParseMode::Diagnostic);
+    assert!(!parser.is_source_literal);
+
+    let [NextArgument(arg)] = &*parser.by_ref().collect::<Vec<Piece<'static>>>() else { panic!() };
+
+    assert_eq!(
+        **arg,
+        Argument {
+            position: ArgumentNamed("thing"),
+            position_span: 2..7,
+            format: FormatSpec { ty: ":blah", ty_span: Some(7..12), ..Default::default() },
+        }
+    );
+
+    assert_eq!(parser.line_spans, &[]);
+    assert!(parser.errors.is_empty());
+}
+
+#[test]
+fn diagnostic_format_mod() {
+    let lit = "{thing:+}";
+    let mut parser = Parser::new(lit, None, None, false, ParseMode::Diagnostic);
+    assert!(!parser.is_source_literal);
+
+    let [NextArgument(arg)] = &*parser.by_ref().collect::<Vec<Piece<'static>>>() else { panic!() };
+
+    assert_eq!(
+        **arg,
+        Argument {
+            position: ArgumentNamed("thing"),
+            position_span: 2..7,
+            format: FormatSpec { ty: ":+", ty_span: Some(7..9), ..Default::default() },
+        }
+    );
+
+    assert_eq!(parser.line_spans, &[]);
+    assert!(parser.errors.is_empty());
+}

compiler/rustc_parse_format/src/tests.rs

@@ -810,7 +810,8 @@ impl<'tcx> OnUnimplementedFormatString {
 
         let mut result = Ok(());
 
-        match FormatString::parse(self.symbol, self.span, &ctx) {
+        let snippet = tcx.sess.source_map().span_to_snippet(self.span).ok();
+        match FormatString::parse(self.symbol, snippet, self.span, &ctx) {
             // Warnings about format specifiers, deprecated parameters, wrong parameters etc.
             // In other words we'd like to let the author know, but we can still try to format the string later
             Ok(FormatString { warnings, .. }) => {
@@ -848,34 +849,27 @@ impl<'tcx> OnUnimplementedFormatString {
                     }
                 }
             }
-            // Errors from the underlying `rustc_parse_format::Parser`
-            Err(errors) => {
+            // Error from the underlying `rustc_parse_format::Parser`
+            Err(e) => {
                 // we cannot return errors from processing the format string as hard error here
                 // as the diagnostic namespace guarantees that malformed input cannot cause an error
                 //
                 // if we encounter any error while processing we nevertheless want to show it as warning
                 // so that users are aware that something is not correct
-                for e in errors {
-                    if self.is_diagnostic_namespace_variant {
-                        if let Some(trait_def_id) = trait_def_id.as_local() {
-                            tcx.emit_node_span_lint(
-                                UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                                tcx.local_def_id_to_hir_id(trait_def_id),
-                                self.span,
-                                WrappedParserError { description: e.description, label: e.label },
-                            );
-                        }
-                    } else {
-                        let reported = struct_span_code_err!(
-                            tcx.dcx(),
+                if self.is_diagnostic_namespace_variant {
+                    if let Some(trait_def_id) = trait_def_id.as_local() {
+                        tcx.emit_node_span_lint(
+                            UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                            tcx.local_def_id_to_hir_id(trait_def_id),
                             self.span,
-                            E0231,
-                            "{}",
-                            e.description,
-                        )
-                        .emit();
-                        result = Err(reported);
+                            WrappedParserError { description: e.description, label: e.label },
+                        );
                     }
+                } else {
+                    let reported =
+                        struct_span_code_err!(tcx.dcx(), self.span, E0231, "{}", e.description,)
+                            .emit();
+                    result = Err(reported);
                 }
             }
         }
@@ -896,7 +890,8 @@ impl<'tcx> OnUnimplementedFormatString {
             Ctx::RustcOnUnimplemented { tcx, trait_def_id }
         };
 
-        if let Ok(s) = FormatString::parse(self.symbol, self.span, &ctx) {
+        // No point passing a snippet here, we already did that in `verify`
+        if let Ok(s) = FormatString::parse(self.symbol, None, self.span, &ctx) {
             s.format(args)
         } else {
             // we cannot return errors from processing the format string as hard error here

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

@@ -198,7 +198,7 @@ enum LitOrArg {
 
 impl FilterFormatString {
     fn parse(input: Symbol) -> Self {
-        let pieces = Parser::new(input.as_str(), None, None, false, ParseMode::Format)
+        let pieces = Parser::new(input.as_str(), None, None, false, ParseMode::Diagnostic)
             .map(|p| match p {
                 Piece::Lit(s) => LitOrArg::Lit(s.to_owned()),
                 // We just ignore formatspecs here

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

@@ -5,12 +5,11 @@ use errors::*;
 use rustc_middle::ty::print::TraitRefPrintSugared;
 use rustc_middle::ty::{GenericParamDefKind, TyCtxt};
 use rustc_parse_format::{
-    Alignment, Argument, Count, FormatSpec, ParseError, ParseMode, Parser, Piece as RpfPiece,
-    Position,
+    Argument, FormatSpec, ParseError, ParseMode, Parser, Piece as RpfPiece, Position,
 };
 use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES;
 use rustc_span::def_id::DefId;
-use rustc_span::{BytePos, Pos, Span, Symbol, kw, sym};
+use rustc_span::{InnerSpan, Span, Symbol, kw, sym};
 
 /// Like [std::fmt::Arguments] this is a string that has been parsed into "pieces",
 /// either as string pieces or dynamic arguments.
@@ -160,32 +159,32 @@ impl FormatString {
 
     pub fn parse<'tcx>(
         input: Symbol,
+        snippet: Option<String>,
         span: Span,
         ctx: &Ctx<'tcx>,
-    ) -> Result<Self, Vec<ParseError>> {
+    ) -> Result<Self, ParseError> {
         let s = input.as_str();
-        let mut parser = Parser::new(s, None, None, false, ParseMode::Format);
-        let mut pieces = Vec::new();
+        let mut parser = Parser::new(s, None, snippet, false, ParseMode::Diagnostic);
+        let pieces: Vec<_> = parser.by_ref().collect();
+
+        if let Some(err) = parser.errors.into_iter().next() {
+            return Err(err);
+        }
         let mut warnings = Vec::new();
 
-        for piece in &mut parser {
-            match piece {
-                RpfPiece::Lit(lit) => {
-                    pieces.push(Piece::Lit(lit.into()));
-                }
+        let pieces = pieces
+            .into_iter()
+            .map(|piece| match piece {
+                RpfPiece::Lit(lit) => Piece::Lit(lit.into()),
                 RpfPiece::NextArgument(arg) => {
-                    warn_on_format_spec(arg.format.clone(), &mut warnings, span);
-                    let arg = parse_arg(&arg, ctx, &mut warnings, span);
-                    pieces.push(Piece::Arg(arg));
+                    warn_on_format_spec(&arg.format, &mut warnings, span, parser.is_source_literal);
+                    let arg = parse_arg(&arg, ctx, &mut warnings, span, parser.is_source_literal);
+                    Piece::Arg(arg)
                 }
-            }
-        }
+            })
+            .collect();
 
-        if parser.errors.is_empty() {
-            Ok(FormatString { input, pieces, span, warnings })
-        } else {
-            Err(parser.errors)
-        }
+        Ok(FormatString { input, pieces, span, warnings })
     }
 
     pub fn format(&self, args: &FormatArgs<'_>) -> String {
@@ -229,11 +228,12 @@ fn parse_arg<'tcx>(
     ctx: &Ctx<'tcx>,
     warnings: &mut Vec<FormatWarning>,
     input_span: Span,
+    is_source_literal: bool,
 ) -> FormatArg {
     let (Ctx::RustcOnUnimplemented { tcx, trait_def_id }
     | Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id }) = ctx;
 
-    let span = slice_span(input_span, arg.position_span.clone());
+    let span = slice_span(input_span, arg.position_span.clone(), is_source_literal);
 
     match arg.position {
         // Something like "hello {name}"
@@ -283,39 +283,24 @@ fn parse_arg<'tcx>(
 
 /// `#[rustc_on_unimplemented]` and `#[diagnostic::...]` don't actually do anything
 /// with specifiers, so emit a warning if they are used.
-fn warn_on_format_spec(spec: FormatSpec<'_>, warnings: &mut Vec<FormatWarning>, input_span: Span) {
-    if !matches!(
-        spec,
-        FormatSpec {
-            fill: None,
-            fill_span: None,
-            align: Alignment::AlignUnknown,
-            sign: None,
-            alternate: false,
-            zero_pad: false,
-            debug_hex: None,
-            precision: Count::CountImplied,
-            precision_span: None,
-            width: Count::CountImplied,
-            width_span: None,
-            ty: _,
-            ty_span: _,
-        },
-    ) {
-        let span = spec.ty_span.map(|inner| slice_span(input_span, inner)).unwrap_or(input_span);
+fn warn_on_format_spec(
+    spec: &FormatSpec<'_>,
+    warnings: &mut Vec<FormatWarning>,
+    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);
         warnings.push(FormatWarning::InvalidSpecifier { span, name: spec.ty.into() })
     }
 }
 
-fn slice_span(input: Span, range: Range<usize>) -> Span {
-    let span = input.data();
-
-    Span::new(
-        span.lo + BytePos::from_usize(range.start),
-        span.lo + BytePos::from_usize(range.end),
-        span.ctxt,
-        span.parent,
-    )
+fn slice_span(input: Span, Range { start, end }: Range<usize>, is_source_literal: bool) -> Span {
+    if is_source_literal { input.from_inner(InnerSpan { start, end }) } else { input }
 }
 
 pub mod errors {

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

@@ -0,0 +1,55 @@
+#![crate_type = "lib"]
+#![deny(unknown_or_malformed_diagnostic_attributes)]
+
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+                                         multiline string \
+                                         {unknown}")]
+//~^ ERROR there is no parameter `unknown` on trait `MultiLine` [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLine {}
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+                                         multiline string {unknown}")]
+//~^ ERROR there is no parameter `unknown` on trait `MultiLine2` [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLine2 {}
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+    multiline string {unknown}")]
+//~^ ERROR there is no parameter `unknown` on trait `MultiLine3` [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLine3 {}
+
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+\
+                \
+                                \
+                                                \
+    multiline string {unknown}")]
+//~^ ERROR there is no parameter `unknown` on trait `MultiLine4` [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLine4 {}
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+                                         multiline string \
+                                         {Self:+}")]
+//~^ ERROR invalid format specifier [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLineFmt {}
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+                                         multiline string {Self:X}")]
+//~^ ERROR invalid format specifier [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLineFmt2 {}
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+    multiline string {Self:#}")]
+//~^ ERROR invalid format specifier [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLineFmt3 {}
+
+
+#[diagnostic::on_unimplemented(message = "here is a big \
+\
+                \
+                                \
+                                                \
+    multiline string {Self:?}")]
+//~^ ERROR invalid format specifier [unknown_or_malformed_diagnostic_attributes]
+pub trait MultiLineFmt4 {}

tests/ui/diagnostic_namespace/multiline_spans.rs

@@ -0,0 +1,71 @@
+error: there is no parameter `unknown` on trait `MultiLine`
+  --> $DIR/multiline_spans.rs:7:43
+   |
+LL | ...                   {unknown}")]
+   |                        ^^^^^^^
+   |
+   = help: expect either a generic argument name or `{Self}` as format argument
+note: the lint level is defined here
+  --> $DIR/multiline_spans.rs:2:9
+   |
+LL | #![deny(unknown_or_malformed_diagnostic_attributes)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: there is no parameter `unknown` on trait `MultiLine2`
+  --> $DIR/multiline_spans.rs:12:60
+   |
+LL | ...                   multiline string {unknown}")]
+   |                                         ^^^^^^^
+   |
+   = help: expect either a generic argument name or `{Self}` as format argument
+
+error: there is no parameter `unknown` on trait `MultiLine3`
+  --> $DIR/multiline_spans.rs:17:23
+   |
+LL |     multiline string {unknown}")]
+   |                       ^^^^^^^
+   |
+   = help: expect either a generic argument name or `{Self}` as format argument
+
+error: there is no parameter `unknown` on trait `MultiLine4`
+  --> $DIR/multiline_spans.rs:27:23
+   |
+LL |     multiline string {unknown}")]
+   |                       ^^^^^^^
+   |
+   = help: expect either a generic argument name or `{Self}` as format argument
+
+error: invalid format specifier
+  --> $DIR/multiline_spans.rs:33:47
+   |
+LL | ...                   {Self:+}")]
+   |                            ^^
+   |
+   = help: no format specifier are supported in this position
+
+error: invalid format specifier
+  --> $DIR/multiline_spans.rs:38:64
+   |
+LL | ...                   multiline string {Self:X}")]
+   |                                             ^^
+   |
+   = help: no format specifier are supported in this position
+
+error: invalid format specifier
+  --> $DIR/multiline_spans.rs:43:27
+   |
+LL |     multiline string {Self:#}")]
+   |                           ^^
+   |
+   = help: no format specifier are supported in this position
+
+error: invalid format specifier
+  --> $DIR/multiline_spans.rs:53:27
+   |
+LL |     multiline string {Self:?}")]
+   |                           ^^
+   |
+   = help: no format specifier are supported in this position
+
+error: aborting due to 8 previous errors
+