From ffad9a8c8c41964ca3f95619a9759ba650ad3703 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 5 Aug 2016 17:52:58 +0200 Subject: [PATCH] Lint print!("...\n") (closes #455) --- CHANGELOG.md | 1 + README.md | 3 +- clippy_lints/src/format.rs | 38 +++++++++++++++------- clippy_lints/src/lib.rs | 1 + clippy_lints/src/print.rs | 40 +++++++++++++++++++++++- tests/compile-fail/print_with_newline.rs | 15 +++++++++ 6 files changed, 84 insertions(+), 14 deletions(-) create mode 100755 tests/compile-fail/print_with_newline.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 725cdcc10..7edbf5748 100644 --- 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 1da19a710..93336e0f2 100644 --- 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 39a93eae9..349ce2cf8 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -69,11 +69,10 @@ impl LateLintPass for Pass { } } -/// 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> { 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 3a5051b93..fdd262a75 100644 --- 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 39f1f5b39..25539ddc7 100644 --- a/clippy_lints/src/print.rs +++ b/clippy_lints/src/print.rs @@ -3,6 +3,24 @@ use rustc::hir::map::Node::{NodeItem, NodeImplItem}; 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 @@ pub struct Pass; 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 @@ impl LateLintPass for Pass { }; 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 index 000000000..fc3f276fe --- /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"); +}