From a9d5101a670edf5494eda17c0439ea16a44efed1 Mon Sep 17 00:00:00 2001 From: Denis Kilseev Date: Sat, 28 Sep 2024 22:56:58 -0400 Subject: [PATCH] Fixes issue 12996. Edits parsing to accomodate helpful message in case '?#', '?x', '?X' is written in formatted string. --- compiler/rustc_builtin_macros/messages.ftl | 4 ++ compiler/rustc_builtin_macros/src/errors.rs | 19 ++++++++++ compiler/rustc_builtin_macros/src/format.rs | 14 +++++++ compiler/rustc_parse_format/src/lib.rs | 38 +++++++++++++++++++ ...-129966-reorder-remove-format-specifier.rs | 14 +++++++ ...966-reorder-remove-format-specifier.stderr | 26 +++++++++++++ 6 files changed, 115 insertions(+) create mode 100644 tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.rs create mode 100644 tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.stderr diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index 9695df9c87e8a..7b9b2c37b76c8 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -162,8 +162,12 @@ builtin_macros_format_redundant_args = redundant {$n -> } .suggestion = this can be removed +builtin_macros_format_remove = consider removing the format specifier instead + builtin_macros_format_remove_raw_ident = remove the `r#` +builtin_macros_format_reorder = consider reordering the format specifier instead + builtin_macros_format_requires_string = requires at least a format string argument builtin_macros_format_string_invalid = invalid format string: {$desc} diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 4fffffb91c8b4..709ef6982c492 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -551,6 +551,25 @@ pub(crate) enum InvalidFormatStringSuggestion { #[primary_span] span: Span, }, + #[multipart_suggestion( + builtin_macros_format_reorder, + style = "tool-only", + applicability = "machine-applicable" + )] + ReorderFormat { + #[suggestion_part(code = "{string}")] + span: Span, + string: String, + }, + #[multipart_suggestion( + builtin_macros_format_remove, + style = "tool-only", + applicability = "machine-applicable" + )] + RemoveCharacter { + #[suggestion_part(code = "")] + span: Span, + }, } #[derive(Diagnostic)] diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 9501e93bad534..6d1d3825555d3 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -302,6 +302,20 @@ fn make_format_args( e.sugg_ = Some(errors::InvalidFormatStringSuggestion::RemoveRawIdent { span }) } } + parse::Suggestion::ReorderFormat(span) => { + let span = fmt_span.from_inner(InnerSpan::new(span.start, span.end)); + if let Ok(original_string) = ecx.source_map().span_to_snippet(span) { + let reversed_string: String = original_string.chars().rev().collect(); + e.sugg_ = Some(errors::InvalidFormatStringSuggestion::ReorderFormat { + span: span, + string: reversed_string, + }); + } + } + parse::Suggestion::RemoveCharacter(span) => { + let span = fmt_span.from_inner(InnerSpan::new(span.start, span.end)); + e.sugg_ = Some(errors::InvalidFormatStringSuggestion::RemoveCharacter { span }) + } } let guar = ecx.dcx().emit_err(e); return ExpandResult::Ready(Err(guar)); diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs index e7ef4385baf4f..974b06f8f2631 100644 --- a/compiler/rustc_parse_format/src/lib.rs +++ b/compiler/rustc_parse_format/src/lib.rs @@ -221,6 +221,10 @@ pub enum Suggestion { /// Remove `r#` from identifier: /// `format!("{r#foo}")` -> `format!("{foo}")` RemoveRawIdent(InnerSpan), + // Reorder `?#` to `#?`. + ReorderFormat(InnerSpan), + // Remove possibly unnecessary character. + RemoveCharacter(InnerSpan), } /// The parser structure for interpreting the input format string. This is @@ -731,6 +735,13 @@ impl<'a> Parser<'a> { } } else if self.consume('?') { spec.ty = "?"; + if let Some(&(_, char)) = self.cur.peek() { + match char { + '#' => self.suggest_format_reorder(char), + 'x' | 'X' => self.suggest_format_remove(char), + _ => {} + } + } } else { spec.ty = self.word(); if !spec.ty.is_empty() { @@ -895,6 +906,33 @@ impl<'a> Parser<'a> { } } + fn suggest_format_reorder(&mut self, reorder: char) { + if let Some(pos) = self.consume_pos(reorder) { + let pos = self.span(pos - 1, pos + 1); + self.errors.insert(0, ParseError { + description: format!("unknown format identifier '?{}'", reorder), + note: Some("if you intended to print `{`, you can escape it using `{{`".to_owned()), + label: format!("help: the format parameter should be written `{}?`", reorder), + span: pos, + secondary_label: None, + suggestion: Suggestion::ReorderFormat(pos), + }); + } + } + fn suggest_format_remove(&mut self, remove: char) { + if let Some(pos) = self.consume_pos(remove) { + let pos = self.span(pos, pos + 1); + self.errors.insert(0, ParseError { + description: format!("unknown format identifier '?{}'", remove), + note: Some("if you intended to print `{`, you can escape it using `{{`".to_owned()), + label: format!("help: consider removing `{}`", remove), + span: pos, + secondary_label: None, + suggestion: Suggestion::RemoveCharacter(pos), + }); + } + } + fn suggest_positional_arg_instead_of_captured_arg(&mut self, arg: Argument<'a>) { if let Some(end) = self.consume_pos('.') { let byte_pos = self.to_span_index(end); diff --git a/tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.rs b/tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.rs new file mode 100644 index 0000000000000..836671eb249f3 --- /dev/null +++ b/tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.rs @@ -0,0 +1,14 @@ +// Test checks for the help messages in the error output/ +//@ check-run-results + +struct Foo(u8, u8); + +fn main() { + let f = Foo(1, 2); + format!("{f:?#}"); +//~^ ERROR invalid format string: unknown format identifier '?#' + format!("{f:?x}"); +//~^ ERROR invalid format string: unknown format identifier '?x' + format!("{f:?X}"); +//~^ ERROR invalid format string: unknown format identifier '?X' +} diff --git a/tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.stderr b/tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.stderr new file mode 100644 index 0000000000000..b15a648575bb5 --- /dev/null +++ b/tests/ui/fmt/help-message-issue-129966-reorder-remove-format-specifier.stderr @@ -0,0 +1,26 @@ +error: invalid format string: unknown format identifier '?#' + --> $DIR/help-message-issue-129966-reorder-remove-format-specifier.rs:8:17 + | +LL | format!("{f:?#}"); + | ^^ help: the format parameter should be written `#?` in format string + | + = note: if you intended to print `{`, you can escape it using `{{` + +error: invalid format string: unknown format identifier '?x' + --> $DIR/help-message-issue-129966-reorder-remove-format-specifier.rs:10:18 + | +LL | format!("{f:?x}"); + | ^ help: consider removing `x` in format string + | + = note: if you intended to print `{`, you can escape it using `{{` + +error: invalid format string: unknown format identifier '?X' + --> $DIR/help-message-issue-129966-reorder-remove-format-specifier.rs:12:18 + | +LL | format!("{f:?X}"); + | ^ help: consider removing `X` in format string + | + = note: if you intended to print `{`, you can escape it using `{{` + +error: aborting due to 3 previous errors +