From fe4d1f9fe979bab54aba6316a5207f7793d751b1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 17 Jul 2023 00:32:13 +0000 Subject: [PATCH 1/3] Fix quotes in output --- compiler/rustc_parse_format/src/lib.rs | 2 +- tests/ui/fmt/format-string-error-2.stderr | 26 +++++++++---------- tests/ui/fmt/format-string-error.stderr | 2 +- tests/ui/fmt/format-string-wrong-order.stderr | 4 +-- tests/ui/fmt/ifmt-bad-arg.stderr | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs index 7de84db211ed8..f66468e979668 100644 --- a/compiler/rustc_parse_format/src/lib.rs +++ b/compiler/rustc_parse_format/src/lib.rs @@ -460,7 +460,7 @@ impl<'a> Parser<'a> { } else { let pos = self.to_span_index(pos); let description = format!("expected `'}}'`, found `{maybe:?}`"); - let label = "expected `}`".to_owned(); + let label = "expected `'}'`".to_owned(); let (note, secondary_label) = if c == '}' { ( Some( diff --git a/tests/ui/fmt/format-string-error-2.stderr b/tests/ui/fmt/format-string-error-2.stderr index 76cdfbb93bf24..dfd24bf60ad52 100644 --- a/tests/ui/fmt/format-string-error-2.stderr +++ b/tests/ui/fmt/format-string-error-2.stderr @@ -10,7 +10,7 @@ error: invalid format string: expected `'}'`, found `'a'` LL | format!("{ | - because of this opening brace LL | a"); - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -21,7 +21,7 @@ LL | format!("{ \ | - because of this opening brace LL | \ LL | b"); - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -29,7 +29,7 @@ error: invalid format string: expected `'}'`, found `'\'` --> $DIR/format-string-error-2.rs:11:18 | LL | format!(r#"{ \ - | - ^ expected `}` in format string + | - ^ expected `'}'` in format string | | | because of this opening brace | @@ -39,7 +39,7 @@ error: invalid format string: expected `'}'`, found `'\'` --> $DIR/format-string-error-2.rs:15:18 | LL | format!(r#"{ \n - | - ^ expected `}` in format string + | - ^ expected `'}'` in format string | | | because of this opening brace | @@ -52,7 +52,7 @@ LL | format!("{ \n | - because of this opening brace LL | \n LL | e"); - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -62,7 +62,7 @@ error: invalid format string: expected `'}'`, found `'a'` LL | { | - because of this opening brace LL | a"); - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -72,7 +72,7 @@ error: invalid format string: expected `'}'`, found `'a'` LL | { | - because of this opening brace LL | a - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -83,7 +83,7 @@ LL | { \ | - because of this opening brace LL | \ LL | b"); - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -94,7 +94,7 @@ LL | { \ | - because of this opening brace LL | \ LL | b \ - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -102,7 +102,7 @@ error: invalid format string: expected `'}'`, found `'\'` --> $DIR/format-string-error-2.rs:45:8 | LL | raw { \ - | - ^ expected `}` in format string + | - ^ expected `'}'` in format string | | | because of this opening brace | @@ -112,7 +112,7 @@ error: invalid format string: expected `'}'`, found `'\'` --> $DIR/format-string-error-2.rs:50:8 | LL | raw { \n - | - ^ expected `}` in format string + | - ^ expected `'}'` in format string | | | because of this opening brace | @@ -125,7 +125,7 @@ LL | { \n | - because of this opening brace LL | \n LL | e"); - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` @@ -135,7 +135,7 @@ error: invalid format string: expected `'}'`, found `'a'` LL | { | - because of this opening brace LL | asdf} - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` diff --git a/tests/ui/fmt/format-string-error.stderr b/tests/ui/fmt/format-string-error.stderr index 8a32c225485fe..37a181e6fcb23 100644 --- a/tests/ui/fmt/format-string-error.stderr +++ b/tests/ui/fmt/format-string-error.stderr @@ -62,7 +62,7 @@ error: invalid format string: expected `'}'`, found `'\'` --> $DIR/format-string-error.rs:19:23 | LL | let _ = format!("{\}"); - | -^ expected `}` in format string + | -^ expected `'}'` in format string | | | because of this opening brace | diff --git a/tests/ui/fmt/format-string-wrong-order.stderr b/tests/ui/fmt/format-string-wrong-order.stderr index 461af354a4e15..0a2e04026d970 100644 --- a/tests/ui/fmt/format-string-wrong-order.stderr +++ b/tests/ui/fmt/format-string-wrong-order.stderr @@ -26,7 +26,7 @@ error: invalid format string: expected `'}'`, found `'?'` --> $DIR/format-string-wrong-order.rs:9:15 | LL | format!("{??}", bar); - | -^ expected `}` in format string + | -^ expected `'}'` in format string | | | because of this opening brace | @@ -36,7 +36,7 @@ error: invalid format string: expected `'}'`, found `'?'` --> $DIR/format-string-wrong-order.rs:11:15 | LL | format!("{?;bar}"); - | -^ expected `}` in format string + | -^ expected `'}'` in format string | | | because of this opening brace | diff --git a/tests/ui/fmt/ifmt-bad-arg.stderr b/tests/ui/fmt/ifmt-bad-arg.stderr index ed008c454a31c..09ce3dca4117e 100644 --- a/tests/ui/fmt/ifmt-bad-arg.stderr +++ b/tests/ui/fmt/ifmt-bad-arg.stderr @@ -178,7 +178,7 @@ error: invalid format string: expected `'}'`, found `'t'` LL | ninth number: { | - because of this opening brace LL | tenth number: {}", - | ^ expected `}` in format string + | ^ expected `'}'` in format string | = note: if you intended to print `{`, you can escape it using `{{` From a872762151760d65bddf6b3ebad19bc3ea7ec31d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 17 Jul 2023 00:36:00 +0000 Subject: [PATCH 2/3] 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 f66468e979668..88452ccdf052e 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 0000000000000..6ad257f943e93 --- /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 0000000000000..aa1e5aff6525f --- /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 + From e02119146f29892ad25ffc442bdcbc508fe7bdc2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 17 Jul 2023 00:55:52 +0000 Subject: [PATCH 3/3] Fix unit tests --- compiler/rustc_parse_format/src/tests.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compiler/rustc_parse_format/src/tests.rs b/compiler/rustc_parse_format/src/tests.rs index 45314e2fb5500..0c594f9104cc5 100644 --- a/compiler/rustc_parse_format/src/tests.rs +++ b/compiler/rustc_parse_format/src/tests.rs @@ -9,6 +9,7 @@ fn same(fmt: &'static str, p: &[Piece<'static>]) { fn fmtdflt() -> FormatSpec<'static> { return FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -128,6 +129,7 @@ fn format_type() { position_span: InnerSpan { start: 2, end: 3 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -152,6 +154,7 @@ fn format_align_fill() { position_span: InnerSpan { start: 2, end: 3 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignRight, sign: None, alternate: false, @@ -173,6 +176,7 @@ fn format_align_fill() { position_span: InnerSpan { start: 2, end: 3 }, format: FormatSpec { fill: Some('0'), + fill_span: Some(InnerSpan::new(4, 5)), align: AlignLeft, sign: None, alternate: false, @@ -194,6 +198,7 @@ fn format_align_fill() { position_span: InnerSpan { start: 2, end: 3 }, format: FormatSpec { fill: Some('*'), + fill_span: Some(InnerSpan::new(4, 5)), align: AlignLeft, sign: None, alternate: false, @@ -218,6 +223,7 @@ fn format_counts() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -239,6 +245,7 @@ fn format_counts() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -260,6 +267,7 @@ fn format_counts() { position_span: InnerSpan { start: 2, end: 3 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -281,6 +289,7 @@ fn format_counts() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -302,6 +311,7 @@ fn format_counts() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -323,6 +333,7 @@ fn format_counts() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -344,6 +355,7 @@ fn format_counts() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false, @@ -368,6 +380,7 @@ fn format_flags() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: Some(Sign::Minus), alternate: false, @@ -389,6 +402,7 @@ fn format_flags() { position_span: InnerSpan { start: 2, end: 2 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: Some(Sign::Plus), alternate: true, @@ -415,6 +429,7 @@ fn format_mixture() { position_span: InnerSpan { start: 7, end: 8 }, format: FormatSpec { fill: None, + fill_span: None, align: AlignUnknown, sign: None, alternate: false,