From 159dcb2194e6af28e070f15b9a69af354f87d6d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jul 2019 18:19:21 -0700 Subject: [PATCH] On `format!()` arg count mismatch provide extra info When positional width and precision formatting flags are present in a formatting string that has an argument count mismatch, provide extra information pointing at them making it easiser to understand where the problem may lay: ``` error: 4 positional arguments in format string, but there are 3 arguments --> $DIR/ifmt-bad-arg.rs:78:15 | LL | println!("{} {:.*} {}", 1, 3.2, 4); | ^^ ^^--^ ^^ --- this parameter corresponds to the precision flag | | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html error: 4 positional arguments in format string, but there are 3 arguments --> $DIR/ifmt-bad-arg.rs:81:15 | LL | println!("{} {:07$.*} {}", 1, 3.2, 4); | ^^ ^^-----^ ^^ --- this parameter corresponds to the precision flag | | | | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected | this width flag expects an `usize` argument at position 7, but there are 3 arguments | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html error: 3 positional arguments in format string, but there are 3 arguments --> $DIR/ifmt-bad-arg.rs:84:15 | LL | println!("{} {:07$} {}", 1, 3.2, 4); | ^^ ^^---^ ^^ | | | this width flag expects an `usize` argument at position 7, but there are 3 arguments | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html ``` --- src/libfmt_macros/lib.rs | 76 +++++++++++++------- src/libsyntax_ext/format.rs | 107 +++++++++++++++++++++++++---- src/test/ui/if/ifmt-bad-arg.rs | 8 +++ src/test/ui/if/ifmt-bad-arg.stderr | 55 ++++++++++++++- 4 files changed, 206 insertions(+), 40 deletions(-) diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index f1c2b1fb871..b01c3a4aa7f 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -59,16 +59,20 @@ pub struct Argument<'a> { /// Specification for the formatting of an argument in the format string. #[derive(Copy, Clone, PartialEq)] pub struct FormatSpec<'a> { - /// Optionally specified character to fill alignment with + /// Optionally specified character to fill alignment with. pub fill: Option, - /// Optionally specified alignment + /// Optionally specified alignment. pub align: Alignment, - /// Packed version of various flags provided + /// Packed version of various flags provided. pub flags: u32, - /// The integer precision to use + /// The integer precision to use. pub precision: Count, - /// The string width requested for the resulting format + /// The span of the precision formatting flag (for diagnostics). + pub precision_span: Option, + /// The string width requested for the resulting format. pub width: Count, + /// The span of the width formatting flag (for diagnostics). + pub width_span: Option, /// The descriptor string representing the name of the format desired for /// this argument, this can be empty or any number of characters, although /// it is required to be one word. @@ -285,19 +289,24 @@ impl<'a> Parser<'a> { } /// Optionally consumes the specified character. If the character is not at - /// the current position, then the current iterator isn't moved and false is - /// returned, otherwise the character is consumed and true is returned. + /// the current position, then the current iterator isn't moved and `false` is + /// returned, otherwise the character is consumed and `true` is returned. fn consume(&mut self, c: char) -> bool { - if let Some(&(_, maybe)) = self.cur.peek() { + self.consume_pos(c).is_some() + } + + /// Optionally consumes the specified character. If the character is not at + /// the current position, then the current iterator isn't moved and `None` is + /// returned, otherwise the character is consumed and the current position is + /// returned. + fn consume_pos(&mut self, c: char) -> Option { + if let Some(&(pos, maybe)) = self.cur.peek() { if c == maybe { self.cur.next(); - true - } else { - false + return Some(pos); } - } else { - false } + None } fn to_span_index(&self, pos: usize) -> InnerOffset { @@ -465,7 +474,9 @@ impl<'a> Parser<'a> { align: AlignUnknown, flags: 0, precision: CountImplied, + precision_span: None, width: CountImplied, + width_span: None, ty: &self.input[..0], }; if !self.consume(':') { @@ -502,6 +513,11 @@ impl<'a> Parser<'a> { } // Width and precision let mut havewidth = false; + + let mut width_span_start = 0; + if let Some((pos, '0')) = self.cur.peek() { + width_span_start = *pos; + } if self.consume('0') { // small ambiguity with '0$' as a format string. In theory this is a // '0' flag and then an ill-formatted format string with just a '$' @@ -515,17 +531,28 @@ impl<'a> Parser<'a> { } } if !havewidth { - spec.width = self.count(); + if width_span_start == 0 { + if let Some((pos, _)) = self.cur.peek() { + width_span_start = *pos; + } + } + let (w, sp) = self.count(width_span_start); + spec.width = w; + spec.width_span = sp; } - if self.consume('.') { - if self.consume('*') { + if let Some(start) = self.consume_pos('.') { + if let Some(end) = self.consume_pos('*') { // Resolve `CountIsNextParam`. // We can do this immediately as `position` is resolved later. let i = self.curarg; self.curarg += 1; spec.precision = CountIsParam(i); + spec.precision_span = + Some(self.to_span_index(start).to(self.to_span_index(end + 1))); } else { - spec.precision = self.count(); + let (p, sp) = self.count(start); + spec.precision = p; + spec.precision_span = sp; } } // Optional radix followed by the actual format specifier @@ -554,24 +581,25 @@ impl<'a> Parser<'a> { /// Parses a Count parameter at the current position. This does not check /// for 'CountIsNextParam' because that is only used in precision, not /// width. - fn count(&mut self) -> Count { + fn count(&mut self, start: usize) -> (Count, Option) { if let Some(i) = self.integer() { - if self.consume('$') { - CountIsParam(i) + if let Some(end) = self.consume_pos('$') { + let span = self.to_span_index(start).to(self.to_span_index(end + 1)); + (CountIsParam(i), Some(span)) } else { - CountIs(i) + (CountIs(i), None) } } else { let tmp = self.cur.clone(); let word = self.word(); if word.is_empty() { self.cur = tmp; - CountImplied + (CountImplied, None) } else if self.consume('$') { - CountIsName(Symbol::intern(word)) + (CountIsName(Symbol::intern(word)), None) } else { self.cur = tmp; - CountImplied + (CountImplied, None) } } } diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index e53660b6568..aa8fc70d806 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -109,6 +109,8 @@ struct Context<'a, 'b> { invalid_refs: Vec<(usize, usize)>, /// Spans of all the formatting arguments, in order. arg_spans: Vec, + /// All the formatting arguments that have formatting flags set, in order for diagnostics. + arg_with_formatting: Vec>, /// Whether this formatting string is a literal or it comes from a macro. is_literal: bool, } @@ -279,14 +281,20 @@ impl<'a, 'b> Context<'a, 'b> { .iter() .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos))); + let mut zero_based_note = false; + if self.names.is_empty() && !numbered_position_args { + let count = self.pieces.len() + self.arg_with_formatting + .iter() + .filter(|fmt| fmt.precision_span.is_some()) + .count(); e = self.ecx.mut_span_err( sp, &format!( "{} positional argument{} in format string, but {}", - self.pieces.len(), - if self.pieces.len() > 1 { "s" } else { "" }, - self.describe_num_args() + count, + if count > 1 { "s" } else { "" }, + self.describe_num_args(), ), ); } else { @@ -317,9 +325,70 @@ impl<'a, 'b> Context<'a, 'b> { &format!("invalid reference to positional {} ({})", arg_list, self.describe_num_args())); - e.note("positional arguments are zero-based"); + zero_based_note = true; }; + for fmt in &self.arg_with_formatting { + if let Some(span) = fmt.precision_span { + let span = self.fmtsp.from_inner(span); + match fmt.precision { + parse::CountIsParam(pos) if pos > self.args.len() => { + e.span_label(span, &format!( + "this precision flag expects an `usize` argument at position {}, \ + but {}", + pos, + self.describe_num_args(), + )); + zero_based_note = true; + } + parse::CountIsParam(pos) => { + let count = self.pieces.len() + self.arg_with_formatting + .iter() + .filter(|fmt| fmt.precision_span.is_some()) + .count(); + e.span_label(span, &format!( + "this precision flag adds an extra required argument at position {}, \ + which is why there {} expected", + pos, + if count == 1 { + "is 1 argument".to_string() + } else { + format!("are {} arguments", count) + }, + )); + e.span_label( + self.args[pos].span, + "this parameter corresponds to the precision flag", + ); + zero_based_note = true; + } + _ => {} + } + } + if let Some(span) = fmt.width_span { + let span = self.fmtsp.from_inner(span); + match fmt.width { + parse::CountIsParam(pos) if pos > self.args.len() => { + e.span_label(span, &format!( + "this width flag expects an `usize` argument at position {}, \ + but {}", + pos, + self.describe_num_args(), + )); + zero_based_note = true; + } + _ => {} + } + } + } + if zero_based_note { + e.note("positional arguments are zero-based"); + } + if !self.arg_with_formatting.is_empty() { + e.note("for information about formatting flags, visit \ + https://doc.rust-lang.org/std/fmt/index.html"); + } + e.emit(); } @@ -435,10 +504,11 @@ impl<'a, 'b> Context<'a, 'b> { /// Builds a static `rt::Argument` from a `parse::Piece` or append /// to the `literal` string. - fn build_piece(&mut self, - piece: &parse::Piece<'_>, - arg_index_consumed: &mut Vec) - -> Option> { + fn build_piece( + &mut self, + piece: &parse::Piece<'a>, + arg_index_consumed: &mut Vec, + ) -> Option> { let sp = self.macsp; match *piece { parse::String(s) => { @@ -496,7 +566,9 @@ impl<'a, 'b> Context<'a, 'b> { align: parse::AlignUnknown, flags: 0, precision: parse::CountImplied, + precision_span: None, width: parse::CountImplied, + width_span: None, ty: arg.format.ty, }, }; @@ -506,6 +578,9 @@ impl<'a, 'b> Context<'a, 'b> { let pos_simple = arg.position.index() == simple_arg.position.index(); + if arg.format.precision_span.is_some() || arg.format.width_span.is_some() { + self.arg_with_formatting.push(arg.format); //'liself.fmtsp.from_inner(span)); + } if !pos_simple || arg.format != simple_arg.format || fill != ' ' { self.all_pieces_simple = false; } @@ -530,7 +605,7 @@ impl<'a, 'b> Context<'a, 'b> { let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "FormatSpec")); let fmt = self.ecx.expr_struct( sp, - path, + path, vec![ self.ecx.field_imm(sp, self.ecx.ident_of("fill"), fill), self.ecx.field_imm(sp, self.ecx.ident_of("align"), align), @@ -657,12 +732,13 @@ impl<'a, 'b> Context<'a, 'b> { self.ecx.expr_call_global(self.macsp, path, fn_args) } - fn format_arg(ecx: &ExtCtxt<'_>, - macsp: Span, - mut sp: Span, - ty: &ArgumentType, - arg: ast::Ident) - -> P { + fn format_arg( + ecx: &ExtCtxt<'_>, + macsp: Span, + mut sp: Span, + ty: &ArgumentType, + arg: ast::Ident, + ) -> P { sp = sp.apply_mark(ecx.current_expansion.id); let arg = ecx.expr_ident(sp, arg); let trait_ = match *ty { @@ -941,6 +1017,7 @@ pub fn expand_preparsed_format_args( fmtsp: fmt.span, invalid_refs: Vec::new(), arg_spans, + arg_with_formatting: Vec::new(), is_literal, }; diff --git a/src/test/ui/if/ifmt-bad-arg.rs b/src/test/ui/if/ifmt-bad-arg.rs index 0ebe1fa2dff..d5c7950542d 100644 --- a/src/test/ui/if/ifmt-bad-arg.rs +++ b/src/test/ui/if/ifmt-bad-arg.rs @@ -75,4 +75,12 @@ ninth number: { tenth number: {}", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10); //~^^ ERROR: invalid format string + println!("{} {:.*} {}", 1, 3.2, 4); + //~^ ERROR 4 positional arguments in format string, but there are 3 arguments + //~| ERROR mismatched types + println!("{} {:07$.*} {}", 1, 3.2, 4); + //~^ ERROR 4 positional arguments in format string, but there are 3 arguments + //~| ERROR mismatched types + println!("{} {:07$} {}", 1, 3.2, 4); + //~^ ERROR 3 positional arguments in format string, but there are 3 arguments } diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr index 835b5b6592b..1eaeb017587 100644 --- a/src/test/ui/if/ifmt-bad-arg.stderr +++ b/src/test/ui/if/ifmt-bad-arg.stderr @@ -220,5 +220,58 @@ LL | tenth number: {}", | = note: if you intended to print `{`, you can escape it using `{{` -error: aborting due to 28 previous errors +error: 4 positional arguments in format string, but there are 3 arguments + --> $DIR/ifmt-bad-arg.rs:78:15 + | +LL | println!("{} {:.*} {}", 1, 3.2, 4); + | ^^ ^^--^ ^^ --- this parameter corresponds to the precision flag + | | + | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected + | + = note: positional arguments are zero-based + = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html +error: 4 positional arguments in format string, but there are 3 arguments + --> $DIR/ifmt-bad-arg.rs:81:15 + | +LL | println!("{} {:07$.*} {}", 1, 3.2, 4); + | ^^ ^^-----^ ^^ --- this parameter corresponds to the precision flag + | | | + | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected + | this width flag expects an `usize` argument at position 7, but there are 3 arguments + | + = note: positional arguments are zero-based + = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html + +error: 3 positional arguments in format string, but there are 3 arguments + --> $DIR/ifmt-bad-arg.rs:84:15 + | +LL | println!("{} {:07$} {}", 1, 3.2, 4); + | ^^ ^^---^ ^^ + | | + | this width flag expects an `usize` argument at position 7, but there are 3 arguments + | + = note: positional arguments are zero-based + = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html + +error[E0308]: mismatched types + --> $DIR/ifmt-bad-arg.rs:78:32 + | +LL | println!("{} {:.*} {}", 1, 3.2, 4); + | ^^^ expected usize, found floating-point number + | + = note: expected type `&usize` + found type `&{float}` + +error[E0308]: mismatched types + --> $DIR/ifmt-bad-arg.rs:81:35 + | +LL | println!("{} {:07$.*} {}", 1, 3.2, 4); + | ^^^ expected usize, found floating-point number + | + = note: expected type `&usize` + found type `&{float}` + +error: aborting due to 33 previous errors + +For more information about this error, try `rustc --explain E0308`.