diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index 83e24a48ea0..985abaf2c1b 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -56,16 +56,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. @@ -282,19 +286,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 { @@ -462,7 +471,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(':') { @@ -499,6 +510,7 @@ impl<'a> Parser<'a> { } // Width and precision let mut havewidth = false; + 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 '$' @@ -512,17 +524,28 @@ impl<'a> Parser<'a> { } } if !havewidth { - spec.width = self.count(); + let width_span_start = if let Some((pos, _)) = self.cur.peek() { + *pos + } else { + 0 + }; + 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 @@ -551,24 +574,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/libfmt_macros/tests.rs b/src/libfmt_macros/tests.rs index 7282d4a5f24..e2ddb8810e9 100644 --- a/src/libfmt_macros/tests.rs +++ b/src/libfmt_macros/tests.rs @@ -12,6 +12,8 @@ fn fmtdflt() -> FormatSpec<'static> { flags: 0, precision: CountImplied, width: CountImplied, + precision_span: None, + width_span: None, ty: "", }; } @@ -79,165 +81,204 @@ fn format_position_nothing_else() { } #[test] fn format_type() { - same("{3:a}", - &[NextArgument(Argument { - position: ArgumentIs(3), - format: FormatSpec { - fill: None, - align: AlignUnknown, - flags: 0, - precision: CountImplied, - width: CountImplied, - ty: "a", - }, - })]); + same( + "{3:a}", + &[NextArgument(Argument { + position: ArgumentIs(3), + format: FormatSpec { + fill: None, + align: AlignUnknown, + flags: 0, + precision: CountImplied, + width: CountImplied, + precision_span: None, + width_span: None, + ty: "a", + }, + })]); } #[test] fn format_align_fill() { - same("{3:>}", - &[NextArgument(Argument { - position: ArgumentIs(3), - format: FormatSpec { - fill: None, - align: AlignRight, - flags: 0, - precision: CountImplied, - width: CountImplied, - ty: "", - }, - })]); - same("{3:0<}", - &[NextArgument(Argument { - position: ArgumentIs(3), - format: FormatSpec { - fill: Some('0'), - align: AlignLeft, - flags: 0, - precision: CountImplied, - width: CountImplied, - ty: "", - }, - })]); - same("{3:*}", + &[NextArgument(Argument { + position: ArgumentIs(3), + format: FormatSpec { + fill: None, + align: AlignRight, + flags: 0, + precision: CountImplied, + width: CountImplied, + precision_span: None, + width_span: None, + ty: "", + }, + })]); + same( + "{3:0<}", + &[NextArgument(Argument { + position: ArgumentIs(3), + format: FormatSpec { + fill: Some('0'), + align: AlignLeft, + flags: 0, + precision: CountImplied, + width: CountImplied, + precision_span: None, + width_span: None, + ty: "", + }, + })]); + same( + "{3:* { 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, } @@ -273,31 +275,44 @@ impl<'a, 'b> Context<'a, 'b> { } else { MultiSpan::from_span(self.fmtsp) }; - let refs_len = self.invalid_refs.len(); - let mut refs = self + let refs = self .invalid_refs .iter() .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos))); - if self.names.is_empty() && !numbered_position_args { + let mut zero_based_note = false; + + let count = self.pieces.len() + self.arg_with_formatting + .iter() + .filter(|fmt| fmt.precision_span.is_some()) + .count(); + if self.names.is_empty() && !numbered_position_args && count != self.args.len() { 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 { - let (arg_list, mut sp) = if refs_len == 1 { - let (reg, pos) = refs.next().unwrap(); + let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip(); + // Avoid `invalid reference to positional arguments 7 and 7 (there is 1 argument)` + // for `println!("{7:7$}", 1);` + refs.sort(); + refs.dedup(); + let (arg_list, mut sp) = if refs.len() == 1 { + let spans: Vec<_> = spans.into_iter().filter_map(|sp| sp.map(|sp| *sp)).collect(); ( - format!("argument {}", reg), - MultiSpan::from_span(*pos.unwrap_or(&self.fmtsp)), + format!("argument {}", refs[0]), + if spans.is_empty() { + MultiSpan::from_span(self.fmtsp) + } else { + MultiSpan::from_spans(spans) + }, ) } else { - let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip(); let pos = MultiSpan::from_spans(spans.into_iter().map(|s| *s.unwrap()).collect()); let reg = refs.pop().unwrap(); ( @@ -317,9 +332,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 +511,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 +573,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 +585,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); + } if !pos_simple || arg.format != simple_arg.format || fill != ' ' { self.all_pieces_simple = false; } @@ -530,7 +612,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 +739,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 { @@ -678,7 +761,21 @@ impl<'a, 'b> Context<'a, 'b> { "x" => "LowerHex", "X" => "UpperHex", _ => { - ecx.span_err(sp, &format!("unknown format trait `{}`", *tyname)); + let mut err = ecx.struct_span_err( + sp, + &format!("unknown format trait `{}`", *tyname), + ); + err.note("the only appropriate formatting traits are:\n\ + - ``, which uses the `Display` trait\n\ + - `?`, which uses the `Debug` trait\n\ + - `e`, which uses the `LowerExp` trait\n\ + - `E`, which uses the `UpperExp` trait\n\ + - `o`, which uses the `Octal` trait\n\ + - `p`, which uses the `Pointer` trait\n\ + - `b`, which uses the `Binary` trait\n\ + - `x`, which uses the `LowerHex` trait\n\ + - `X`, which uses the `UpperHex` trait"); + err.emit(); return DummyResult::raw_expr(sp, true); } } @@ -941,6 +1038,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/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index fa6e3609138..02a7433d946 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -1402,6 +1402,7 @@ pub struct MalformedSourceMapPositions { pub end_pos: BytePos } +/// Range inside of a `Span` used for diagnostics when we only have access to relative positions. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct InnerSpan { pub start: usize, diff --git a/src/test/ui/if/ifmt-bad-arg.rs b/src/test/ui/if/ifmt-bad-arg.rs index 0ebe1fa2dff..ba897f171af 100644 --- a/src/test/ui/if/ifmt-bad-arg.rs +++ b/src/test/ui/if/ifmt-bad-arg.rs @@ -75,4 +75,15 @@ 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 invalid reference to positional argument 7 (there are 3 arguments) + println!("{:foo}", 1); //~ ERROR unknown format trait `foo` + println!("{5} {:4$} {6:7$}", 1); + //~^ ERROR invalid reference to positional arguments 4, 5, 6 and 7 (there is 1 argument) } diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr index 835b5b6592b..336ea2254bf 100644 --- a/src/test/ui/if/ifmt-bad-arg.stderr +++ b/src/test/ui/if/ifmt-bad-arg.stderr @@ -220,5 +220,87 @@ 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: invalid reference to positional argument 7 (there are 3 arguments) + --> $DIR/ifmt-bad-arg.rs:84:18 + | +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: unknown format trait `foo` + --> $DIR/ifmt-bad-arg.rs:86:24 + | +LL | println!("{:foo}", 1); + | ^ + | + = note: the only appropriate formatting traits are: + - ``, which uses the `Display` trait + - `?`, which uses the `Debug` trait + - `e`, which uses the `LowerExp` trait + - `E`, which uses the `UpperExp` trait + - `o`, which uses the `Octal` trait + - `p`, which uses the `Pointer` trait + - `b`, which uses the `Binary` trait + - `x`, which uses the `LowerHex` trait + - `X`, which uses the `UpperHex` trait + +error: invalid reference to positional arguments 4, 5, 6 and 7 (there is 1 argument) + --> $DIR/ifmt-bad-arg.rs:87:15 + | +LL | println!("{5} {:4$} {6:7$}", 1); + | ^^^ ^^--^ ^^^--^ + | | | + | | this width flag expects an `usize` argument at position 7, but there is 1 argument + | this width flag expects an `usize` argument at position 4, but there is 1 argument + | + = 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 35 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/if/ifmt-unknown-trait.stderr b/src/test/ui/if/ifmt-unknown-trait.stderr index 9ea367c81ec..7853b5ca0c9 100644 --- a/src/test/ui/if/ifmt-unknown-trait.stderr +++ b/src/test/ui/if/ifmt-unknown-trait.stderr @@ -3,6 +3,17 @@ error: unknown format trait `notimplemented` | LL | format!("{:notimplemented}", "3"); | ^^^ + | + = note: the only appropriate formatting traits are: + - ``, which uses the `Display` trait + - `?`, which uses the `Debug` trait + - `e`, which uses the `LowerExp` trait + - `E`, which uses the `UpperExp` trait + - `o`, which uses the `Octal` trait + - `p`, which uses the `Pointer` trait + - `b`, which uses the `Binary` trait + - `x`, which uses the `LowerHex` trait + - `X`, which uses the `UpperHex` trait error: aborting due to previous error