Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 455723c

Browse files
committedNov 11, 2016
Add foreign formatting directive detection.
This teaches `format_args!` how to interpret format printf- and shell-style format directives. This is used in cases where there are unused formatting arguments, and the reason for that *might* be because the programmer is trying to use the wrong kind of formatting string. This was prompted by an issue encountered by simulacrum on the #rust IRC channel. In short: although `println!` told them that they weren't using all of the conversion arguments, the problem was in using printf-syle directives rather than ones `println!` would undertand. Where possible, `format_args!` will tell the programmer what they should use instead. For example, it will suggest replacing `%05d` with `{:0>5}`, or `%2$.*3$s` with `{1:.3$}`. Even if it cannot suggest a replacement, it will explicitly note that Rust does not support that style of directive, and direct the user to the `std::fmt` documentation.
1 parent e96b9d2 commit 455723c

File tree

6 files changed

+1163
-2
lines changed

6 files changed

+1163
-2
lines changed
 

‎src/libsyntax_ext/format.rs

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use syntax::ptr::P;
2222
use syntax_pos::{Span, DUMMY_SP};
2323
use syntax::tokenstream;
2424

25-
use std::collections::HashMap;
25+
use std::collections::{HashMap, HashSet};
2626
use std::collections::hash_map::Entry;
2727

2828
#[derive(PartialEq)]
@@ -767,6 +767,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
767767

768768
// Make sure that all arguments were used and all arguments have types.
769769
let num_pos_args = cx.args.len() - cx.names.len();
770+
let mut errs = vec![];
770771
for (i, ty) in cx.arg_types.iter().enumerate() {
771772
if ty.len() == 0 {
772773
if cx.count_positions.contains_key(&i) {
@@ -779,9 +780,80 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
779780
// positional argument
780781
"argument never used"
781782
};
782-
cx.ecx.span_err(cx.args[i].span, msg);
783+
errs.push((cx.args[i].span, msg));
783784
}
784785
}
786+
if errs.len() > 0 {
787+
let args_used = cx.arg_types.len() - errs.len();
788+
let args_unused = errs.len();
789+
790+
let mut diag = {
791+
if errs.len() == 1 {
792+
let (sp, msg) = errs.into_iter().next().unwrap();
793+
cx.ecx.struct_span_err(sp, msg)
794+
} else {
795+
let mut diag = cx.ecx.struct_span_err(cx.fmtsp,
796+
"multiple unused formatting arguments");
797+
for (sp, msg) in errs {
798+
diag.span_note(sp, msg);
799+
}
800+
diag
801+
}
802+
};
803+
804+
// Decide if we want to look for foreign formatting directives.
805+
if args_used < args_unused {
806+
use super::format_foreign as foreign;
807+
let fmt_str = &fmt.node.0[..];
808+
809+
// The set of foreign substitutions we've explained. This prevents spamming the user
810+
// with `%d should be written as {}` over and over again.
811+
let mut explained = HashSet::new();
812+
813+
// Used to ensure we only report translations for *one* kind of foreign format.
814+
let mut found_foreign = false;
815+
816+
macro_rules! check_foreign {
817+
($kind:ident) => {{
818+
let mut show_doc_note = false;
819+
820+
for sub in foreign::$kind::iter_subs(fmt_str) {
821+
let trn = match sub.translate() {
822+
Some(trn) => trn,
823+
824+
// If it has no translation, don't call it out specifically.
825+
None => continue,
826+
};
827+
828+
let sub = String::from(sub.as_str());
829+
if explained.contains(&sub) {
830+
continue;
831+
}
832+
explained.insert(sub.clone());
833+
834+
if !found_foreign {
835+
found_foreign = true;
836+
show_doc_note = true;
837+
}
838+
839+
diag.help(&format!("`{}` should be written as `{}`", sub, trn));
840+
}
841+
842+
if show_doc_note {
843+
diag.note(concat!(stringify!($kind), " formatting not supported; see \
844+
the documentation for `std::fmt`"));
845+
}
846+
}};
847+
}
848+
849+
check_foreign!(printf);
850+
if !found_foreign {
851+
check_foreign!(shell);
852+
}
853+
}
854+
855+
diag.emit();
856+
}
785857

786858
cx.into_expr()
787859
}

‎src/libsyntax_ext/format_foreign.rs

Lines changed: 1013 additions & 0 deletions
Large diffs are not rendered by default.

‎src/libsyntax_ext/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ mod concat;
4040
mod concat_idents;
4141
mod env;
4242
mod format;
43+
mod format_foreign;
4344
mod log_syntax;
4445
mod trace_macros;
4546

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ fn main() {
1717
//~^ ERROR: argument never used
1818
format!("{foo}"); //~ ERROR: no argument named `foo`
1919

20+
format!("", 1, 2); //~ ERROR: multiple unused formatting arguments
2021
format!("{}", 1, 2); //~ ERROR: argument never used
2122
format!("{1}", 1, 2); //~ ERROR: argument never used
2223
format!("{}", 1, foo=2); //~ ERROR: named argument never used
@@ -53,4 +54,6 @@ fn main() {
5354

5455
format!("foo } bar"); //~ ERROR: unmatched `}` found
5556
format!("foo }"); //~ ERROR: unmatched `}` found
57+
58+
format!("foo %s baz", "bar"); //~ ERROR: argument never used
5659
}

‎src/test/ui/macros/format-foreign.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
fn main() {
12+
println!("%.*3$s %s!\n", "Hello,", "World", 4);
13+
println!("%1$*2$.*3$f", 123.456);
14+
15+
// This should *not* produce hints, on the basis that there's equally as
16+
// many "correct" format specifiers. It's *probably* just an actual typo.
17+
println!("{} %f", "one", 2.0);
18+
19+
println!("Hi there, $NAME.", NAME="Tim");
20+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
error: multiple unused formatting arguments
2+
--> $DIR/format-foreign.rs:12:5
3+
|
4+
12 | println!("%.*3$s %s!/n", "Hello,", "World", 4);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
note: argument never used
8+
--> $DIR/format-foreign.rs:12:30
9+
|
10+
12 | println!("%.*3$s %s!/n", "Hello,", "World", 4);
11+
| ^^^^^^^^
12+
note: argument never used
13+
--> $DIR/format-foreign.rs:12:40
14+
|
15+
12 | println!("%.*3$s %s!/n", "Hello,", "World", 4);
16+
| ^^^^^^^
17+
note: argument never used
18+
--> $DIR/format-foreign.rs:12:49
19+
|
20+
12 | println!("%.*3$s %s!/n", "Hello,", "World", 4);
21+
| ^
22+
= help: `%.*3$s` should be written as `{:.2$}`
23+
= help: `%s` should be written as `{}`
24+
= note: printf formatting not supported; see the documentation for `std::fmt`
25+
= note: this error originates in a macro outside of the current crate
26+
27+
error: argument never used
28+
--> $DIR/format-foreign.rs:13:29
29+
|
30+
13 | println!("%1$*2$.*3$f", 123.456);
31+
| ^^^^^^^
32+
|
33+
= help: `%1$*2$.*3$f` should be written as `{0:1$.2$}`
34+
= note: printf formatting not supported; see the documentation for `std::fmt`
35+
36+
error: argument never used
37+
--> $DIR/format-foreign.rs:17:30
38+
|
39+
17 | println!("{} %f", "one", 2.0);
40+
| ^^^
41+
42+
error: named argument never used
43+
--> $DIR/format-foreign.rs:19:39
44+
|
45+
19 | println!("Hi there, $NAME.", NAME="Tim");
46+
| ^^^^^
47+
|
48+
= help: `$NAME` should be written as `{NAME}`
49+
= note: shell formatting not supported; see the documentation for `std::fmt`
50+
51+
error: aborting due to 4 previous errors
52+

0 commit comments

Comments
 (0)
Please sign in to comment.