Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message when closing bracket interpreted as formatting fill character #113774

Merged
merged 3 commits into from
Jul 19, 2023
+97 −75
Merged

Improve error message when closing bracket interpreted as formatting fill character #113774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe4d1f9
Fix quotes in output
compiler-errors Jul 17, 2023
a872762
Improve error message when closing bracket interpreted as formatting …
compiler-errors Jul 17, 2023
e021191
Fix unit tests
compiler-errors Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 45 additions & 58 deletions compiler/rustc_parse_format/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ pub struct Argument<'a> {
pub struct FormatSpec<'a> {
/// Optionally specified character to fill alignment with.
pub fill: Option<char>,
/// Span of the optionally specified fill character.
pub fill_span: Option<InnerSpan>,
/// Optionally specified alignment.
pub align: Alignment,
/// The `+` or `-` flag.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<usize> {
fn consume_closing_brace(&mut self, arg: &Argument<'_>) -> Option<usize> {
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
Expand Down Expand Up @@ -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,
Expand All @@ -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();
}
}
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_parse_format/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/fmt/closing-brace-as-fill.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
12 changes: 12 additions & 0 deletions tests/ui/fmt/closing-brace-as-fill.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

Loading