Rollup merge of #66134 - estebank:unknown-formatting-trait, r=nikomatsakis

Point at formatting descriptor string when it is invalid When a formatting string contains an invalid descriptor, point at it instead of the argument: ``` error: unknown format trait `foo` --> $DIR/ifmt-bad-arg.rs:86:17 | 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 ```

Author: Centril

src/libfmt_macros/lib.rs

@@ -35,7 +35,7 @@ impl InnerOffset {
 
 /// A piece is a portion of the format string which represents the next part
 /// to emit. These are emitted as a stream by the `Parser` class.
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum Piece<'a> {
  /// A literal string which should directly be emitted
  String(&'a str),
@@ -45,7 +45,7 @@ pub enum Piece<'a> {
 }
 
 /// Representation of an argument specification.
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub struct Argument<'a> {
  /// Where to find this argument
  pub position: Position,
@@ -54,7 +54,7 @@ pub struct Argument<'a> {
 }
 
 /// Specification for the formatting of an argument in the format string.
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub struct FormatSpec<'a> {
  /// Optionally specified character to fill alignment with.
  pub fill: Option<char>,
@@ -74,10 +74,12 @@ pub struct FormatSpec<'a> {
  /// this argument, this can be empty or any number of characters, although
  /// it is required to be one word.
  pub ty: &'a str,
+ /// The span of the descriptor string (for diagnostics).
+ pub ty_span: Option<InnerSpan>,
 }
 
 /// Enum describing where an argument for a format can be located.
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum Position {
  /// The argument is implied to be located at an index
  ArgumentImplicitlyIs(usize),
@@ -97,7 +99,7 @@ impl Position {
 }
 
 /// Enum of alignments which are supported.
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum Alignment {
  /// The value will be aligned to the left.
  AlignLeft,
@@ -111,7 +113,7 @@ pub enum Alignment {
 
 /// Various flags which can be applied to format strings. The meaning of these
 /// flags is defined by the formatters themselves.
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum Flag {
  /// A `+` will be used to denote positive numbers.
  FlagSignPlus,
@@ -131,7 +133,7 @@ pub enum Flag {
 
 /// A count is used for the precision and width parameters of an integer, and
 /// can reference either an argument or a literal integer.
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum Count {
  /// The count is specified explicitly.
  CountIs(usize),
@@ -475,6 +477,7 @@ impl<'a> Parser<'a> {
  width: CountImplied,
  width_span: None,
  ty: &self.input[..0],
+ ty_span: None,
  };
  if !self.consume(':') {
  return spec;
@@ -548,6 +551,7 @@ impl<'a> Parser<'a> {
  spec.precision_span = sp;
  }
  }
+ let ty_span_start = self.cur.peek().map(|(pos, _)| *pos);
  // Optional radix followed by the actual format specifier
  if self.consume('x') {
  if self.consume('?') {
@@ -567,6 +571,12 @@ impl<'a> Parser<'a> {
  spec.ty = "?";
  } else {
  spec.ty = self.word();
+ let ty_span_end = self.cur.peek().map(|(pos, _)| *pos);
+ if !spec.ty.is_empty() {
+ spec.ty_span = ty_span_start
+ .and_then(|s| ty_span_end.map(|e| (s, e)))
+ .map(|(start, end)| self.to_span_index(start).to(self.to_span_index(end)));
+ }
  }
  spec
  }

src/libfmt_macros/tests.rs

@@ -2,7 +2,7 @@ use super::*;
 
 fn same(fmt: &'static str, p: &[Piece<'static>]) {
  let parser = Parser::new(fmt, None, vec![], false);
- assert!(parser.collect::<Vec<Piece<'static>>>() == p);
+ assert_eq!(parser.collect::<Vec<Piece<'static>>>(), p);
 }
 
 fn fmtdflt() -> FormatSpec<'static> {
@@ -15,6 +15,7 @@ fn fmtdflt() -> FormatSpec<'static> {
  precision_span: None,
  width_span: None,
  ty: "",
+ ty_span: None,
  };
 }
 
@@ -82,7 +83,7 @@ fn format_position_nothing_else() {
 #[test]
 fn format_type() {
  same(
- "{3:a}",
+ "{3:x}",
  &[NextArgument(Argument {
  position: ArgumentIs(3),
  format: FormatSpec {
@@ -93,7 +94,8 @@ fn format_type() {
  width: CountImplied,
  precision_span: None,
  width_span: None,
- ty: "a",
+ ty: "x",
+ ty_span: None,
  },
  })]);
 }
@@ -112,6 +114,7 @@ fn format_align_fill() {
  precision_span: None,
  width_span: None,
  ty: "",
+ ty_span: None,
  },
  })]);
  same(
@@ -127,6 +130,7 @@ fn format_align_fill() {
  precision_span: None,
  width_span: None,
  ty: "",
+ ty_span: None,
  },
  })]);
  same(
@@ -142,6 +146,7 @@ fn format_align_fill() {
  precision_span: None,
  width_span: None,
  ty: "abcd",
+ ty_span: Some(InnerSpan::new(6, 10)),
  },
  })]);
 }
@@ -150,7 +155,7 @@ fn format_counts() {
  use syntax_pos::{GLOBALS, Globals, edition};
  GLOBALS.set(&Globals::new(edition::DEFAULT_EDITION), || {
  same(
- "{:10s}",
+ "{:10x}",
  &[NextArgument(Argument {
  position: ArgumentImplicitlyIs(0),
  format: FormatSpec {
@@ -161,11 +166,12 @@ fn format_counts() {
  width: CountIs(10),
  precision_span: None,
  width_span: None,
- ty: "s",
+ ty: "x",
+ ty_span: None,
  },
  })]);
  same(
- "{:10$.10s}",
+ "{:10$.10x}",
  &[NextArgument(Argument {
  position: ArgumentImplicitlyIs(0),
  format: FormatSpec {
@@ -176,11 +182,12 @@ fn format_counts() {
  width: CountIsParam(10),
  precision_span: None,
  width_span: Some(InnerSpan::new(3, 6)),
- ty: "s",
+ ty: "x",
+ ty_span: None,
  },
  })]);
  same(
- "{:.*s}",
+ "{:.*x}",
  &[NextArgument(Argument {
  position: ArgumentImplicitlyIs(1),
  format: FormatSpec {
@@ -191,11 +198,12 @@ fn format_counts() {
  width: CountImplied,
  precision_span: Some(InnerSpan::new(3, 5)),
  width_span: None,
- ty: "s",
+ ty: "x",
+ ty_span: None,
  },
  })]);
  same(
- "{:.10$s}",
+ "{:.10$x}",
  &[NextArgument(Argument {
  position: ArgumentImplicitlyIs(0),
  format: FormatSpec {
@@ -206,11 +214,12 @@ fn format_counts() {
  width: CountImplied,
  precision_span: Some(InnerSpan::new(3, 7)),
  width_span: None,
- ty: "s",
+ ty: "x",
+ ty_span: None,
  },
  })]);
  same(
- "{:a$.b$s}",
+ "{:a$.b$?}",
  &[NextArgument(Argument {
  position: ArgumentImplicitlyIs(0),
  format: FormatSpec {
@@ -221,7 +230,8 @@ fn format_counts() {
  width: CountIsName(Symbol::intern("a")),
  precision_span: None,
  width_span: None,
- ty: "s",
+ ty: "?",
+ ty_span: None,
  },
  })]);
  });
@@ -241,6 +251,7 @@ fn format_flags() {
  precision_span: None,
  width_span: None,
  ty: "",
+ ty_span: None,
  },
  })]);
  same(
@@ -256,13 +267,14 @@ fn format_flags() {
  precision_span: None,
  width_span: None,
  ty: "",
+ ty_span: None,
  },
  })]);
 }
 #[test]
 fn format_mixture() {
  same(
- "abcd {3:a} efg",
+ "abcd {3:x} efg",
  &[
  String("abcd "),
  NextArgument(Argument {
@@ -275,7 +287,8 @@ fn format_mixture() {
  width: CountImplied,
  precision_span: None,
  width_span: None,
- ty: "a",
+ ty: "x",
+ ty_span: None,
  },
  }),
  String(" efg"),

src/libsyntax_ext/format.rs

@@ -21,7 +21,7 @@ use std::collections::hash_map::Entry;
 
 #[derive(PartialEq)]
 enum ArgumentType {
- Placeholder(String),
+ Placeholder(&'static str),
  Count,
 }
 
@@ -244,7 +244,57 @@ impl<'a, 'b> Context<'a, 'b> {
  parse::ArgumentNamed(s) => Named(s),
  };
 
- let ty = Placeholder(arg.format.ty.to_string());
+ let ty = Placeholder(match &arg.format.ty[..] {
+ "" => "Display",
+ "?" => "Debug",
+ "e" => "LowerExp",
+ "E" => "UpperExp",
+ "o" => "Octal",
+ "p" => "Pointer",
+ "b" => "Binary",
+ "x" => "LowerHex",
+ "X" => "UpperHex",
+ _ => {
+ let fmtsp = self.fmtsp;
+ let sp = arg.format.ty_span.map(|sp| fmtsp.from_inner(sp));
+ let mut err = self.ecx.struct_span_err(
+ sp.unwrap_or(fmtsp),
+ &format!("unknown format trait `{}`", arg.format.ty),
+ );
+ 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");
+ if let Some(sp) = sp {
+ for (fmt, name) in &[
+ ("", "Display"),
+ ("?", "Debug"),
+ ("e", "LowerExp"),
+ ("E", "UpperExp"),
+ ("o", "Octal"),
+ ("p", "Pointer"),
+ ("b", "Binary"),
+ ("x", "LowerHex"),
+ ("X", "UpperHex"),
+ ] {
+ err.tool_only_span_suggestion(
+ sp,
+ &format!("use the `{}` trait", name),
+ fmt.to_string(),
+ Applicability::MaybeIncorrect,
+ );
+ }
+ }
+ err.emit();
+ "<invalid>"
+ }
+ });
  self.verify_arg_type(pos, ty);
  self.curpiece += 1;
  }
@@ -590,6 +640,7 @@ impl<'a, 'b> Context<'a, 'b> {
  width: parse::CountImplied,
  width_span: None,
  ty: arg.format.ty,
+ ty_span: arg.format.ty_span,
  },
  };
 
@@ -761,37 +812,8 @@ impl<'a, 'b> Context<'a, 'b> {
  sp = ecx.with_def_site_ctxt(sp);
  let arg = ecx.expr_ident(sp, arg);
  let trait_ = match *ty {
- Placeholder(ref tyname) => {
- match &tyname[..] {
- "" => "Display",
- "?" => "Debug",
- "e" => "LowerExp",
- "E" => "UpperExp",
- "o" => "Octal",
- "p" => "Pointer",
- "b" => "Binary",
- "x" => "LowerHex",
- "X" => "UpperHex",
- _ => {
- 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);
- }
- }
- }
+ Placeholder(trait_) if trait_ == "<invalid>" => return DummyResult::raw_expr(sp, true),
+ Placeholder(trait_) => trait_,
  Count => {
  let path = ecx.std_path(&[sym::fmt, sym::ArgumentV1, sym::from_usize]);
  return ecx.expr_call_global(macsp, path, vec![arg]);

src/test/ui/if/ifmt-bad-arg.stderr

@@ -257,10 +257,10 @@ LL | println!("{} {:07$} {}", 1, 3.2, 4);
  = 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
+ --> $DIR/ifmt-bad-arg.rs:86:17
  |
 LL | println!("{:foo}", 1);
- |  ^
+ | ^^^
  |
  = note: the only appropriate formatting traits are:
  - ``, which uses the `Display` trait