From cae9cedeb5acc9b4edc23b376e15fba339254c31 Mon Sep 17 00:00:00 2001 From: mcarton Date: Fri, 29 Sep 2017 18:36:03 +0200 Subject: [PATCH 1/2] Fix regression with `format!` --- clippy_lints/src/format.rs | 27 +++++++++++++++------------ tests/ui/format.stderr | 14 +++++++++++++- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index f1a450e58..26b500d55 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -50,8 +50,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!`"); @@ -96,17 +97,19 @@ pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Exp None } -/// Checks if the expressions matches -/// ```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 `&[""]` +fn check_static_str(expr: &Expr) -> bool { + if_let_chain! {[ + let ExprAddrOf(_, ref expr) = expr.node, // &[""] + let ExprArray(ref exprs) = expr.node, // [""] + exprs.len() == 1, + let ExprLit(ref lit) = exprs[0].node, + let LitKind::Str(ref lit, _) = lit.node, + ], { + return lit.as_str().is_empty(); + }} + + false } /// Checks if the expressions matches diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index 5f5bdc02a..d2c9f3938 100644 --- a/tests/ui/format.stderr +++ b/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 From 7e956ac7c4a38c862e344e5e60243b3995bc5831 Mon Sep 17 00:00:00 2001 From: mcarton Date: Fri, 29 Sep 2017 19:13:21 +0200 Subject: [PATCH 2/2] Fix regression with `print!` --- clippy_lints/src/format.rs | 29 ----------------------------- clippy_lints/src/print.rs | 26 ++++++++++++++++++++------ tests/ui/print_with_newline.stderr | 28 ++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 35 deletions(-) diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 26b500d55..6a6cbadb6 100644 --- a/clippy_lints/src/format.rs +++ b/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}; @@ -70,33 +68,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } } -/// 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> { - if_let_chain! {[ - let ExprBlock(ref block) = expr.node, - block.stmts.len() == 1, - let StmtDecl(ref decl, _) = block.stmts[0].node, - let DeclItem(ref decl) = decl.node, - let Some(NodeItem(decl)) = cx.tcx.hir.find(decl.id), - 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(); - 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 `&[""]` fn check_static_str(expr: &Expr) -> bool { if_let_chain! {[ diff --git a/clippy_lints/src/print.rs b/clippy_lints/src/print.rs index 078e20846..96557b8b0 100644 --- a/clippy_lints/src/print.rs +++ b/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(last_str) = fmtstrs.last(), - let Some('\n') = last_str.chars().last(), + let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]), + let Some('\n') = fmtstr.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 +} diff --git a/tests/ui/print_with_newline.stderr b/tests/ui/print_with_newline.stderr index e69de29bb..1bacc40bf 100644 --- a/tests/ui/print_with_newline.stderr +++ b/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 +