diff --git a/clippy_lints/src/print.rs b/clippy_lints/src/print.rs index 6d9880e1335..a52d76e81da 100644 --- a/clippy_lints/src/print.rs +++ b/clippy_lints/src/print.rs @@ -78,12 +78,28 @@ "use of `Debug`-based formatting" } +/// **What it does:** This lint warns about the use of literals as `print!`/`println!` args. +/// +/// **Why is this bad?** Using literals as `println!` args is inefficient +/// (c.f., https://github.com/matthiaskrgr/rust-str-bench) and unnecessary +/// (i.e., just put the literal in the format string) +/// +/// **Example:** +/// ```rust +/// println!("{}", "foo"); +/// ``` +declare_lint! { + pub PRINT_LITERAL, + Allow, + "printing a literal with a format string" +} + #[derive(Copy, Clone, Debug)] pub struct Pass; impl LintPass for Pass { fn get_lints(&self) -> LintArray { - lint_array!(PRINT_WITH_NEWLINE, PRINTLN_EMPTY_STRING, PRINT_STDOUT, USE_DEBUG) + lint_array!(PRINT_WITH_NEWLINE, PRINTLN_EMPTY_STRING, PRINT_STDOUT, USE_DEBUG, PRINT_LITERAL) } } @@ -107,6 +123,10 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name)); + // Check for literals in the print!/println! args + // Also, ensure the format string is `{}` with no special options, like `{:X}` + check_print_args_for_literal(cx, args); + if_chain! { // ensure we're calling Arguments::new_v1 if args.len() == 1; @@ -146,6 +166,49 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { } } +// Check for literals in print!/println! args +// ensuring the format string for the literal is `DISPLAY_FMT_METHOD` +// e.g., `println!("... {} ...", "foo")` +// ^ literal in `println!` +fn check_print_args_for_literal<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + args: &HirVec +) { + if_chain! { + if args.len() == 1; + if let ExprCall(_, ref args_args) = args[0].node; + if args_args.len() > 1; + if let ExprAddrOf(_, ref match_expr) = args_args[1].node; + if let ExprMatch(ref matchee, ref arms, _) = match_expr.node; + if let ExprTup(ref tup) = matchee.node; + if arms.len() == 1; + if let ExprArray(ref arm_body_exprs) = arms[0].body.node; + then { + // it doesn't matter how many args there are in the `print!`/`println!`, + // if there's one literal, we should warn the user + for (idx, tup_arg) in tup.iter().enumerate() { + if_chain! { + // first, make sure we're dealing with a literal (i.e., an ExprLit) + if let ExprAddrOf(_, ref tup_val) = tup_arg.node; + if let ExprLit(_) = tup_val.node; + + // next, check the corresponding match arm body to ensure + // this is "{}", or DISPLAY_FMT_METHOD + if let ExprCall(_, ref body_args) = arm_body_exprs[idx].node; + if body_args.len() == 2; + if let ExprPath(ref body_qpath) = body_args[1].node; + if let Some(fun_def_id) = opt_def_id(resolve_node(cx, body_qpath, body_args[1].hir_id)); + if match_def_path(cx.tcx, fun_def_id, &paths::DISPLAY_FMT_METHOD) || + match_def_path(cx.tcx, fun_def_id, &paths::DEBUG_FMT_METHOD); + then { + span_lint(cx, PRINT_LITERAL, tup_val.span, "printing a literal with an empty format string"); + } + } + } + } + } +} + // Check for print!("... \n", ...). fn check_print<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, diff --git a/tests/ui/print_literal.rs b/tests/ui/print_literal.rs new file mode 100644 index 00000000000..c803294ab0a --- /dev/null +++ b/tests/ui/print_literal.rs @@ -0,0 +1,32 @@ + + +#![warn(print_literal)] + +fn main() { + // these should be fine + print!("Hello"); + println!("Hello"); + let world = "world"; + println!("Hello {}", world); + println!("3 in hex is {:X}", 3); + + // these should throw warnings + print!("Hello {}", "world"); + println!("Hello {} {}", world, "world"); + println!("Hello {}", "world"); + println!("10 / 4 is {}", 2.5); + println!("2 + 1 = {}", 3); + println!("2 + 1 = {:.4}", 3); + println!("2 + 1 = {:5.4}", 3); + println!("Debug test {:?}", "hello, world"); + + // positional args don't change the fact + // that we're using a literal -- this should + // throw a warning + println!("{0} {1}", "hello", "world"); + println!("{1} {0}", "hello", "world"); + + // named args shouldn't change anything either + println!("{foo} {bar}", foo="hello", bar="world"); + println!("{bar} {foo}", foo="hello", bar="world"); +} diff --git a/tests/ui/print_literal.stderr b/tests/ui/print_literal.stderr new file mode 100644 index 00000000000..982be7dc537 --- /dev/null +++ b/tests/ui/print_literal.stderr @@ -0,0 +1,100 @@ +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:14:24 + | +14 | print!("Hello {}", "world"); + | ^^^^^^^ + | + = note: `-D print-literal` implied by `-D warnings` + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:15:36 + | +15 | println!("Hello {} {}", world, "world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:16:26 + | +16 | println!("Hello {}", "world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:17:30 + | +17 | println!("10 / 4 is {}", 2.5); + | ^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:18:28 + | +18 | println!("2 + 1 = {}", 3); + | ^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:19:31 + | +19 | println!("2 + 1 = {:.4}", 3); + | ^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:20:32 + | +20 | println!("2 + 1 = {:5.4}", 3); + | ^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:21:33 + | +21 | println!("Debug test {:?}", "hello, world"); + | ^^^^^^^^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:26:25 + | +26 | println!("{0} {1}", "hello", "world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:26:34 + | +26 | println!("{0} {1}", "hello", "world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:27:25 + | +27 | println!("{1} {0}", "hello", "world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:27:34 + | +27 | println!("{1} {0}", "hello", "world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:30:33 + | +30 | println!("{foo} {bar}", foo="hello", bar="world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:30:46 + | +30 | println!("{foo} {bar}", foo="hello", bar="world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:31:33 + | +31 | println!("{bar} {foo}", foo="hello", bar="world"); + | ^^^^^^^ + +error: printing a literal with an empty format string + --> $DIR/print_literal.rs:31:46 + | +31 | println!("{bar} {foo}", foo="hello", bar="world"); + | ^^^^^^^ + +error: aborting due to 16 previous errors +