Skip to content

Commit

Permalink
Add suggestion to write_literal and print_literal
Browse files Browse the repository at this point in the history 
Don't lint on a mixture of raw and regular strings
Fix spans in format strings
  • Loading branch information
@Jarcho
Jarcho committed Mar 1, 2021
1 parent 85f645a commit 7537128
Show file tree
Hide file tree
Showing 4 changed files with 296 additions and 31 deletions.
62 changes: 48 additions & 14 deletions clippy_lints/src/write.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use std::borrow::Cow;
use std::ops::Range;
use std::iter;
use std::ops::{Deref, Range};

use crate::utils::{snippet_opt, snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then};
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};
Expand Down Expand Up @@ -437,7 +438,7 @@ impl Write {
fn parse_fmt_string(&self, cx: &EarlyContext<'_>, str: &StrLit) -> Option<SimpleFormatArgs> {
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),
Expand Down Expand Up @@ -513,21 +514,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,
},
_ => {
Expand All @@ -536,8 +533,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,
);
},
);
}
}
}
Expand Down
70 changes: 62 additions & 8 deletions tests/ui/print_literal.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

20 changes: 20 additions & 0 deletions tests/ui/write_literal.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,24 @@ fn main() {
// named args shouldn't change anything either
writeln!(&mut v, "{foo} {bar}", foo = "hello", bar = "world");
writeln!(&mut v, "{bar} {foo}", foo = "hello", bar = "world");

// Replacement test
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",
);
}
Loading

0 comments on commit 7537128

Please sign in to comment.