Skip to content

Commit b577b9a

Browse files
tommyiptommyip
committed
Retain information on whether a format argument has explicit position
1 parent 13a4162 commit b577b9a

File tree

4 files changed

+57
-32
lines changed

4 files changed

+57
-32
lines changed

‎src/libfmt_macros/lib.rs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ pub struct FormatSpec<'a> {
7373
/// Enum describing where an argument for a format can be located.
7474
#[derive(Copy, Clone, PartialEq)]
7575
pub enum Position<'a> {
76-
/// The argument is located at a specific index.
76+
/// The arugment is implied to be located at an index
77+
ArgumentImplicitlyIs(usize),
78+
/// The argument is located at a specific index given in the format
7779
ArgumentIs(usize),
7880
/// The argument has a name.
7981
ArgumentNamed(&'a str),
@@ -275,7 +277,7 @@ impl<'a> Parser<'a> {
275277
None => {
276278
let i = self.curarg;
277279
self.curarg += 1;
278-
ArgumentIs(i)
280+
ArgumentImplicitlyIs(i)
279281
}
280282
};
281283

@@ -517,7 +519,7 @@ mod tests {
517519
fn format_nothing() {
518520
same("{}",
519521
&[NextArgument(Argument {
520-
position: ArgumentIs(0),
522+
position: ArgumentImplicitlyIs(0),
521523
format: fmtdflt(),
522524
})]);
523525
}
@@ -595,7 +597,7 @@ mod tests {
595597
fn format_counts() {
596598
same("{:10s}",
597599
&[NextArgument(Argument {
598-
position: ArgumentIs(0),
600+
position: ArgumentImplicitlyIs(0),
599601
format: FormatSpec {
600602
fill: None,
601603
align: AlignUnknown,
@@ -607,7 +609,7 @@ mod tests {
607609
})]);
608610
same("{:10$.10s}",
609611
&[NextArgument(Argument {
610-
position: ArgumentIs(0),
612+
position: ArgumentImplicitlyIs(0),
611613
format: FormatSpec {
612614
fill: None,
613615
align: AlignUnknown,
@@ -619,7 +621,7 @@ mod tests {
619621
})]);
620622
same("{:.*s}",
621623
&[NextArgument(Argument {
622-
position: ArgumentIs(1),
624+
position: ArgumentImplicitlyIs(1),
623625
format: FormatSpec {
624626
fill: None,
625627
align: AlignUnknown,
@@ -631,7 +633,7 @@ mod tests {
631633
})]);
632634
same("{:.10$s}",
633635
&[NextArgument(Argument {
634-
position: ArgumentIs(0),
636+
position: ArgumentImplicitlyIs(0),
635637
format: FormatSpec {
636638
fill: None,
637639
align: AlignUnknown,
@@ -643,7 +645,7 @@ mod tests {
643645
})]);
644646
same("{:a$.b$s}",
645647
&[NextArgument(Argument {
646-
position: ArgumentIs(0),
648+
position: ArgumentImplicitlyIs(0),
647649
format: FormatSpec {
648650
fill: None,
649651
align: AlignUnknown,
@@ -658,7 +660,7 @@ mod tests {
658660
fn format_flags() {
659661
same("{:-}",
660662
&[NextArgument(Argument {
661-
position: ArgumentIs(0),
663+
position: ArgumentImplicitlyIs(0),
662664
format: FormatSpec {
663665
fill: None,
664666
align: AlignUnknown,
@@ -670,7 +672,7 @@ mod tests {
670672
})]);
671673
same("{:+#}",
672674
&[NextArgument(Argument {
673-
position: ArgumentIs(0),
675+
position: ArgumentImplicitlyIs(0),
674676
format: FormatSpec {
675677
fill: None,
676678
align: AlignUnknown,

‎src/librustc/traits/on_unimplemented.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString {
254254
}
255255
},
256256
// `{:1}` and `{}` are not to be used
257-
Position::ArgumentIs(_) => {
257+
Position::ArgumentIs(_) | Position::ArgumentImplicitlyIs(_) => {
258258
span_err!(tcx.sess, span, E0231,
259259
"only named substitution \
260260
parameters are allowed");

‎src/libsyntax_ext/format.rs‎

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ impl<'a, 'b> Context<'a, 'b> {
228228
// argument second, if it's an implicit positional parameter
229229
// it's written second, so it should come after width/precision.
230230
let pos = match arg.position {
231-
parse::ArgumentIs(i) => Exact(i),
231+
parse::ArgumentIs(i) | parse::ArgumentImplicitlyIs(i) => Exact(i),
232232
parse::ArgumentNamed(s) => Named(s.to_string()),
233233
};
234234

@@ -254,25 +254,28 @@ impl<'a, 'b> Context<'a, 'b> {
254254
fn describe_num_args(&self) -> String {
255255
match self.args.len() {
256256
0 => "no arguments were given".to_string(),
257-
1 => "there is only 1 argument".to_string(),
258-
x => format!("there are only {} arguments", x),
257+
1 => "there is 1 argument".to_string(),
258+
x => format!("there are {} arguments", x),
259259
}
260260
}
261261

262262
/// Handle invalid references to positional arguments. Output different
263263
/// errors for the case where all arguments are positional and for when
264-
/// there are named arguments in the format string.
265-
fn report_invalid_references(&self) {
264+
/// there are named arguments or numbered positional arguments in the
265+
/// format string.
266+
fn report_invalid_references(&self, numbered_position_args: bool) {
267+
let mut e;
266268
let mut refs: Vec<String> = self.invalid_refs
267269
.iter()
268270
.map(|r| r.to_string())
269271
.collect();
270272

271-
let msg = if self.names.is_empty() {
272-
format!("{} positional argument{} in format string, but {}",
273-
self.pieces.len(),
274-
if self.pieces.len() > 1 { "s" } else { "" },
275-
self.describe_num_args())
273+
if self.names.is_empty() && !numbered_position_args {
274+
e = self.ecx.mut_span_err(self.fmtsp,
275+
&format!("{} positional argument{} in format string, but {}",
276+
self.pieces.len(),
277+
if self.pieces.len() > 1 { "s" } else { "" },
278+
self.describe_num_args()));
276279
} else {
277280
let arg_list = match refs.len() {
278281
1 => format!("argument {}", refs.pop().unwrap()),
@@ -281,12 +284,14 @@ impl<'a, 'b> Context<'a, 'b> {
281284
head=refs.join(", "))
282285
};
283286

284-
format!("invalid reference to positional {} ({})",
285-
arg_list,
286-
self.describe_num_args())
287+
e = self.ecx.mut_span_err(self.fmtsp,
288+
&format!("invalid reference to positional {} ({})",
289+
arg_list,
290+
self.describe_num_args()));
291+
e.note("positional arguments are zero-based");
287292
};
288293

289-
self.ecx.span_err(self.fmtsp, &msg[..]);
294+
e.emit();
290295
}
291296

292297
/// Actually verifies and tracks a given format placeholder
@@ -431,7 +436,8 @@ impl<'a, 'b> Context<'a, 'b> {
431436
}
432437
};
433438
match arg.position {
434-
parse::ArgumentIs(i) => {
439+
parse::ArgumentIs(i)
440+
| parse::ArgumentImplicitlyIs(i) => {
435441
// Map to index in final generated argument array
436442
// in case of multiple types specified
437443
let arg_idx = match arg_index_consumed.get_mut(i) {
@@ -740,6 +746,18 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
740746
}
741747
}
742748

749+
let numbered_position_args = pieces.iter().any(|arg: &parse::Piece| {
750+
match *arg {
751+
parse::String(_) => false,
752+
parse::NextArgument(arg) => {
753+
match arg.position {
754+
parse::Position::ArgumentIs(_) => true,
755+
_ => false,
756+
}
757+
}
758+
}
759+
});
760+
743761
cx.build_index_map();
744762

745763
let mut arg_index_consumed = vec![0usize; cx.arg_index_map.len()];
@@ -766,7 +784,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
766784
}
767785

768786
if cx.invalid_refs.len() >= 1 {
769-
cx.report_invalid_references();
787+
cx.report_invalid_references(numbered_position_args);
770788
}
771789

772790
// Make sure that all arguments were used and all arguments have types.

‎src/test/compile-fail/ifmt-bad-arg.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,26 @@ fn main() {
1717
//~^ ERROR: 1 positional argument in format string, but no arguments were given
1818

1919
format!("{1}", 1);
20-
//~^ ERROR: 1 positional argument in format string, but there is only 1 argument
20+
//~^ ERROR: invalid reference to positional argument 1 (there is 1 argument)
2121
//~^^ ERROR: argument never used
2222

2323
format!("{} {}");
2424
//~^ ERROR: 2 positional arguments in format string, but no arguments were given
2525

2626
format!("{0} {1}", 1);
27-
//~^ ERROR: 2 positional arguments in format string, but there is only 1 argument
27+
//~^ ERROR: invalid reference to positional argument 1 (there is 1 argument)
2828

2929
format!("{0} {1} {2}", 1, 2);
30-
//~^ ERROR: 3 positional arguments in format string, but there are only 2 arguments
30+
//~^ ERROR: invalid reference to positional argument 2 (there are 2 arguments)
3131

3232
format!("{} {value} {} {}", 1, value=2);
33-
//~^ ERROR: invalid reference to positional argument 2 (there are only 2 arguments)
33+
//~^ ERROR: invalid reference to positional argument 2 (there are 2 arguments)
3434
format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2);
35-
//~^ ERROR: invalid reference to positional arguments 3, 4 and 5 (there are only 3 arguments)
35+
//~^ ERROR: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments)
36+
37+
format!("{} {foo} {} {bar} {}", 1, 2, 3);
38+
//~^ ERROR: there is no argument named `foo`
39+
//~^^ ERROR: there is no argument named `bar`
3640

3741
format!("{foo}"); //~ ERROR: no argument named `foo`
3842
format!("", 1, 2); //~ ERROR: multiple unused formatting arguments
@@ -41,6 +45,7 @@ fn main() {
4145
format!("{}", 1, foo=2); //~ ERROR: named argument never used
4246
format!("{foo}", 1, foo=2); //~ ERROR: argument never used
4347
format!("", foo=2); //~ ERROR: named argument never used
48+
format!("{} {}", 1, 2, foo=1, bar=2); //~ ERROR: multiple unused formatting arguments
4449

4550
format!("{foo}", foo=1, foo=2); //~ ERROR: duplicate argument
4651
format!("", foo=1, 2); //~ ERROR: positional arguments cannot follow

0 commit comments

Comments
 (0)