Check for literals as println! args

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<Expr>
+) {
+    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>,

tests/ui/format.rs

@@ -1,5 +1,5 @@
 
-
+#![allow(print_literal)]
 #![warn(useless_format)]
 
 fn main() {

tests/ui/print.rs

@@ -1,5 +1,6 @@
 
 
+#![allow(print_literal)]
 #![warn(print_stdout, use_debug)]
 
 use std::fmt::{Debug, Display, Formatter, Result};

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

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");
+}

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
+

tests/ui/print_with_newline.rs

@@ -1,5 +1,6 @@
 
 
+#![allow(print_literal)]
 #![warn(print_with_newline)]
 
 fn main() {

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`