Skip to content

Commit

Permalink
Add foreign formatting directive detection.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DanielKeep committed Nov 11, 2016
1 parent e96b9d2 commit 455723c
Show file tree
Hide file tree
Showing 6 changed files with 1,163 additions and 2 deletions.
76 changes: 74 additions & 2 deletions src/libsyntax_ext/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use syntax::ptr::P;
use syntax_pos::{Span, DUMMY_SP};
use syntax::tokenstream;

use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::collections::hash_map::Entry;

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

// Make sure that all arguments were used and all arguments have types.
let num_pos_args = cx.args.len() - cx.names.len();
let mut errs = vec![];
for (i, ty) in cx.arg_types.iter().enumerate() {
if ty.len() == 0 {
if cx.count_positions.contains_key(&i) {
Expand All @@ -779,9 +780,80 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
// positional argument
"argument never used"
};
cx.ecx.span_err(cx.args[i].span, msg);
errs.push((cx.args[i].span, msg));
}
}
if errs.len() > 0 {
let args_used = cx.arg_types.len() - errs.len();
let args_unused = errs.len();

let mut diag = {
if errs.len() == 1 {
let (sp, msg) = errs.into_iter().next().unwrap();
cx.ecx.struct_span_err(sp, msg)
} else {
let mut diag = cx.ecx.struct_span_err(cx.fmtsp,
"multiple unused formatting arguments");
for (sp, msg) in errs {
diag.span_note(sp, msg);
}
diag
}
};

// Decide if we want to look for foreign formatting directives.
if args_used < args_unused {
use super::format_foreign as foreign;
let fmt_str = &fmt.node.0[..];

// The set of foreign substitutions we've explained. This prevents spamming the user
// with `%d should be written as {}` over and over again.
let mut explained = HashSet::new();

// Used to ensure we only report translations for *one* kind of foreign format.
let mut found_foreign = false;

macro_rules! check_foreign {
($kind:ident) => {{
let mut show_doc_note = false;

for sub in foreign::$kind::iter_subs(fmt_str) {
let trn = match sub.translate() {
Some(trn) => trn,

// If it has no translation, don't call it out specifically.
None => continue,
};

let sub = String::from(sub.as_str());
if explained.contains(&sub) {
continue;
}
explained.insert(sub.clone());

if !found_foreign {
found_foreign = true;
show_doc_note = true;
}

diag.help(&format!("`{}` should be written as `{}`", sub, trn));
}

if show_doc_note {
diag.note(concat!(stringify!($kind), " formatting not supported; see \
the documentation for `std::fmt`"));
}
}};
}

check_foreign!(printf);
if !found_foreign {
check_foreign!(shell);
}
}

diag.emit();
}

cx.into_expr()
}
Loading

0 comments on commit 455723c

Please sign in to comment.