Merge pull request #2097 from rust-lang-nursery/fix-2041

Fix detection of format and print macros

clippy_lints/src/format.rs

@@ -1,9 +1,7 @@
 use rustc::hir::*;
-use rustc::hir::map::Node::NodeItem;
 use rustc::lint::*;
 use rustc::ty;
 use syntax::ast::LitKind;
-use syntax::symbol::InternedString;
 use utils::paths;
 use utils::{is_expn_of, match_def_path, match_type, resolve_node, span_lint, walk_ptrs_ty, opt_def_id};
 
@@ -50,8 +48,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                         let Some(fun_def_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id)),
                         match_def_path(cx.tcx, fun_def_id, &paths::FMT_ARGUMENTS_NEWV1),
                         // ensure the format string is `"{..}"` with only one argument and no text
-                        check_static_str(cx, &args[0]),
+                        check_static_str(&args[0]),
                         // ensure the format argument is `{}` ie. Display with no fancy option
+                        // and that the argument is a string
                         check_arg_is_display(cx, &args[1])
                     ], {
                         span_lint(cx, USELESS_FORMAT, span, "useless use of `format!`");
@@ -69,44 +68,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
     }
 }
 
-/// Returns the slice of format string parts in an `Arguments::new_v1` call.
+/// Checks if the expressions matches `&[""]`
-/// Public because it's shared with a lint in print.rs.
+fn check_static_str(expr: &Expr) -> bool {
-pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Expr) -> Option<Vec<InternedString>> {
     if_let_chain! {[
-        let ExprBlock(ref block) = expr.node,
+        let ExprAddrOf(_, ref expr) = expr.node, // &[""]
-        block.stmts.len() == 1,
+        let ExprArray(ref exprs) = expr.node, // [""]
-        let StmtDecl(ref decl, _) = block.stmts[0].node,
+        exprs.len() == 1,
-        let DeclItem(ref decl) = decl.node,
+        let ExprLit(ref lit) = exprs[0].node,
-        let Some(NodeItem(decl)) = cx.tcx.hir.find(decl.id),
+        let LitKind::Str(ref lit, _) = lit.node,
-        decl.name == "__STATIC_FMTSTR",
-        let ItemStatic(_, _, ref expr) = decl.node,
-        let ExprAddrOf(_, ref expr) = cx.tcx.hir.body(*expr).value.node, // &["…", "…", …]
-        let ExprArray(ref exprs) = expr.node,
     ], {
-        let mut result = Vec::new();
+        return lit.as_str().is_empty();
-        for expr in exprs {
-            if let ExprLit(ref lit) = expr.node {
-                if let LitKind::Str(ref lit, _) = lit.node {
-                    result.push(lit.as_str());
-                }
-            }
-        }
-        return Some(result);
     }}
-    None
-}
 
-/// Checks if the expressions matches
+    false
-/// ```rust, ignore
-/// { static __STATIC_FMTSTR: &'static[&'static str] = &["a", "b", c];
-/// __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

clippy_lints/src/print.rs

@@ -1,9 +1,10 @@
 use rustc::hir::*;
 use rustc::hir::map::Node::{NodeImplItem, NodeItem};
 use rustc::lint::*;
-use utils::{paths, opt_def_id};
+use syntax::ast::LitKind;
+use syntax::symbol::InternedString;
 use utils::{is_expn_of, match_def_path, match_path, resolve_node, span_lint};
-use format::get_argument_fmtstr_parts;
+use utils::{paths, opt_def_id};
 
 /// **What it does:** This lint warns when you using `print!()` with a format
 /// string that
@@ -103,15 +104,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                         let ExprTup(ref args) = args.node,
 
                         // collect the format string parts and check the last one
-                        let Some(fmtstrs) = get_argument_fmtstr_parts(cx, &args_args[0]),
+                        let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]),
-                        let Some(last_str) = fmtstrs.last(),
+                        let Some('\n') = fmtstr.chars().last(),
-                        let Some('\n') = last_str.chars().last(),
 
                         // "foo{}bar" is made into two strings + one argument,
                         // if the format string starts with `{}` (eg. "{}foo"),
                         // the string array is prepended an empty string "".
                         // We only want to check the last string after any `{}`:
-                        args.len() < fmtstrs.len(),
+                        args.len() < fmtlen,
                     ], {
                         span_lint(cx, PRINT_WITH_NEWLINE, span,
                                   "using `print!()` with a format string that ends in a \
@@ -150,3 +150,17 @@ fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool {
 
     false
 }
+
+/// Returns the slice of format string parts in an `Arguments::new_v1` call.
+fn get_argument_fmtstr_parts(expr: &Expr) -> Option<(InternedString, usize)> {
+    if_let_chain! {[
+        let ExprAddrOf(_, ref expr) = expr.node, // &["…", "…", …]
+        let ExprArray(ref exprs) = expr.node,
+        let Some(expr) = exprs.last(),
+        let ExprLit(ref lit) = expr.node,
+        let LitKind::Str(ref lit, _) = lit.node,
+    ], {
+        return Some((lit.as_str(), exprs.len()));
+    }}
+    None
+}

tests/ui/format.stderr

@@ -6,5 +6,17 @@ error: useless use of `format!`
   |
   = note: `-D useless-format` implied by `-D warnings`
 
-error: aborting due to previous error
+error: useless use of `format!`
+ --> $DIR/format.rs:8:5
+  |
+8 |     format!("{}", "foo");
+  |     ^^^^^^^^^^^^^^^^^^^^^
+
+error: useless use of `format!`
+  --> $DIR/format.rs:15:5
+   |
+15 |     format!("{}", arg);
+   |     ^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
 

tests/ui/print_with_newline.stderr

@@ -0,0 +1,28 @@
+error: using `print!()` with a format string that ends in a newline, consider using `println!()` instead
+ --> $DIR/print_with_newline.rs:6:5
+  |
+6 |     print!("Hello/n");
+  |     ^^^^^^^^^^^^^^^^^^
+  |
+  = note: `-D print-with-newline` implied by `-D warnings`
+
+error: using `print!()` with a format string that ends in a newline, consider using `println!()` instead
+ --> $DIR/print_with_newline.rs:7:5
+  |
+7 |     print!("Hello {}/n", "world");
+  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: using `print!()` with a format string that ends in a newline, consider using `println!()` instead
+ --> $DIR/print_with_newline.rs:8:5
+  |
+8 |     print!("Hello {} {}/n/n", "world", "#2");
+  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: using `print!()` with a format string that ends in a newline, consider using `println!()` instead
+ --> $DIR/print_with_newline.rs:9:5
+  |
+9 |     print!("{}/n", 1265);
+  |     ^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+