From 4450c21f5125c363397d87ff74ed1a1d52a9f1a8 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 1 Mar 2021 13:14:52 -0500 Subject: [PATCH 1/4] Keep track of spans in format strings --- clippy_lints/src/write.rs | 238 ++++++++++++++++++++++++-------------- 1 file changed, 148 insertions(+), 90 deletions(-) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index fd3e5a7ce91..b5470c3bc70 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -2,9 +2,8 @@ use std::borrow::Cow; use std::ops::Range; use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::source::snippet_with_applicability; -use if_chain::if_chain; -use rustc_ast::ast::{Expr, ExprKind, ImplKind, Item, ItemKind, LitKind, MacCall, StrLit, StrStyle}; +use clippy_utils::source::{snippet_opt, snippet_with_applicability}; +use rustc_ast::ast::{Expr, ExprKind, ImplKind, Item, ItemKind, LitKind, MacCall, Path, StrLit, StrStyle}; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_errors::Applicability; @@ -12,8 +11,9 @@ use rustc_lexer::unescape::{self, EscapeError}; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_parse::parser; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::symbol::kw; -use rustc_span::{sym, BytePos, Span}; +use rustc_span::symbol::{kw, Symbol}; +use rustc_span::{sym, BytePos, Span, DUMMY_SP}; +use smallvec::SmallVec; declare_clippy_lint! { /// **What it does:** This lint warns when you use `println!("")` to @@ -354,7 +354,117 @@ fn newline_span(fmtstr: &StrLit) -> Span { sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi) } +/// Stores a list of replacement spans for each argument, but only if all the replacements used an +/// empty format string. +#[derive(Default)] +struct SimpleFormatArgs { + unnamed: Vec>, + named: Vec<(Symbol, SmallVec<[Span; 1]>)>, +} +impl SimpleFormatArgs { + fn get_unnamed(&self) -> impl Iterator { + self.unnamed.iter().map(|x| match x.as_slice() { + // Ignore the dummy span added from out of order format arguments. + [DUMMY_SP] => &[], + x => x, + }) + } + + fn get_named(&self, n: &Path) -> &[Span] { + self.named.iter().find(|x| *n == x.0).map_or(&[], |x| x.1.as_slice()) + } + + fn push(&mut self, arg: rustc_parse_format::Argument<'_>, span: Span) { + use rustc_parse_format::{ + AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, + }; + + const SIMPLE: FormatSpec<'_> = FormatSpec { + fill: None, + align: AlignUnknown, + flags: 0, + precision: CountImplied, + precision_span: None, + width: CountImplied, + width_span: None, + ty: "", + ty_span: None, + }; + + match arg.position { + ArgumentIs(n) | ArgumentImplicitlyIs(n) => { + if self.unnamed.len() <= n { + // Use a dummy span to mark all unseen arguments. + self.unnamed.resize_with(n, || SmallVec::from([DUMMY_SP])); + if arg.format == SIMPLE { + self.unnamed.push(SmallVec::from([span])); + } else { + self.unnamed.push(SmallVec::new()); + } + } else { + let args = &mut self.unnamed[n]; + match (args.as_mut_slice(), arg.format == SIMPLE) { + // A non-empty format string has been seen already. + ([], _) => (), + // Replace the dummy span, if it exists. + ([dummy @ DUMMY_SP], true) => *dummy = span, + ([_, ..], true) => args.push(span), + ([_, ..], false) => *args = SmallVec::new(), + } + } + }, + ArgumentNamed(n) => { + if let Some(x) = self.named.iter_mut().find(|x| x.0 == n) { + match x.1.as_slice() { + // A non-empty format string has been seen already. + [] => (), + [_, ..] if arg.format == SIMPLE => x.1.push(span), + [_, ..] => x.1 = SmallVec::new(), + } + } else if arg.format == SIMPLE { + self.named.push((n, SmallVec::from([span]))); + } else { + self.named.push((n, SmallVec::new())); + } + }, + }; + } +} + impl Write { + /// Parses a format string into a collection of spans for each argument. This only keeps track + /// of empty format arguments. Will also lint usages of debug format strings outside of debug + /// impls. + fn parse_fmt_string(&self, cx: &EarlyContext<'_>, str: &StrLit) -> Option { + use rustc_parse_format::{ParseMode, Parser, Piece}; + + let str_sym = str.symbol.as_str(); + let style = match str.style { + StrStyle::Cooked => None, + StrStyle::Raw(n) => Some(n as usize), + }; + + let mut parser = Parser::new(&str_sym, style, snippet_opt(cx, str.span), false, ParseMode::Format); + let mut args = SimpleFormatArgs::default(); + + while let Some(arg) = parser.next() { + let arg = match arg { + Piece::String(_) => continue, + Piece::NextArgument(arg) => arg, + }; + let span = parser.arg_places.last().map_or(DUMMY_SP, |&x| str.span.from_inner(x)); + + if !self.in_debug_impl && arg.format.ty == "?" { + // FIXME: modify rustc's fmt string parser to give us the current span + span_lint(cx, USE_DEBUG, str.span, "use of `Debug`-based formatting"); + } + + args.push(arg, span); + } + + parser.errors.is_empty().then(move || args) + } + /// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two /// `Option`s. The first `Option` of the tuple is the macro's format string. It includes /// the contents of the string, whether it's a raw string, and the span of the literal in the @@ -376,57 +486,31 @@ impl Write { /// ``` #[allow(clippy::too_many_lines)] fn check_tts<'a>(&self, cx: &EarlyContext<'a>, tts: TokenStream, is_write: bool) -> (Option, Option) { - use rustc_parse_format::{ - AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, ParseMode, Parser, - Piece, - }; - let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, false, None); - let mut expr: Option = None; - if is_write { - expr = match parser.parse_expr().map_err(|mut err| err.cancel()) { - Ok(p) => Some(p.into_inner()), - Err(_) => return (None, None), - }; - // might be `writeln!(foo)` - if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() { - return (None, expr); + let expr = if is_write { + match parser.parse_expr().map(|e| e.into_inner()).map_err(|mut e| e.cancel()) { + // write!(e, ...) + Ok(p) if parser.eat(&token::Comma) => Some(p), + // write!(e) or error + e => return (None, e.ok()), } - } + } else { + None + }; let fmtstr = match parser.parse_str_lit() { Ok(fmtstr) => fmtstr, Err(_) => return (None, expr), }; - let tmp = fmtstr.symbol.as_str(); - let mut args = vec![]; - let mut fmt_parser = Parser::new(&tmp, None, None, false, ParseMode::Format); - while let Some(piece) = fmt_parser.next() { - if !fmt_parser.errors.is_empty() { - return (None, expr); - } - if let Piece::NextArgument(arg) = piece { - if !self.in_debug_impl && arg.format.ty == "?" { - // FIXME: modify rustc's fmt string parser to give us the current span - span_lint(cx, USE_DEBUG, parser.prev_token.span, "use of `Debug`-based formatting"); - } - args.push(arg); - } - } + + let args = match self.parse_fmt_string(cx, &fmtstr) { + Some(args) => args, + None => return (Some(fmtstr), expr), + }; + let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL }; - let mut idx = 0; + let mut unnamed_args = args.get_unnamed(); loop { - const SIMPLE: FormatSpec<'_> = FormatSpec { - fill: None, - align: AlignUnknown, - flags: 0, - precision: CountImplied, - precision_span: None, - width: CountImplied, - width_span: None, - ty: "", - ty_span: None, - }; if !parser.eat(&token::Comma) { return (Some(fmtstr), expr); } @@ -435,52 +519,26 @@ impl Write { } else { return (Some(fmtstr), None); }; - match &token_expr.kind { + let (fmt_spans, span) = match &token_expr.kind { ExprKind::Lit(lit) if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..)) => { - let mut all_simple = true; - let mut seen = false; - for arg in &args { - match arg.position { - ArgumentImplicitlyIs(n) | ArgumentIs(n) => { - if n == idx { - all_simple &= arg.format == SIMPLE; - seen = true; - } - }, - ArgumentNamed(_) => {}, - } - } - if all_simple && seen { - span_lint(cx, lint, token_expr.span, "literal with an empty format string"); - } - idx += 1; + (unnamed_args.next().unwrap_or(&[]), token_expr.span) }, - ExprKind::Assign(lhs, rhs, _) => { - if_chain! { - if let ExprKind::Lit(ref lit) = rhs.kind; - if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..)); - if let ExprKind::Path(_, p) = &lhs.kind; - then { - let mut all_simple = true; - let mut seen = false; - for arg in &args { - match arg.position { - ArgumentImplicitlyIs(_) | ArgumentIs(_) => {}, - ArgumentNamed(name) => { - if *p == name { - seen = true; - all_simple &= arg.format == SIMPLE; - } - }, - } - } - if all_simple && seen { - span_lint(cx, lint, rhs.span, "literal with an empty format string"); - } - } - } + ExprKind::Assign(lhs, rhs, _) => match (&lhs.kind, &rhs.kind) { + (ExprKind::Path(_, p), ExprKind::Lit(lit)) + if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..)) => + { + (args.get_named(p), rhs.span) + }, + _ => continue, }, - _ => idx += 1, + _ => { + unnamed_args.next(); + continue; + }, + }; + + if !fmt_spans.is_empty() { + span_lint(cx, lint, span, "literal with an empty format string"); } } } From 4c1047167d20460dcb84e3d947787ce91d5fd0d4 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 1 Mar 2021 13:28:36 -0500 Subject: [PATCH 2/4] More specific spans for `use_debug` lint --- clippy_lints/src/write.rs | 2 +- tests/ui/print.stderr | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index b5470c3bc70..210ddc7465f 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -456,7 +456,7 @@ impl Write { if !self.in_debug_impl && arg.format.ty == "?" { // FIXME: modify rustc's fmt string parser to give us the current span - span_lint(cx, USE_DEBUG, str.span, "use of `Debug`-based formatting"); + span_lint(cx, USE_DEBUG, span, "use of `Debug`-based formatting"); } args.push(arg, span); diff --git a/tests/ui/print.stderr b/tests/ui/print.stderr index 208d9532628..1754c418381 100644 --- a/tests/ui/print.stderr +++ b/tests/ui/print.stderr @@ -1,8 +1,8 @@ error: use of `Debug`-based formatting - --> $DIR/print.rs:11:19 + --> $DIR/print.rs:11:20 | LL | write!(f, "{:?}", 43.1415) - | ^^^^^^ + | ^^^^ | = note: `-D clippy::use-debug` implied by `-D warnings` @@ -33,10 +33,10 @@ LL | print!("Hello {:?}", "World"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of `Debug`-based formatting - --> $DIR/print.rs:28:12 + --> $DIR/print.rs:28:19 | LL | print!("Hello {:?}", "World"); - | ^^^^^^^^^^^^ + | ^^^^ error: use of `print!` --> $DIR/print.rs:30:5 @@ -45,10 +45,10 @@ LL | print!("Hello {:#?}", "#orld"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of `Debug`-based formatting - --> $DIR/print.rs:30:12 + --> $DIR/print.rs:30:19 | LL | print!("Hello {:#?}", "#orld"); - | ^^^^^^^^^^^^^ + | ^^^^^ error: aborting due to 8 previous errors From a7fa2a6fa88477ab2542bb15347e3085d9547f95 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 1 Mar 2021 16:31:04 -0500 Subject: [PATCH 3/4] Add suggestion to `write_literal` and `print_literal` Don't lint on a mixture of raw and regular strings Fix spans in format strings --- clippy_lints/src/write.rs | 62 ++++++++++++++----- tests/ui/print_literal.stderr | 70 ++++++++++++++++++--- tests/ui/write_literal.stderr | 70 ++++++++++++++++++--- tests/ui/write_literal_2.rs | 27 ++++++++ tests/ui/write_literal_2.stderr | 106 ++++++++++++++++++++++++++++++++ 5 files changed, 305 insertions(+), 30 deletions(-) create mode 100644 tests/ui/write_literal_2.rs create mode 100644 tests/ui/write_literal_2.stderr diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 210ddc7465f..e416eab7914 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -1,10 +1,11 @@ use std::borrow::Cow; -use std::ops::Range; +use std::iter; +use std::ops::{Deref, Range}; use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet_opt, snippet_with_applicability}; -use rustc_ast::ast::{Expr, ExprKind, ImplKind, Item, ItemKind, LitKind, MacCall, Path, StrLit, StrStyle}; -use rustc_ast::token; +use rustc_ast::ast::{Expr, ExprKind, ImplKind, Item, ItemKind, MacCall, Path, StrLit, StrStyle}; +use rustc_ast::token::{self, LitKind}; use rustc_ast::tokenstream::TokenStream; use rustc_errors::Applicability; use rustc_lexer::unescape::{self, EscapeError}; @@ -438,7 +439,7 @@ impl Write { fn parse_fmt_string(&self, cx: &EarlyContext<'_>, str: &StrLit) -> Option { use rustc_parse_format::{ParseMode, Parser, Piece}; - let str_sym = str.symbol.as_str(); + let str_sym = str.symbol_unescaped.as_str(); let style = match str.style { StrStyle::Cooked => None, StrStyle::Raw(n) => Some(n as usize), @@ -514,21 +515,17 @@ impl Write { if !parser.eat(&token::Comma) { return (Some(fmtstr), expr); } + + let comma_span = parser.prev_token.span; let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) { expr } else { return (Some(fmtstr), None); }; - let (fmt_spans, span) = match &token_expr.kind { - ExprKind::Lit(lit) if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..)) => { - (unnamed_args.next().unwrap_or(&[]), token_expr.span) - }, + let (fmt_spans, lit) = match &token_expr.kind { + ExprKind::Lit(lit) => (unnamed_args.next().unwrap_or(&[]), lit), ExprKind::Assign(lhs, rhs, _) => match (&lhs.kind, &rhs.kind) { - (ExprKind::Path(_, p), ExprKind::Lit(lit)) - if !matches!(lit.kind, LitKind::Int(..) | LitKind::Float(..)) => - { - (args.get_named(p), rhs.span) - }, + (ExprKind::Path(_, p), ExprKind::Lit(lit)) => (args.get_named(p), lit), _ => continue, }, _ => { @@ -537,8 +534,45 @@ impl Write { }, }; + let replacement: String = match lit.token.kind { + LitKind::Integer | LitKind::Float | LitKind::Err => continue, + LitKind::StrRaw(_) | LitKind::ByteStrRaw(_) if matches!(fmtstr.style, StrStyle::Raw(_)) => { + lit.token.symbol.as_str().replace("{", "{{").replace("}", "}}") + }, + LitKind::Str | LitKind::ByteStr if matches!(fmtstr.style, StrStyle::Cooked) => { + lit.token.symbol.as_str().replace("{", "{{").replace("}", "}}") + }, + LitKind::StrRaw(_) | LitKind::Str | LitKind::ByteStrRaw(_) | LitKind::ByteStr => continue, + LitKind::Byte | LitKind::Char => match lit.token.symbol.as_str().deref() { + "\"" if matches!(fmtstr.style, StrStyle::Cooked) => "\\\"", + "\"" if matches!(fmtstr.style, StrStyle::Raw(0)) => continue, + "\\\\" if matches!(fmtstr.style, StrStyle::Raw(_)) => "\\", + "\\'" => "'", + "{" => "{{", + "}" => "}}", + x if matches!(fmtstr.style, StrStyle::Raw(_)) && x.starts_with("\\") => continue, + x => x, + } + .into(), + LitKind::Bool => lit.token.symbol.as_str().deref().into(), + }; + if !fmt_spans.is_empty() { - span_lint(cx, lint, span, "literal with an empty format string"); + span_lint_and_then( + cx, + lint, + token_expr.span, + "literal with an empty format string", + |diag| { + diag.multipart_suggestion( + "try this", + iter::once((comma_span.to(token_expr.span), String::new())) + .chain(fmt_spans.iter().cloned().zip(iter::repeat(replacement))) + .collect(), + Applicability::MachineApplicable, + ); + }, + ); } } } diff --git a/tests/ui/print_literal.stderr b/tests/ui/print_literal.stderr index e284aece236..54a4084c89e 100644 --- a/tests/ui/print_literal.stderr +++ b/tests/ui/print_literal.stderr @@ -5,66 +5,120 @@ LL | print!("Hello {}", "world"); | ^^^^^^^ | = note: `-D clippy::print-literal` implied by `-D warnings` +help: try this + | +LL | print!("Hello world"); + | ^^^^^-- error: literal with an empty format string --> $DIR/print_literal.rs:26:36 | LL | println!("Hello {} {}", world, "world"); | ^^^^^^^ + | +help: try this + | +LL | println!("Hello {} world", world); + | ^^^^^ -- error: literal with an empty format string --> $DIR/print_literal.rs:27:26 | LL | println!("Hello {}", "world"); | ^^^^^^^ + | +help: try this + | +LL | println!("Hello world"); + | ^^^^^-- error: literal with an empty format string --> $DIR/print_literal.rs:32:25 | LL | println!("{0} {1}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | println!("hello {1}", "world"); + | ^^^^^ -- error: literal with an empty format string --> $DIR/print_literal.rs:32:34 | LL | println!("{0} {1}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | println!("{0} world", "hello"); + | ^^^^^ -- error: literal with an empty format string --> $DIR/print_literal.rs:33:25 | LL | println!("{1} {0}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | println!("{1} hello", "world"); + | ^^^^^-- error: literal with an empty format string --> $DIR/print_literal.rs:33:34 | LL | println!("{1} {0}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | println!("world {0}", "hello"); + | ^^^^^ -- error: literal with an empty format string - --> $DIR/print_literal.rs:36:35 + --> $DIR/print_literal.rs:36:29 | LL | println!("{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | println!("hello {bar}", bar = "world"); + | ^^^^^ -- error: literal with an empty format string - --> $DIR/print_literal.rs:36:50 + --> $DIR/print_literal.rs:36:44 | LL | println!("{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | println!("{foo} world", foo = "hello"); + | ^^^^^ -- error: literal with an empty format string - --> $DIR/print_literal.rs:37:35 + --> $DIR/print_literal.rs:37:29 | LL | println!("{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | println!("{bar} hello", bar = "world"); + | ^^^^^-- error: literal with an empty format string - --> $DIR/print_literal.rs:37:50 + --> $DIR/print_literal.rs:37:44 | LL | println!("{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | println!("world {foo}", foo = "hello"); + | ^^^^^ -- error: aborting due to 11 previous errors diff --git a/tests/ui/write_literal.stderr b/tests/ui/write_literal.stderr index e54d89ecf29..507a78e8280 100644 --- a/tests/ui/write_literal.stderr +++ b/tests/ui/write_literal.stderr @@ -5,66 +5,120 @@ LL | write!(&mut v, "Hello {}", "world"); | ^^^^^^^ | = note: `-D clippy::write-literal` implied by `-D warnings` +help: try this + | +LL | write!(&mut v, "Hello world"); + | ^^^^^-- error: literal with an empty format string --> $DIR/write_literal.rs:31:44 | LL | writeln!(&mut v, "Hello {} {}", world, "world"); | ^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "Hello {} world", world); + | ^^^^^ -- error: literal with an empty format string --> $DIR/write_literal.rs:32:34 | LL | writeln!(&mut v, "Hello {}", "world"); | ^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "Hello world"); + | ^^^^^-- error: literal with an empty format string --> $DIR/write_literal.rs:37:33 | LL | writeln!(&mut v, "{0} {1}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "hello {1}", "world"); + | ^^^^^ -- error: literal with an empty format string --> $DIR/write_literal.rs:37:42 | LL | writeln!(&mut v, "{0} {1}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "{0} world", "hello"); + | ^^^^^ -- error: literal with an empty format string --> $DIR/write_literal.rs:38:33 | LL | writeln!(&mut v, "{1} {0}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "{1} hello", "world"); + | ^^^^^-- error: literal with an empty format string --> $DIR/write_literal.rs:38:42 | LL | writeln!(&mut v, "{1} {0}", "hello", "world"); | ^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "world {0}", "hello"); + | ^^^^^ -- error: literal with an empty format string - --> $DIR/write_literal.rs:41:43 + --> $DIR/write_literal.rs:41:37 | LL | writeln!(&mut v, "{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "hello {bar}", bar = "world"); + | ^^^^^ -- error: literal with an empty format string - --> $DIR/write_literal.rs:41:58 + --> $DIR/write_literal.rs:41:52 | LL | writeln!(&mut v, "{foo} {bar}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "{foo} world", foo = "hello"); + | ^^^^^ -- error: literal with an empty format string - --> $DIR/write_literal.rs:42:43 + --> $DIR/write_literal.rs:42:37 | LL | writeln!(&mut v, "{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "{bar} hello", bar = "world"); + | ^^^^^-- error: literal with an empty format string - --> $DIR/write_literal.rs:42:58 + --> $DIR/write_literal.rs:42:52 | LL | writeln!(&mut v, "{bar} {foo}", foo = "hello", bar = "world"); - | ^^^^^^^ + | ^^^^^^^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, "world {foo}", foo = "hello"); + | ^^^^^ -- error: aborting due to 11 previous errors diff --git a/tests/ui/write_literal_2.rs b/tests/ui/write_literal_2.rs new file mode 100644 index 00000000000..f341e8215e1 --- /dev/null +++ b/tests/ui/write_literal_2.rs @@ -0,0 +1,27 @@ +#![allow(unused_must_use)] +#![warn(clippy::write_literal)] + +use std::io::Write; + +fn main() { + let mut v = Vec::new(); + + writeln!(&mut v, "{}", "{hello}"); + writeln!(&mut v, r"{}", r"{hello}"); + writeln!(&mut v, "{}", '\''); + writeln!(&mut v, "{}", '"'); + writeln!(&mut v, r"{}", '"'); // don't lint + writeln!(&mut v, r"{}", '\''); + writeln!( + &mut v, + "some {}", + "hello \ + world!" + ); + writeln!( + &mut v, + "some {}\ + {} \\ {}", + "1", "2", "3", + ); +} diff --git a/tests/ui/write_literal_2.stderr b/tests/ui/write_literal_2.stderr new file mode 100644 index 00000000000..5b488358011 --- /dev/null +++ b/tests/ui/write_literal_2.stderr @@ -0,0 +1,106 @@ +error: literal with an empty format string + --> $DIR/write_literal_2.rs:9:28 + | +LL | writeln!(&mut v, "{}", "{hello}"); + | ^^^^^^^^^ + | + = note: `-D clippy::write-literal` implied by `-D warnings` +help: try this + | +LL | writeln!(&mut v, "{{hello}}"); + | ^^^^^^^^^-- + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:10:29 + | +LL | writeln!(&mut v, r"{}", r"{hello}"); + | ^^^^^^^^^^ + | +help: try this + | +LL | writeln!(&mut v, r"{{hello}}"); + | ^^^^^^^^^-- + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:11:28 + | +LL | writeln!(&mut v, "{}", '/''); + | ^^^^ + | +help: try this + | +LL | writeln!(&mut v, "'"); + | ^-- + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:12:28 + | +LL | writeln!(&mut v, "{}", '"'); + | ^^^ + | +help: try this + | +LL | writeln!(&mut v, "/""); + | ^^-- + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:14:29 + | +LL | writeln!(&mut v, r"{}", '/''); + | ^^^^ + | +help: try this + | +LL | writeln!(&mut v, r"'"); + | ^-- + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:18:9 + | +LL | / "hello / +LL | | world!" + | |_______________^ + | +help: try this + | +LL | "some hello / +LL | world!" + | + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:25:9 + | +LL | "1", "2", "3", + | ^^^ + | +help: try this + | +LL | "some 1{} / {}", "2", "3", + | ^ -- + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:25:14 + | +LL | "1", "2", "3", + | ^^^ + | +help: try this + | +LL | 2 / {}", +LL | "1", "3", + | + +error: literal with an empty format string + --> $DIR/write_literal_2.rs:25:19 + | +LL | "1", "2", "3", + | ^^^ + | +help: try this + | +LL | {} / 3", +LL | "1", "2", + | + +error: aborting due to 9 previous errors + From d45873b4c151898876da73746300ac29d5448e62 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Tue, 16 Mar 2021 12:06:41 -0400 Subject: [PATCH 4/4] Remove SmallVec --- clippy_lints/src/write.rs | 42 ++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index e416eab7914..12a47a6b703 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -14,7 +14,6 @@ use rustc_parse::parser; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::{kw, Symbol}; use rustc_span::{sym, BytePos, Span, DUMMY_SP}; -use smallvec::SmallVec; declare_clippy_lint! { /// **What it does:** This lint warns when you use `println!("")` to @@ -359,8 +358,8 @@ fn newline_span(fmtstr: &StrLit) -> Span { /// empty format string. #[derive(Default)] struct SimpleFormatArgs { - unnamed: Vec>, - named: Vec<(Symbol, SmallVec<[Span; 1]>)>, + unnamed: Vec>, + named: Vec<(Symbol, Vec)>, } impl SimpleFormatArgs { fn get_unnamed(&self) -> impl Iterator { @@ -396,11 +395,11 @@ impl SimpleFormatArgs { ArgumentIs(n) | ArgumentImplicitlyIs(n) => { if self.unnamed.len() <= n { // Use a dummy span to mark all unseen arguments. - self.unnamed.resize_with(n, || SmallVec::from([DUMMY_SP])); + self.unnamed.resize_with(n, || vec![DUMMY_SP]); if arg.format == SIMPLE { - self.unnamed.push(SmallVec::from([span])); + self.unnamed.push(vec![span]); } else { - self.unnamed.push(SmallVec::new()); + self.unnamed.push(Vec::new()); } } else { let args = &mut self.unnamed[n]; @@ -410,7 +409,7 @@ impl SimpleFormatArgs { // Replace the dummy span, if it exists. ([dummy @ DUMMY_SP], true) => *dummy = span, ([_, ..], true) => args.push(span), - ([_, ..], false) => *args = SmallVec::new(), + ([_, ..], false) => *args = Vec::new(), } } }, @@ -420,12 +419,12 @@ impl SimpleFormatArgs { // A non-empty format string has been seen already. [] => (), [_, ..] if arg.format == SIMPLE => x.1.push(span), - [_, ..] => x.1 = SmallVec::new(), + [_, ..] => x.1 = Vec::new(), } } else if arg.format == SIMPLE { - self.named.push((n, SmallVec::from([span]))); + self.named.push((n, vec![span])); } else { - self.named.push((n, SmallVec::new())); + self.named.push((n, Vec::new())); } }, }; @@ -436,16 +435,16 @@ impl Write { /// Parses a format string into a collection of spans for each argument. This only keeps track /// of empty format arguments. Will also lint usages of debug format strings outside of debug /// impls. - fn parse_fmt_string(&self, cx: &EarlyContext<'_>, str: &StrLit) -> Option { + fn parse_fmt_string(&self, cx: &EarlyContext<'_>, str_lit: &StrLit) -> Option { use rustc_parse_format::{ParseMode, Parser, Piece}; - let str_sym = str.symbol_unescaped.as_str(); - let style = match str.style { + let str_sym = str_lit.symbol_unescaped.as_str(); + let style = match str_lit.style { StrStyle::Cooked => None, StrStyle::Raw(n) => Some(n as usize), }; - let mut parser = Parser::new(&str_sym, style, snippet_opt(cx, str.span), false, ParseMode::Format); + let mut parser = Parser::new(&str_sym, style, snippet_opt(cx, str_lit.span), false, ParseMode::Format); let mut args = SimpleFormatArgs::default(); while let Some(arg) = parser.next() { @@ -453,7 +452,10 @@ impl Write { Piece::String(_) => continue, Piece::NextArgument(arg) => arg, }; - let span = parser.arg_places.last().map_or(DUMMY_SP, |&x| str.span.from_inner(x)); + let span = parser + .arg_places + .last() + .map_or(DUMMY_SP, |&x| str_lit.span.from_inner(x)); if !self.in_debug_impl && arg.format.ty == "?" { // FIXME: modify rustc's fmt string parser to give us the current span @@ -489,7 +491,11 @@ impl Write { fn check_tts<'a>(&self, cx: &EarlyContext<'a>, tts: TokenStream, is_write: bool) -> (Option, Option) { let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, false, None); let expr = if is_write { - match parser.parse_expr().map(|e| e.into_inner()).map_err(|mut e| e.cancel()) { + match parser + .parse_expr() + .map(rustc_ast::ptr::P::into_inner) + .map_err(|mut e| e.cancel()) + { // write!(e, ...) Ok(p) if parser.eat(&token::Comma) => Some(p), // write!(e) or error @@ -543,14 +549,14 @@ impl Write { lit.token.symbol.as_str().replace("{", "{{").replace("}", "}}") }, LitKind::StrRaw(_) | LitKind::Str | LitKind::ByteStrRaw(_) | LitKind::ByteStr => continue, - LitKind::Byte | LitKind::Char => match lit.token.symbol.as_str().deref() { + LitKind::Byte | LitKind::Char => match &*lit.token.symbol.as_str() { "\"" if matches!(fmtstr.style, StrStyle::Cooked) => "\\\"", "\"" if matches!(fmtstr.style, StrStyle::Raw(0)) => continue, "\\\\" if matches!(fmtstr.style, StrStyle::Raw(_)) => "\\", "\\'" => "'", "{" => "{{", "}" => "}}", - x if matches!(fmtstr.style, StrStyle::Raw(_)) && x.starts_with("\\") => continue, + x if matches!(fmtstr.style, StrStyle::Raw(_)) && x.starts_with('\\') => continue, x => x, } .into(),