rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Point only at invalid positional arguments

Browse Source
This commit is contained in:
Esteban Küber
2018-07-23 15:09:00 -07:00
parent 42306591b9
commit 6bcf8777fe
2 changed files with 84 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

138
src/libsyntax_ext/format.rs
View File

@@ -14,18 +14,18 @@ use self::Position::*;
use fmt_macros as parse; use fmt_macros as parse;
use syntax::ast; use syntax::ast;
use syntax::ext::base::*;
use syntax::ext::base; use syntax::ext::base;
use syntax::ext::base::*;
use syntax::ext::build::AstBuilder; use syntax::ext::build::AstBuilder;
use syntax::feature_gate; use syntax::feature_gate;
use syntax::parse::token; use syntax::parse::token;
use syntax::ptr::P; use syntax::ptr::P;
use syntax::symbol::Symbol; use syntax::symbol::Symbol;
use syntax_pos::{Span, MultiSpan, DUMMY_SP};
use syntax::tokenstream; use syntax::tokenstream;
use syntax_pos::{MultiSpan, Span, DUMMY_SP};
use std::collections::{HashMap, HashSet};
use std::collections::hash_map::Entry; use std::collections::hash_map::Entry;
use std::collections::{HashMap, HashSet};
#[derive(PartialEq)] #[derive(PartialEq)]
enum ArgumentType { enum ArgumentType {
@@ -111,9 +111,11 @@ struct Context<'a, 'b: 'a> {
/// still existed in this phase of processing. /// still existed in this phase of processing.
/// Used only for `all_pieces_simple` tracking in `build_piece`. /// Used only for `all_pieces_simple` tracking in `build_piece`.
curarg: usize, curarg: usize,
/// Current piece being evaluated, used for error reporting.
curpiece: usize, curpiece: usize,
/// Keep track of invalid references to positional arguments /// Keep track of invalid references to positional arguments.
invalid_refs: Vec<usize>, invalid_refs: Vec<(usize, usize)>,
/// Spans of all the formatting arguments, in order.
arg_spans: Vec<Span>, arg_spans: Vec<Span>,
} }
@@ -157,15 +159,20 @@ fn parse_args(ecx: &mut ExtCtxt,
i i
} }
_ if named => { _ if named => {
ecx.span_err(p.span, ecx.span_err(
"expected ident, positional arguments \ p.span,
cannot follow named arguments"); "expected ident, positional arguments cannot follow named arguments",
);
return None; return None;
} }
_ => { _ => {
ecx.span_err(p.span, ecx.span_err(
&format!("expected ident for named argument, found `{}`", p.span,
p.this_token_to_string())); &format!(
"expected ident for named argument, found `{}`",
p.this_token_to_string()
),
);
return None; return None;
} }
}; };
@@ -267,34 +274,47 @@ impl<'a, 'b> Context<'a, 'b> {
/// errors for the case where all arguments are positional and for when /// errors for the case where all arguments are positional and for when
/// there are named arguments or numbered positional arguments in the /// there are named arguments or numbered positional arguments in the
/// format string. /// format string.
fn report_invalid_references(&self, numbered_position_args: bool, arg_places: &[(usize, usize)]) { fn report_invalid_references(&self, numbered_position_args: bool) {
let mut e; let mut e;
let sps = arg_places.iter() let sp = MultiSpan::from_spans(self.arg_spans.clone());
.map(|&(start, end)| self.fmtsp.from_inner_byte_pos(start, end)) let mut refs: Vec<_> = self
.collect::<Vec<_>>(); .invalid_refs
let sp = MultiSpan::from_spans(sps);
let mut refs: Vec<_> = self.invalid_refs
.iter() .iter()
.map(|r| r.to_string()) .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos)))
.collect(); .collect();
if self.names.is_empty() && !numbered_position_args { if self.names.is_empty() && !numbered_position_args {
e = self.ecx.mut_span_err(sp, e = self.ecx.mut_span_err(
&format!("{} positional argument{} in format string, but {}", sp,
&format!(
"{} positional argument{} in format string, but {}",
self.pieces.len(), self.pieces.len(),
if self.pieces.len() > 1 { "s" } else { "" }, if self.pieces.len() > 1 { "s" } else { "" },
self.describe_num_args())); self.describe_num_args()
),
);
} else { } else {
let arg_list = match refs.len() { let (arg_list, sp) = match refs.len() {
1 => { 1 => {
let reg = refs.pop().unwrap(); let (reg, pos) = refs.pop().unwrap();
format!("argument {}", reg) (
}, format!("argument {}", reg),
MultiSpan::from_span(*pos.unwrap_or(&self.fmtsp)),
)
}
_ => { _ => {
let pos =
MultiSpan::from_spans(refs.iter().map(|(_, p)| *p.unwrap()).collect());
let mut refs: Vec<String> = refs.iter().map(|(s, _)| s.to_owned()).collect();
let reg = refs.pop().unwrap(); let reg = refs.pop().unwrap();
format!("arguments {head} and {tail}", (
tail=reg, format!(
head=refs.join(", ")) "arguments {head} and {tail}",
tail = reg,
head = refs.join(", ")
),
pos,
)
} }
}; };
@@ -314,7 +334,7 @@ impl<'a, 'b> Context<'a, 'b> {
match arg { match arg {
Exact(arg) => { Exact(arg) => {
if self.args.len() <= arg { if self.args.len() <= arg {
self.invalid_refs.push(arg); self.invalid_refs.push((arg, self.curpiece));
return; return;
} }
match ty { match ty {
@@ -520,33 +540,27 @@ impl<'a, 'b> Context<'a, 'b> {
let prec = self.build_count(arg.format.precision); let prec = self.build_count(arg.format.precision);
let width = self.build_count(arg.format.width); let width = self.build_count(arg.format.width);
let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "FormatSpec")); let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "FormatSpec"));
let fmt = let fmt = self.ecx.expr_struct(
self.ecx.expr_struct(sp, sp,
path, path,
vec![self.ecx vec![
.field_imm(sp, self.ecx.ident_of("fill"), fill), self.ecx.field_imm(sp, self.ecx.ident_of("fill"), fill),
self.ecx.field_imm(sp, self.ecx.field_imm(sp, self.ecx.ident_of("align"), align),
self.ecx.ident_of("align"), self.ecx.field_imm(sp, self.ecx.ident_of("flags"), flags),
align), self.ecx.field_imm(sp, self.ecx.ident_of("precision"), prec),
self.ecx.field_imm(sp, self.ecx.field_imm(sp, self.ecx.ident_of("width"), width),
self.ecx.ident_of("flags"), ],
flags), );
self.ecx.field_imm(sp,
self.ecx.ident_of("precision"),
prec),
self.ecx.field_imm(sp,
self.ecx.ident_of("width"),
width)]);
let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "Argument")); let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "Argument"));
Some(self.ecx.expr_struct(sp, Some(self.ecx.expr_struct(
sp,
path, path,
vec![self.ecx.field_imm(sp, vec![
self.ecx.ident_of("position"), self.ecx.field_imm(sp, self.ecx.ident_of("position"), pos),
pos), self.ecx.field_imm(sp, self.ecx.ident_of("format"), fmt),
self.ecx.field_imm(sp, ],
self.ecx.ident_of("format"), ))
fmt)]))
} }
} }
} }
@@ -559,9 +573,9 @@ impl<'a, 'b> Context<'a, 'b> {
let mut pats = Vec::new(); let mut pats = Vec::new();
let mut heads = Vec::new(); let mut heads = Vec::new();
let names_pos: Vec<_> = (0..self.args.len()).map(|i| { let names_pos: Vec<_> = (0..self.args.len())
self.ecx.ident_of(&format!("arg{}", i)).gensym() .map(|i| self.ecx.ident_of(&format!("arg{}", i)).gensym())
}).collect(); .collect();
// First, build up the static array which will become our precompiled // First, build up the static array which will become our precompiled
// format "string" // format "string"
@@ -705,10 +719,11 @@ pub fn expand_format_args<'cx>(ecx: &'cx mut ExtCtxt,
} }
} }
pub fn expand_format_args_nl<'cx>(ecx: &'cx mut ExtCtxt, pub fn expand_format_args_nl<'cx>(
ecx: &'cx mut ExtCtxt,
mut sp: Span, mut sp: Span,
tts: &[tokenstream::TokenTree]) tts: &[tokenstream::TokenTree],
-> Box<dyn base::MacResult + 'cx> { ) -> Box<dyn base::MacResult + 'cx> {
//if !ecx.ecfg.enable_allow_internal_unstable() { //if !ecx.ecfg.enable_allow_internal_unstable() {
// For some reason, the only one that actually works for `println` is the first check // For some reason, the only one that actually works for `println` is the first check
@@ -759,7 +774,6 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
let sugg_fmt = match args.len() { let sugg_fmt = match args.len() {
0 => "{}".to_string(), 0 => "{}".to_string(),
_ => format!("{}{{}}", "{} ".repeat(args.len())), _ => format!("{}{{}}", "{} ".repeat(args.len())),
}; };
err.span_suggestion( err.span_suggestion(
fmt_sp.shrink_to_lo(), fmt_sp.shrink_to_lo(),
@@ -768,7 +782,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
); );
err.emit(); err.emit();
return DummyResult::raw_expr(sp); return DummyResult::raw_expr(sp);
}, }
}; };
let mut cx = Context { let mut cx = Context {
@@ -862,7 +876,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
} }
if cx.invalid_refs.len() >= 1 { if cx.invalid_refs.len() >= 1 {
cx.report_invalid_references(numbered_position_args, &parser.arg_places); cx.report_invalid_references(numbered_position_args);
} }
// Make sure that all arguments were used and all arguments have types. // Make sure that all arguments were used and all arguments have types.
@@ -894,7 +908,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
} else { } else {
let mut diag = cx.ecx.struct_span_err( let mut diag = cx.ecx.struct_span_err(
errs.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(), errs.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(),
"multiple unused formatting arguments" "multiple unused formatting arguments",
); );
diag.span_label(cx.fmtsp, "multiple missing formatting arguments"); diag.span_label(cx.fmtsp, "multiple missing formatting arguments");
diag diag

16
src/test/ui/ifmt-bad-arg.stderr
View File

@@ -25,34 +25,34 @@ LL | format!("{} {}");
| ^^ ^^ | ^^ ^^
error: invalid reference to positional argument 1 (there is 1 argument) error: invalid reference to positional argument 1 (there is 1 argument)
--> $DIR/ifmt-bad-arg.rs:26:14 --> $DIR/ifmt-bad-arg.rs:26:18
| |
LL | format!("{0} {1}", 1); LL | format!("{0} {1}", 1);
| ^^^ ^^^ | ^^^
| |
= note: positional arguments are zero-based = note: positional arguments are zero-based
error: invalid reference to positional argument 2 (there are 2 arguments) error: invalid reference to positional argument 2 (there are 2 arguments)
--> $DIR/ifmt-bad-arg.rs:29:14 --> $DIR/ifmt-bad-arg.rs:29:22
| |
LL | format!("{0} {1} {2}", 1, 2); LL | format!("{0} {1} {2}", 1, 2);
| ^^^ ^^^ ^^^ | ^^^
| |
= note: positional arguments are zero-based = note: positional arguments are zero-based
error: invalid reference to positional argument 2 (there are 2 arguments) error: invalid reference to positional argument 2 (there are 2 arguments)
--> $DIR/ifmt-bad-arg.rs:32:14 --> $DIR/ifmt-bad-arg.rs:32:28
| |
LL | format!("{} {value} {} {}", 1, value=2); LL | format!("{} {value} {} {}", 1, value=2);
| ^^ ^^^^^^^ ^^ ^^ | ^^
| |
= note: positional arguments are zero-based = note: positional arguments are zero-based
error: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments) error: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments)
--> $DIR/ifmt-bad-arg.rs:34:14 --> $DIR/ifmt-bad-arg.rs:34:38
| |
LL | format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2); LL | format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2);
| ^^^^^^ ^^^^^^^ ^^ ^^ ^^ ^^ ^^ ^^ | ^^ ^^ ^^
| |
= note: positional arguments are zero-based = note: positional arguments are zero-based