Skip to content

Commit e5929a8

Browse files
JarchoJarcho
committed
Keep track of spans in format strings
1 parent ac2f041 commit e5929a8

File tree

1 file changed

+148
-90
lines changed

1 file changed

+148
-90
lines changed

‎clippy_lints/src/write.rs

Lines changed: 148 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
use std::borrow::Cow;
22
use std::ops::Range;
33

4-
use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then};
5-
use if_chain::if_chain;
6-
use rustc_ast::ast::{Expr, ExprKind, ImplKind, Item, ItemKind, LitKind, MacCall, StrLit, StrStyle};
4+
use crate::utils::{snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then};
5+
use rustc_ast::ast::{Expr, ExprKind, ImplKind, Item, ItemKind, LitKind, MacCall, Path, StrLit, StrStyle};
76
use rustc_ast::token;
87
use rustc_ast::tokenstream::TokenStream;
98
use rustc_errors::Applicability;
109
use rustc_lexer::unescape::{self, EscapeError};
1110
use rustc_lint::{EarlyContext, EarlyLintPass};
1211
use rustc_parse::parser;
1312
use rustc_session::{declare_tool_lint, impl_lint_pass};
14-
use rustc_span::symbol::kw;
15-
use rustc_span::{sym, BytePos, Span};
13+
use rustc_span::symbol::{kw, Symbol};
14+
use rustc_span::{sym, BytePos, Span, DUMMY_SP};
15+
use smallvec::SmallVec;
1616

1717
declare_clippy_lint! {
1818
/// **What it does:** This lint warns when you use `println!("")` to
@@ -353,7 +353,117 @@ fn newline_span(fmtstr: &StrLit) -> Span {
353353
sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
354354
}
355355

356+
/// Stores a list of replacement spans for each argument, but only if all the replacements used an
357+
/// empty format string.
358+
#[derive(Default)]
359+
struct SimpleFormatArgs {
360+
unnamed: Vec<SmallVec<[Span; 1]>>,
361+
named: Vec<(Symbol, SmallVec<[Span; 1]>)>,
362+
}
363+
impl SimpleFormatArgs {
364+
fn get_unnamed(&self) -> impl Iterator<Item = &[Span]> {
365+
self.unnamed.iter().map(|x| match x.as_slice() {
366+
// Ignore the dummy span added from out of order format arguments.
367+
[DUMMY_SP] => &[],
368+
x => x,
369+
})
370+
}
371+
372+
fn get_named(&self, n: &Path) -> &[Span] {
373+
self.named.iter().find(|x| *n == x.0).map_or(&[], |x| x.1.as_slice())
374+
}
375+
376+
fn push(&mut self, arg: rustc_parse_format::Argument<'_>, span: Span) {
377+
use rustc_parse_format::{
378+
AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec,
379+
};
380+
381+
const SIMPLE: FormatSpec<'_> = FormatSpec {
382+
fill: None,
383+
align: AlignUnknown,
384+
flags: 0,
385+
precision: CountImplied,
386+
precision_span: None,
387+
width: CountImplied,
388+
width_span: None,
389+
ty: "",
390+
ty_span: None,
391+
};
392+
393+
match arg.position {
394+
ArgumentIs(n) | ArgumentImplicitlyIs(n) => {
395+
if self.unnamed.len() <= n {
396+
// Use a dummy span to mark all unseen arguments.
397+
self.unnamed.resize_with(n, || SmallVec::from([DUMMY_SP]));
398+
if arg.format == SIMPLE {
399+
self.unnamed.push(SmallVec::from([span]));
400+
} else {
401+
self.unnamed.push(SmallVec::new());
402+
}
403+
} else {
404+
let args = &mut self.unnamed[n];
405+
match (args.as_mut_slice(), arg.format == SIMPLE) {
406+
// A non-empty format string has been seen already.
407+
([], _) => (),
408+
// Replace the dummy span, if it exists.
409+
([dummy @ DUMMY_SP], true) => *dummy = span,
410+
([_, ..], true) => args.push(span),
411+
([_, ..], false) => *args = SmallVec::new(),
412+
}
413+
}
414+
},
415+
ArgumentNamed(n) => {
416+
if let Some(x) = self.named.iter_mut().find(|x| x.0 == n) {
417+
match x.1.as_slice() {
418+
// A non-empty format string has been seen already.
419+
[] => (),
420+
[_, ..] if arg.format == SIMPLE => x.1.push(span),
421+
[_, ..] => x.1 = SmallVec::new(),
422+
}
423+
} else if arg.format == SIMPLE {
424+
self.named.push((n, SmallVec::from([span])));
425+
} else {
426+
self.named.push((n, SmallVec::new()));
427+
}
428+
},
429+
};
430+
}
431+
}
432+
356433
impl Write {
434+
/// Parses a format string into a collection of spans for each argument. This only keeps track
435+
/// of empty format arguments. Will also lint usages of debug format strings outside of debug
436+
/// impls.
437+
fn parse_fmt_string(&self, cx: &EarlyContext<'_>, str: &StrLit) -> Option<SimpleFormatArgs> {
438+
use rustc_parse_format::{ParseMode, Parser, Piece};
439+
440+
let str_sym = str.symbol.as_str();
441+
let style = match str.style {
442+
StrStyle::Cooked => None,
443+
StrStyle::Raw(n) => Some(n as usize),
444+
};
445+
446+
let mut parser = Parser::new(&str_sym, style, snippet_opt(cx, str.span), false, ParseMode::Format);
447+
let mut args = SimpleFormatArgs::default();
448+
449+
while let Some(arg) = parser.next() {
450+
let arg = match arg {
451+
Piece::String(_) => continue,
452+
Piece::NextArgument(arg) => arg,
453+
};
454+
let span = parser.arg_places.last().map_or(DUMMY_SP, |&x| str.span.from_inner(x));
455+
456+
if !self.in_debug_impl && arg.format.ty == "?" {
457+
// FIXME: modify rustc's fmt string parser to give us the current span
458+
span_lint(cx, USE_DEBUG, str.span, "use of `Debug`-based formatting");
459+
}
460+
461+
args.push(arg, span);
462+
}
463+
464+
parser.errors.is_empty().then(move || args)
465+
}
466+
357467
/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
358468
/// `Option`s. The first `Option` of the tuple is the macro's format string. It includes
359469
/// the contents of the string, whether it's a raw string, and the span of the literal in the
@@ -375,57 +485,31 @@ impl Write {
375485
/// ```
376486
#[allow(clippy::too_many_lines)]
377487
fn check_tts<'a>(&self, cx: &EarlyContext<'a>, tts: TokenStream, is_write: bool) -> (Option<StrLit>, Option<Expr>) {
378-
use rustc_parse_format::{
379-
AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, ParseMode, Parser,
380-
Piece,
381-
};
382-
383488
let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, false, None);
384-
let mut expr: Option<Expr> = None;
385-
if is_write {
386-
expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
387-
Ok(p) => Some(p.into_inner()),
388-
Err(_) => return (None, None),
389-
};
390-
// might be `writeln!(foo)`
391-
if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
392-
return (None, expr);
489+
let expr = if is_write {
490+
match parser.parse_expr().map(|e| e.into_inner()).map_err(|mut e| e.cancel()) {
491+
// write!(e, ...)
492+
Ok(p) if parser.eat(&token::Comma) => Some(p),
493+
// write!(e) or error
494+
e => return (None, e.ok()),
393495
}
394-
}
496+
} else {
497+
None
498+
};
395499

396500
let fmtstr = match parser.parse_str_lit() {
397501
Ok(fmtstr) => fmtstr,
398502
Err(_) => return (None, expr),
399503
};
400-
let tmp = fmtstr.symbol.as_str();
401-
let mut args = vec![];
402-
let mut fmt_parser = Parser::new(&tmp, None, None, false, ParseMode::Format);
403-
while let Some(piece) = fmt_parser.next() {
404-
if !fmt_parser.errors.is_empty() {
405-
return (None, expr);
406-
}
407-
if let Piece::NextArgument(arg) = piece {
408-
if !self.in_debug_impl && arg.format.ty == "?" {
409-
// FIXME: modify rustc's fmt string parser to give us the current span
410-
span_lint(cx, USE_DEBUG, parser.prev_token.span, "use of `Debug`-based formatting");
411-
}
412-
args.push(arg);
413-
}
414-
}
504+
505+
let args = match self.parse_fmt_string(cx, &fmtstr) {
506+
Some(args) => args,
507+
None => return (Some(fmtstr), expr),
508+
};
509+
415510
let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL };
416-
let mut idx = 0;
511+
let mut unnamed_args = args.get_unnamed();
417512
loop {
418-
const SIMPLE: FormatSpec<'_> = FormatSpec {
419-
fill: None,
420-
align: AlignUnknown,
421-
flags: 0,
422-
precision: CountImplied,
423-
precision_span: None,
424-
width: CountImplied,
425-
width_span: None,
426-
ty: "",
427-
ty_span: None,
428-
};
429513
if !parser.eat(&token::Comma) {
430514
return (Some(fmtstr), expr);
431515
}
@@ -434,52 +518,26 @@ impl Write {
434518
} else {
435519
return (Some(fmtstr), None);
436520
};
437-
match &token_expr.kind {
521+
let (fmt_spans, span) = match &token_expr.kind {
438522
ExprKind::Lit(lit) if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..)) => {
439-
let mut all_simple = true;
440-
let mut seen = false;
441-
for arg in &args {
442-
match arg.position {
443-
ArgumentImplicitlyIs(n) | ArgumentIs(n) => {
444-
if n == idx {
445-
all_simple &= arg.format == SIMPLE;
446-
seen = true;
447-
}
448-
},
449-
ArgumentNamed(_) => {},
450-
}
451-
}
452-
if all_simple && seen {
453-
span_lint(cx, lint, token_expr.span, "literal with an empty format string");
454-
}
455-
idx += 1;
523+
(unnamed_args.next().unwrap_or(&[]), token_expr.span)
456524
},
457-
ExprKind::Assign(lhs, rhs, _) => {
458-
if_chain! {
459-
if let ExprKind::Lit(ref lit) = rhs.kind;
460-
if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..));
461-
if let ExprKind::Path(_, p) = &lhs.kind;
462-
then {
463-
let mut all_simple = true;
464-
let mut seen = false;
465-
for arg in &args {
466-
match arg.position {
467-
ArgumentImplicitlyIs(_) | ArgumentIs(_) => {},
468-
ArgumentNamed(name) => {
469-
if *p == name {
470-
seen = true;
471-
all_simple &= arg.format == SIMPLE;
472-
}
473-
},
474-
}
475-
}
476-
if all_simple && seen {
477-
span_lint(cx, lint, rhs.span, "literal with an empty format string");
478-
}
479-
}
480-
}
525+
ExprKind::Assign(lhs, rhs, _) => match (&lhs.kind, &rhs.kind) {
526+
(ExprKind::Path(_, p), ExprKind::Lit(lit))
527+
if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..)) =>
528+
{
529+
(args.get_named(p), rhs.span)
530+
},
531+
_ => continue,
481532
},
482-
_ => idx += 1,
533+
_ => {
534+
unnamed_args.next();
535+
continue;
536+
},
537+
};
538+
539+
if !fmt_spans.is_empty() {
540+
span_lint(cx, lint, span, "literal with an empty format string");
483541
}
484542
}
485543
}

0 commit comments

Comments
 (0)