Skip to content

Commit b619dcd

Browse files
authored
Rollup merge of rust-lang#37613 - DanielKeep:eww-you-got-printf-in-your-format, r=alexcrichton
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. ----- **Example**: given: ```rust fn main() { println!("%.*3$s %s!\n", "Hello,", "World", 4); println!("%1$*2$.*3$f", 123.456); } ``` The compiler outputs the following: ```text error: multiple unused formatting arguments --> local/fmt.rs:2:5 | 2 | println!("%.*3$s %s!\n", "Hello,", "World", 4); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: argument never used --> local/fmt.rs:2:30 | 2 | println!("%.*3$s %s!\n", "Hello,", "World", 4); | ^^^^^^^^ note: argument never used --> local/fmt.rs:2:40 | 2 | println!("%.*3$s %s!\n", "Hello,", "World", 4); | ^^^^^^^ note: argument never used --> local/fmt.rs:2:49 | 2 | println!("%.*3$s %s!\n", "Hello,", "World", 4); | ^ = help: `%.*3$s` should be written as `{:.2$}` = help: `%s` should be written as `{}` = note: printf formatting not supported; see the documentation for `std::fmt` = note: this error originates in a macro outside of the current crate error: argument never used --> local/fmt.rs:6:29 | 6 | println!("%1$*2$.*3$f", 123.456); | ^^^^^^^ | = help: `%1$*2$.*3$f` should be written as `{0:1$.2$}` = note: printf formatting not supported; see the documentation for `std::fmt` ```
2 parents 25f1dee + 455723c commit b619dcd

File tree

6 files changed

+1163
-2
lines changed

6 files changed

+1163
-2
lines changed

src/libsyntax_ext/format.rs

+74-2
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
}

0 commit comments

Comments
 (0)