diff --git a/README.md b/README.md index 4d655c85951..87b58b320ff 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 117 lints included in this crate: +There are 118 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -118,6 +118,7 @@ name [unstable_as_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice) | warn | as_slice is not stable and can be replaced by & v[..]see https://github.com/rust-lang/rust/issues/27729 [unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop [unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes) | warn | unused lifetimes in function definitions +[use_debug](https://github.com/Manishearth/rust-clippy/wiki#use_debug) | allow | use `Debug`-based formatting [used_underscore_binding](https://github.com/Manishearth/rust-clippy/wiki#used_underscore_binding) | warn | using a binding which is prefixed with an underscore [useless_transmute](https://github.com/Manishearth/rust-clippy/wiki#useless_transmute) | warn | transmutes that have the same to and from types [useless_vec](https://github.com/Manishearth/rust-clippy/wiki#useless_vec) | warn | useless `vec!` diff --git a/src/lib.rs b/src/lib.rs index cd9ac226321..ba9236012d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -169,6 +169,7 @@ pub fn plugin_registrar(reg: &mut Registry) { mut_mut::MUT_MUT, mutex_atomic::MUTEX_INTEGER, print::PRINT_STDOUT, + print::USE_DEBUG, shadow::SHADOW_REUSE, shadow::SHADOW_SAME, shadow::SHADOW_UNRELATED, diff --git a/src/print.rs b/src/print.rs index d8f5fd488aa..3c10b4bed13 100644 --- a/src/print.rs +++ b/src/print.rs @@ -1,6 +1,8 @@ use rustc::lint::*; use rustc_front::hir::*; -use utils::{IO_PRINT_PATH, is_expn_of, match_path, span_lint}; +use rustc::front::map::Node::{NodeItem, NodeImplItem}; +use utils::{FMT_ARGUMENTV1_NEW_PATH, DEBUG_FMT_METHOD_PATH, IO_PRINT_PATH}; +use utils::{is_expn_of, match_path, span_lint}; /// **What it does:** This lint warns whenever you print on *stdout*. The purpose of this lint is to catch debugging remnants. /// @@ -16,21 +18,36 @@ declare_lint! { "printing on stdout" } +/// **What it does:** This lint warns whenever you use `Debug` formatting. The purpose of this lint is to catch debugging remnants. +/// +/// **Why is this bad?** The purpose of the `Debug` trait is to facilitate debugging Rust code. It +/// should not be used in in user-facing output. +/// +/// **Example:** `println!("{:?}", foo);` +declare_lint! { + pub USE_DEBUG, + Allow, + "use `Debug`-based formatting" +} + #[derive(Copy, Clone, Debug)] pub struct PrintLint; impl LintPass for PrintLint { fn get_lints(&self) -> LintArray { - lint_array!(PRINT_STDOUT) + lint_array!(PRINT_STDOUT, USE_DEBUG) } } impl LateLintPass for PrintLint { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if let ExprCall(ref fun, _) = expr.node { + if let ExprCall(ref fun, ref args) = expr.node { if let ExprPath(_, ref path) = fun.node { + // Search for `std::io::_print(..)` which is unique in a + // `print!` expansion. if match_path(path, &IO_PRINT_PATH) { if let Some(span) = is_expn_of(cx, expr.span, "print") { + // `println!` uses `print!`. let (span, name) = match is_expn_of(cx, span, "println") { Some(span) => (span, "println"), None => (span, "print"), @@ -39,7 +56,32 @@ impl LateLintPass for PrintLint { span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name)); } } + // Search for something like + // `::std::fmt::ArgumentV1::new(__arg0, ::std::fmt::Debug::fmt)` + else if args.len() == 2 && match_path(path, &FMT_ARGUMENTV1_NEW_PATH) { + if let ExprPath(None, ref path) = args[1].node { + if match_path(path, &DEBUG_FMT_METHOD_PATH) && + !is_in_debug_impl(cx, expr) && + is_expn_of(cx, expr.span, "panic").is_none() { + span_lint(cx, USE_DEBUG, args[0].span, "use of `Debug`-based formatting"); + } + } + } } } } } + +fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool { + let map = &cx.tcx.map; + + if let Some(NodeImplItem(item)) = map.find(map.get_parent(expr.id)) { // `fmt` method + if let Some(NodeItem(item)) = map.find(map.get_parent(item.id)) { // `Debug` impl + if let ItemImpl(_, _, _, Some(ref tr), _, _) = item.node { + return match_path(&tr.path, &["Debug"]); + } + } + } + + false +} diff --git a/src/utils.rs b/src/utils.rs index a8890f31cb0..4c89b7f113d 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -26,8 +26,10 @@ pub const BTREEMAP_PATH: [&'static str; 4] = ["collections", "btree", "map", "BT pub const CLONE_PATH: [&'static str; 3] = ["clone", "Clone", "clone"]; pub const CLONE_TRAIT_PATH: [&'static str; 2] = ["clone", "Clone"]; pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"]; +pub const DEBUG_FMT_METHOD_PATH: [&'static str; 4] = ["std", "fmt", "Debug", "fmt"]; pub const DEFAULT_TRAIT_PATH: [&'static str; 3] = ["core", "default", "Default"]; pub const DROP_PATH: [&'static str; 3] = ["core", "mem", "drop"]; +pub const FMT_ARGUMENTV1_NEW_PATH: [&'static str; 4] = ["std", "fmt", "ArgumentV1", "new"]; pub const HASHMAP_ENTRY_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHMAP_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"]; pub const HASH_PATH: [&'static str; 2] = ["hash", "Hash"]; diff --git a/tests/compile-fail/print.rs b/tests/compile-fail/print.rs index 8141cdd2645..34c38dca286 100755 --- a/tests/compile-fail/print.rs +++ b/tests/compile-fail/print.rs @@ -1,11 +1,41 @@ #![feature(plugin)] #![plugin(clippy)] +#![deny(print_stdout, use_debug)] -#[deny(print_stdout)] +use std::fmt::{Debug, Display, Formatter, Result}; + +#[allow(dead_code)] +struct Foo; + +impl Display for Foo { + fn fmt(&self, f: &mut Formatter) -> Result { + write!(f, "{:?}", 43.1415) + //~^ ERROR use of `Debug`-based formatting + } +} + +impl Debug for Foo { + fn fmt(&self, f: &mut Formatter) -> Result { + // ok, we can use `Debug` formatting in `Debug` implementations + write!(f, "{:?}", 42.718) + } +} fn main() { println!("Hello"); //~ERROR use of `println!` print!("Hello"); //~ERROR use of `print!` + print!("Hello {}", "World"); //~ERROR use of `print!` + + print!("Hello {:?}", "World"); + //~^ ERROR use of `print!` + //~| ERROR use of `Debug`-based formatting + + print!("Hello {:#?}", "#orld"); + //~^ ERROR use of `print!` + //~| ERROR use of `Debug`-based formatting + + assert_eq!(42, 1337); + vec![1, 2]; }