Lint print!("...\n") (closes #455)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 725cdcc10b8a2504aa4e8970fe880dec24ce5563..7edbf5748929bbedd3733326ea01d0245a6ae005 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -252,6 +252,7 @@ All notable changes to this project will be documented in this file.
 [`panic_params`]: https://github.com/Manishearth/rust-clippy/wiki#panic_params
 [`precedence`]: https://github.com/Manishearth/rust-clippy/wiki#precedence
 [`print_stdout`]: https://github.com/Manishearth/rust-clippy/wiki#print_stdout
+[`print_with_newline`]: https://github.com/Manishearth/rust-clippy/wiki#print_with_newline
 [`ptr_arg`]: https://github.com/Manishearth/rust-clippy/wiki#ptr_arg
 [`range_step_by_zero`]: https://github.com/Manishearth/rust-clippy/wiki#range_step_by_zero
 [`range_zip_with_len`]: https://github.com/Manishearth/rust-clippy/wiki#range_zip_with_len

diff --git a/README.md b/README.md
index 1da19a71044ba79e8e8810fcb046ff55ec6ef04c..93336e0f260e13ab1a9fc9ee0d31edc7dbc270c8 100644 (file)
--- a/README.md
+++ b/README.md
@@ -17,7 +17,7 @@ Table of contents:
 
 ## Lints
 
-There are 164 lints included in this crate:
+There are 165 lints included in this crate:
 
 name                                                                                                                 | default | triggers on
 ---------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -130,6 +130,7 @@ name
 [panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params)                                         | warn    | missing parameters in `panic!` calls
 [precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence)                                             | warn    | operations where precedence may be unclear
 [print_stdout](https://github.com/Manishearth/rust-clippy/wiki#print_stdout)                                         | allow   | printing on stdout
+[print_with_newline](https://github.com/Manishearth/rust-clippy/wiki#print_with_newline)                             | warn    | using `print!()` with a format string that ends in a newline
 [ptr_arg](https://github.com/Manishearth/rust-clippy/wiki#ptr_arg)                                                   | warn    | arguments of the type `&Vec<...>` (instead of `&[...]`) or `&String` (instead of `&str`)
 [range_step_by_zero](https://github.com/Manishearth/rust-clippy/wiki#range_step_by_zero)                             | warn    | using `Range::step_by(0)`, which produces an infinite iterator
 [range_zip_with_len](https://github.com/Manishearth/rust-clippy/wiki#range_zip_with_len)                             | warn    | zipping iterator with a range when `enumerate()` would do

diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs
index 39a93eae94a0c21d71b161b68e10cc813a2e03b6..349ce2cf8b819d0fde29bd7b85dea8c3e194b2be 100644 (file)
--- a/clippy_lints/src/format.rs
+++ b/clippy_lints/src/format.rs
@@ -69,11 +69,10 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
     }
 }
 
-/// Checks if the expressions matches
-/// ```rust
-/// { static __STATIC_FMTSTR: &[""] = _; __STATIC_FMTSTR }
-/// ```
-fn check_static_str(cx: &LateContext, expr: &Expr) -> bool {
+/// Returns the slice of format string parts in an `Arguments::new_v1` call.
+/// Public because it's shared with a lint in print.rs.
+pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Expr)
+                                         -> Option<Vec<&'a str>> {
     if_let_chain! {[
         let ExprBlock(ref block) = expr.node,
         block.stmts.len() == 1,
@@ -83,16 +82,31 @@ fn check_static_str(cx: &LateContext, expr: &Expr) -> bool {
         decl.name.as_str() == "__STATIC_FMTSTR",
         let ItemStatic(_, _, ref expr) = decl.node,
         let ExprAddrOf(_, ref expr) = expr.node, // &[""]
-        let ExprVec(ref expr) = expr.node,
-        expr.len() == 1,
-        let ExprLit(ref lit) = expr[0].node,
-        let LitKind::Str(ref lit, _) = lit.node,
-        lit.is_empty()
+        let ExprVec(ref exprs) = expr.node,
     ], {
-        return true;
+        let mut result = Vec::new();
+        for expr in exprs {
+            if let ExprLit(ref lit) = expr.node {
+                if let LitKind::Str(ref lit, _) = lit.node {
+                    result.push(&**lit);
+                }
+            }
+        }
+        return Some(result);
     }}
+    None
+}
 
-    false
+/// Checks if the expressions matches
+/// ```rust
+/// { static __STATIC_FMTSTR: &[""] = _; __STATIC_FMTSTR }
+/// ```
+fn check_static_str(cx: &LateContext, expr: &Expr) -> bool {
+    if let Some(expr) = get_argument_fmtstr_parts(cx, expr) {
+        expr.len() == 1 && expr[0].is_empty()
+    } else {
+        false
+    }
 }
 
 /// Checks if the expressions matches

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 3a5051b9308f81df71d0f47cdae1ca03d2f7c06f..fdd262a75e13e805b3e9f672a0fbc25a5352db8a 100644 (file)
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -403,6 +403,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
         panic::PANIC_PARAMS,
         precedence::PRECEDENCE,
+        print::PRINT_WITH_NEWLINE,
         ptr_arg::PTR_ARG,
         ranges::RANGE_STEP_BY_ZERO,
         ranges::RANGE_ZIP_WITH_LEN,

diff --git a/clippy_lints/src/print.rs b/clippy_lints/src/print.rs
index 39f1f5b39a211a159be1b84aaeee1515292ec870..25539ddc75291318a4a94ce58671317f7a1303cf 100644 (file)
--- a/clippy_lints/src/print.rs
+++ b/clippy_lints/src/print.rs
@@ -3,6 +3,24 @@
 use rustc::lint::*;
 use utils::paths;
 use utils::{is_expn_of, match_path, span_lint};
+use format::get_argument_fmtstr_parts;
+
+/// **What it does:** This lint warns when you using `print!()` with a format string that
+/// ends in a newline.
+///
+/// **Why is this bad?** You should use `println!()` instead, which appends the newline.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// print!("Hello {}!\n", name);
+/// ```
+declare_lint! {
+    pub PRINT_WITH_NEWLINE,
+    Warn,
+    "using `print!()` with a format string that ends in a newline"
+}
 
 /// **What it does:** Checks for printing on *stdout*. The purpose of this lint
 /// is to catch debugging remnants.
@@ -43,7 +61,7 @@
 
 impl LintPass for Pass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(PRINT_STDOUT, USE_DEBUG)
+        lint_array!(PRINT_WITH_NEWLINE, PRINT_STDOUT, USE_DEBUG)
     }
 }
 
@@ -62,6 +80,26 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
                         };
 
                         span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name));
+
+                        // Check print! with format string ending in "\n".
+                        if_let_chain!{[
+                            name == "print",
+                            // ensure we're calling Arguments::new_v1
+                            args.len() == 1,
+                            let ExprCall(ref args_fun, ref args_args) = args[0].node,
+                            let ExprPath(_, ref args_path) = args_fun.node,
+                            match_path(args_path, &paths::FMT_ARGUMENTS_NEWV1),
+                            args_args.len() == 2,
+                            // collect the format string parts and check the last one
+                            let Some(fmtstrs) = get_argument_fmtstr_parts(cx, &args_args[0]),
+                            let Some(last_str) = fmtstrs.last(),
+                            let Some(last_chr) = last_str.chars().last(),
+                            last_chr == '\n'
+                        ], {
+                            span_lint(cx, PRINT_WITH_NEWLINE, span,
+                                      "using `print!()` with a format string that ends in a \
+                                       newline, consider using `println!()` instead");
+                        }}
                     }
                 }
                 // Search for something like

diff --git a/tests/compile-fail/print_with_newline.rs b/tests/compile-fail/print_with_newline.rs
new file mode 100755 (executable)
index 0000000..fc3f276
--- /dev/null
+++ b/tests/compile-fail/print_with_newline.rs
@@ -0,0 +1,15 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+#![deny(print_with_newline)]
+
+fn main() {
+    print!("Hello");
+    print!("Hello\n"); //~ERROR using `print!()` with a format string
+    print!("Hello {}\n", "world"); //~ERROR using `print!()` with a format string
+    print!("Hello {} {}\n\n", "world", "#2"); //~ERROR using `print!()` with a format string
+
+    // these are all fine
+    println!("Hello");
+    println!("Hello\n");
+    println!("Hello {}\n", "world");
+}