From a872762151760d65bddf6b3ebad19bc3ea7ec31d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 17 Jul 2023 00:36:00 +0000 Subject: [PATCH] Improve error message when closing bracket interpreted as formatting fill character --- compiler/rustc_parse_format/src/lib.rs | 103 ++++++++++------------ tests/ui/fmt/closing-brace-as-fill.rs | 8 ++ tests/ui/fmt/closing-brace-as-fill.stderr | 12 +++ 3 files changed, 65 insertions(+), 58 deletions(-) create mode 100644 tests/ui/fmt/closing-brace-as-fill.rs create mode 100644 tests/ui/fmt/closing-brace-as-fill.stderr diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs index f66468e9796..88452ccdf05 100644 --- a/compiler/rustc_parse_format/src/lib.rs +++ b/compiler/rustc_parse_format/src/lib.rs @@ -109,6 +109,8 @@ pub struct Argument<'a> { pub struct FormatSpec<'a> { /// Optionally specified character to fill alignment with. pub fill: Option, + /// Span of the optionally specified fill character. + pub fill_span: Option, /// Optionally specified alignment. pub align: Alignment, /// The `+` or `-` flag. @@ -264,7 +266,7 @@ impl<'a> Iterator for Parser<'a> { Some(String(self.string(pos + 1))) } else { let arg = self.argument(lbrace_end); - if let Some(rbrace_pos) = self.must_consume('}') { + if let Some(rbrace_pos) = self.consume_closing_brace(&arg) { if self.is_source_literal { let lbrace_byte_pos = self.to_span_index(pos); let rbrace_byte_pos = self.to_span_index(rbrace_pos); @@ -450,69 +452,51 @@ impl<'a> Parser<'a> { /// Forces consumption of the specified character. If the character is not /// found, an error is emitted. - fn must_consume(&mut self, c: char) -> Option { + fn consume_closing_brace(&mut self, arg: &Argument<'_>) -> Option { self.ws(); - if let Some(&(pos, maybe)) = self.cur.peek() { - if c == maybe { + let pos; + let description; + + if let Some(&(peek_pos, maybe)) = self.cur.peek() { + if maybe == '}' { self.cur.next(); - Some(pos) - } else { - let pos = self.to_span_index(pos); - let description = format!("expected `'}}'`, found `{maybe:?}`"); - let label = "expected `'}'`".to_owned(); - let (note, secondary_label) = if c == '}' { - ( - Some( - "if you intended to print `{`, you can escape it using `{{`".to_owned(), - ), - self.last_opening_brace - .map(|sp| ("because of this opening brace".to_owned(), sp)), - ) - } else { - (None, None) - }; - self.errors.push(ParseError { - description, - note, - label, - span: pos.to(pos), - secondary_label, - should_be_replaced_with_positional_argument: false, - }); - None + return Some(peek_pos); } + + pos = peek_pos; + description = format!("expected `'}}'`, found `{maybe:?}`"); } else { - let description = format!("expected `{c:?}` but string was terminated"); + description = "expected `'}'` but string was terminated".to_owned(); // point at closing `"` - let pos = self.input.len() - if self.append_newline { 1 } else { 0 }; - let pos = self.to_span_index(pos); - if c == '}' { - let label = format!("expected `{c:?}`"); - let (note, secondary_label) = if c == '}' { - ( - Some( - "if you intended to print `{`, you can escape it using `{{`".to_owned(), - ), - self.last_opening_brace - .map(|sp| ("because of this opening brace".to_owned(), sp)), - ) - } else { - (None, None) - }; - self.errors.push(ParseError { - description, - note, - label, - span: pos.to(pos), - secondary_label, - should_be_replaced_with_positional_argument: false, - }); - } else { - self.err(description, format!("expected `{c:?}`"), pos.to(pos)); - } - None + pos = self.input.len() - if self.append_newline { 1 } else { 0 }; } + + let pos = self.to_span_index(pos); + + let label = "expected `'}'`".to_owned(); + let (note, secondary_label) = if arg.format.fill == Some('}') { + ( + Some("the character `'}'` is interpreted as a fill character because of the `:` that precedes it".to_owned()), + arg.format.fill_span.map(|sp| ("this is not interpreted as a formatting closing brace".to_owned(), sp)), + ) + } else { + ( + Some("if you intended to print `{`, you can escape it using `{{`".to_owned()), + self.last_opening_brace.map(|sp| ("because of this opening brace".to_owned(), sp)), + ) + }; + + self.errors.push(ParseError { + description, + note, + label, + span: pos.to(pos), + secondary_label, + should_be_replaced_with_positional_argument: false, + }); + + None } /// Consumes all whitespace characters until the first non-whitespace character @@ -608,6 +592,7 @@ impl<'a> Parser<'a> { fn format(&mut self) -> FormatSpec<'a> { let mut spec = FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -625,9 +610,10 @@ impl<'a> Parser<'a> { } // fill character - if let Some(&(_, c)) = self.cur.peek() { + if let Some(&(idx, c)) = self.cur.peek() { if let Some((_, '>' | '<' | '^')) = self.cur.clone().nth(1) { spec.fill = Some(c); + spec.fill_span = Some(self.span(idx, idx + 1)); self.cur.next(); } } @@ -722,6 +708,7 @@ impl<'a> Parser<'a> { fn inline_asm(&mut self) -> FormatSpec<'a> { let mut spec = FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, diff --git a/tests/ui/fmt/closing-brace-as-fill.rs b/tests/ui/fmt/closing-brace-as-fill.rs new file mode 100644 index 00000000000..6ad257f943e --- /dev/null +++ b/tests/ui/fmt/closing-brace-as-fill.rs @@ -0,0 +1,8 @@ +// issue: 112732 + +// `}` is typoed since it is interpreted as a fill character rather than a closing bracket + +fn main() { + println!("Hello, world! {0:}<3", 2); + //~^ ERROR invalid format string: expected `'}'` but string was terminated +} diff --git a/tests/ui/fmt/closing-brace-as-fill.stderr b/tests/ui/fmt/closing-brace-as-fill.stderr new file mode 100644 index 00000000000..aa1e5aff652 --- /dev/null +++ b/tests/ui/fmt/closing-brace-as-fill.stderr @@ -0,0 +1,12 @@ +error: invalid format string: expected `'}'` but string was terminated + --> $DIR/closing-brace-as-fill.rs:6:35 + | +LL | println!("Hello, world! {0:}<3", 2); + | - ^ expected `'}'` in format string + | | + | this is not interpreted as a formatting closing brace + | + = note: the character `'}'` is interpreted as a fill character because of the `:` that precedes it + +error: aborting due to previous error +