diff --git a/clippy_lints/src/print.rs b/clippy_lints/src/print.rs index 6d9880e13351..ddfe6d68f4a0 100644 --- a/clippy_lints/src/print.rs +++ b/clippy_lints/src/print.rs @@ -78,12 +78,31 @@ declare_clippy_lint! { "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) +/// +/// **Known problems:** Will also warn with macro calls as arguments that expand to literals +/// -- e.g., `println!("{}", env!("FOO"))`. +/// +/// **Example:** +/// ```rust +/// println!("{}", "foo"); +/// ``` +declare_clippy_lint! { + pub PRINT_LITERAL, + style, + "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 +126,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { 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 +169,49 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } } +// 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/format.rs b/tests/ui/format.rs index e9379d0a05bd..5e18b74bb2c4 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -1,5 +1,5 @@ - +#![allow(print_literal)] #![warn(useless_format)] fn main() { diff --git a/tests/ui/print.rs b/tests/ui/print.rs index 91304d961a7a..786398cfe5ed 100644 --- a/tests/ui/print.rs +++ b/tests/ui/print.rs @@ -1,5 +1,6 @@ +#![allow(print_literal)] #![warn(print_stdout, use_debug)] use std::fmt::{Debug, Display, Formatter, Result}; diff --git a/tests/ui/print.stderr b/tests/ui/print.stderr index 789e1218b780..457ed38a1b5f 100644 --- a/tests/ui/print.stderr +++ b/tests/ui/print.stderr @@ -1,53 +1,53 @@ error: use of `Debug`-based formatting - --> $DIR/print.rs:12:27 + --> $DIR/print.rs:13:27 | -12 | write!(f, "{:?}", 43.1415) +13 | write!(f, "{:?}", 43.1415) | ^^^^^^^ | = note: `-D use-debug` implied by `-D warnings` error: use of `println!` - --> $DIR/print.rs:24:5 + --> $DIR/print.rs:25:5 | -24 | println!("Hello"); +25 | println!("Hello"); | ^^^^^^^^^^^^^^^^^^ | = note: `-D print-stdout` implied by `-D warnings` error: use of `print!` - --> $DIR/print.rs:25:5 + --> $DIR/print.rs:26:5 | -25 | print!("Hello"); +26 | print!("Hello"); | ^^^^^^^^^^^^^^^^ error: use of `print!` - --> $DIR/print.rs:27:5 + --> $DIR/print.rs:28:5 | -27 | print!("Hello {}", "World"); +28 | print!("Hello {}", "World"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of `print!` - --> $DIR/print.rs:29:5 + --> $DIR/print.rs:30:5 | -29 | print!("Hello {:?}", "World"); +30 | print!("Hello {:?}", "World"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of `Debug`-based formatting - --> $DIR/print.rs:29:26 + --> $DIR/print.rs:30:26 | -29 | print!("Hello {:?}", "World"); +30 | print!("Hello {:?}", "World"); | ^^^^^^^ error: use of `print!` - --> $DIR/print.rs:31:5 + --> $DIR/print.rs:32:5 | -31 | print!("Hello {:#?}", "#orld"); +32 | print!("Hello {:#?}", "#orld"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of `Debug`-based formatting - --> $DIR/print.rs:31:27 + --> $DIR/print.rs:32:27 | -31 | print!("Hello {:#?}", "#orld"); +32 | print!("Hello {:#?}", "#orld"); | ^^^^^^^ error: aborting due to 8 previous errors diff --git a/tests/ui/print_literal.rs b/tests/ui/print_literal.rs new file mode 100644 index 000000000000..c803294ab0ad --- /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 000000000000..982be7dc5373 --- /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 + diff --git a/tests/ui/print_with_newline.rs b/tests/ui/print_with_newline.rs index 5cc50dea8104..5445c862096f 100644 --- a/tests/ui/print_with_newline.rs +++ b/tests/ui/print_with_newline.rs @@ -1,5 +1,6 @@ +#![allow(print_literal)] #![warn(print_with_newline)] fn main() { diff --git a/tests/ui/print_with_newline.stderr b/tests/ui/print_with_newline.stderr index 4f32d1b2a2d4..5f2013e728e8 100644 --- a/tests/ui/print_with_newline.stderr +++ b/tests/ui/print_with_newline.stderr @@ -1,7 +1,7 @@ error: using `print!()` with a format string that ends in a newline, consider using `println!()` instead - --> $DIR/print_with_newline.rs:6:5 + --> $DIR/print_with_newline.rs:7:5 | -6 | print!("Hello/n"); +7 | print!("Hello/n"); | ^^^^^^^^^^^^^^^^^^ | = note: `-D print-with-newline` implied by `-D warnings`