Skip to content

Commit

Permalink
Rollup merge of rust-lang#66134 - estebank:unknown-formatting-trait, …
Browse files Browse the repository at this point in the history 
…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
```
  • Loading branch information
@Centril
Centril committed Nov 9, 2019
2 parents 523a7d5 + 543fe5b commit 7c73ea6
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 59 deletions.
24 changes: 17 additions & 7 deletions src/libfmt_macros/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
Expand All @@ -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>,
Expand All @@ -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),
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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('?') {
Expand All @@ -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
}
Expand Down
43 changes: 28 additions & 15 deletions src/libfmt_macros/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -15,6 +15,7 @@ fn fmtdflt() -> FormatSpec<'static> {
precision_span: None,
width_span: None,
ty: "",
ty_span: None,
};
}

Expand Down Expand Up @@ -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 {
Expand All @@ -93,7 +94,8 @@ fn format_type() {
width: CountImplied,
precision_span: None,
width_span: None,
ty: "a",
ty: "x",
ty_span: None,
},
})]);
}
Expand All @@ -112,6 +114,7 @@ fn format_align_fill() {
precision_span: None,
width_span: None,
ty: "",
ty_span: None,
},
})]);
same(
Expand All @@ -127,6 +130,7 @@ fn format_align_fill() {
precision_span: None,
width_span: None,
ty: "",
ty_span: None,
},
})]);
same(
Expand All @@ -142,6 +146,7 @@ fn format_align_fill() {
precision_span: None,
width_span: None,
ty: "abcd",
ty_span: Some(InnerSpan::new(6, 10)),
},
})]);
}
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -221,7 +230,8 @@ fn format_counts() {
width: CountIsName(Symbol::intern("a")),
precision_span: None,
width_span: None,
ty: "s",
ty: "?",
ty_span: None,
},
})]);
});
Expand All @@ -241,6 +251,7 @@ fn format_flags() {
precision_span: None,
width_span: None,
ty: "",
ty_span: None,
},
})]);
same(
Expand All @@ -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 {
Expand All @@ -275,7 +287,8 @@ fn format_mixture() {
width: CountImplied,
precision_span: None,
width_span: None,
ty: "a",
ty: "x",
ty_span: None,
},
}),
String(" efg"),
Expand Down
88 changes: 55 additions & 33 deletions src/libsyntax_ext/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::collections::hash_map::Entry;

#[derive(PartialEq)]
enum ArgumentType {
Placeholder(String),
Placeholder(&'static str),
Count,
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
},
};

Expand Down Expand Up @@ -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]);
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/if/ifmt-bad-arg.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 7c73ea6

Please sign in to comment.