diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs new file mode 100644 index 000000000000..9650dd0909ca --- /dev/null +++ b/clippy_lints/src/explicit_write.rs @@ -0,0 +1,101 @@ +use rustc::hir::*; +use rustc::lint::*; +use utils::{is_expn_of, match_def_path, resolve_node, span_lint}; +use utils::opt_def_id; + +/// **What it does:** Checks for usage of `write!()` / `writeln()!` which can be +/// replaced with `(e)print!()` / `(e)println!()` +/// +/// **Why is this bad?** Using `(e)println! is clearer and more concise +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// // this would be clearer as `eprintln!("foo: {:?}", bar);` +/// writeln!(&mut io::stderr(), "foo: {:?}", bar).unwrap(); +/// ``` +declare_lint! { + pub EXPLICIT_WRITE, + Warn, + "using the `write!()` family of functions instead of the `print!()` family \ + of functions, when using the latter would work" +} + +#[derive(Copy, Clone, Debug)] +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(EXPLICIT_WRITE) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + if_let_chain! {[ + // match call to unwrap + let ExprMethodCall(ref unwrap_fun, _, ref unwrap_args) = expr.node, + unwrap_fun.name == "unwrap", + // match call to write_fmt + unwrap_args.len() > 0, + let ExprMethodCall(ref write_fun, _, ref write_args) = + unwrap_args[0].node, + write_fun.name == "write_fmt", + // match calls to std::io::stdout() / std::io::stderr () + write_args.len() > 0, + let ExprCall(ref dest_fun, _) = write_args[0].node, + let ExprPath(ref qpath) = dest_fun.node, + let Some(dest_fun_id) = + opt_def_id(resolve_node(cx, qpath, dest_fun.hir_id)), + let Some(dest_name) = if match_def_path(cx.tcx, dest_fun_id, &["std", "io", "stdio", "stdout"]) { + Some("stdout") + } else if match_def_path(cx.tcx, dest_fun_id, &["std", "io", "stdio", "stderr"]) { + Some("stderr") + } else { + None + }, + ], { + let write_span = unwrap_args[0].span; + let calling_macro = + // ordering is important here, since `writeln!` uses `write!` internally + if is_expn_of(write_span, "writeln").is_some() { + Some("writeln") + } else if is_expn_of(write_span, "write").is_some() { + Some("write") + } else { + None + }; + let prefix = if dest_name == "stderr" { + "e" + } else { + "" + }; + if let Some(macro_name) = calling_macro { + span_lint( + cx, + EXPLICIT_WRITE, + expr.span, + &format!( + "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead", + macro_name, + dest_name, + prefix, + macro_name.replace("write", "print") + ) + ); + } else { + span_lint( + cx, + EXPLICIT_WRITE, + expr.span, + &format!( + "use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", + dest_name, + prefix, + ) + ); + } + }} + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 412d83d8ec89..355b03b2b6a7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -146,6 +146,7 @@ pub mod serde_api; pub mod shadow; pub mod should_assert_eq; pub mod strings; +pub mod explicit_write; pub mod swap; pub mod temporary_assignment; pub mod transmute; @@ -327,6 +328,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box unused_io_amount::UnusedIoAmount); reg.register_late_lint_pass(box large_enum_variant::LargeEnumVariant::new(conf.enum_variant_size_threshold)); reg.register_late_lint_pass(box should_assert_eq::ShouldAssertEq); + reg.register_late_lint_pass(box explicit_write::Pass); reg.register_late_lint_pass(box needless_pass_by_value::NeedlessPassByValue); reg.register_early_lint_pass(box literal_digit_grouping::LiteralDigitGrouping); reg.register_late_lint_pass(box use_self::UseSelf); @@ -542,6 +544,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { serde_api::SERDE_API_MISUSE, should_assert_eq::SHOULD_ASSERT_EQ, strings::STRING_LIT_AS_BYTES, + explicit_write::EXPLICIT_WRITE, swap::ALMOST_SWAPPED, swap::MANUAL_SWAP, temporary_assignment::TEMPORARY_ASSIGNMENT, diff --git a/tests/ui/explicit_write.rs b/tests/ui/explicit_write.rs new file mode 100644 index 000000000000..71992123ceb6 --- /dev/null +++ b/tests/ui/explicit_write.rs @@ -0,0 +1,46 @@ +#![warn(explicit_write)] + + +fn stdout() -> String { + String::new() +} + +fn stderr() -> String { + String::new() +} + +fn main() { + // these should warn + { + use std::io::Write; + write!(std::io::stdout(), "test").unwrap(); + write!(std::io::stderr(), "test").unwrap(); + writeln!(std::io::stdout(), "test").unwrap(); + writeln!(std::io::stderr(), "test").unwrap(); + std::io::stdout().write_fmt(format_args!("test")).unwrap(); + std::io::stderr().write_fmt(format_args!("test")).unwrap(); + } + // these should not warn, different destination + { + use std::fmt::Write; + let mut s = String::new(); + write!(s, "test").unwrap(); + write!(s, "test").unwrap(); + writeln!(s, "test").unwrap(); + writeln!(s, "test").unwrap(); + s.write_fmt(format_args!("test")).unwrap(); + s.write_fmt(format_args!("test")).unwrap(); + write!(stdout(), "test").unwrap(); + write!(stderr(), "test").unwrap(); + writeln!(stdout(), "test").unwrap(); + writeln!(stderr(), "test").unwrap(); + stdout().write_fmt(format_args!("test")).unwrap(); + stderr().write_fmt(format_args!("test")).unwrap(); + } + // these should not warn, no unwrap + { + use std::io::Write; + std::io::stdout().write_fmt(format_args!("test")).expect("no stdout"); + std::io::stderr().write_fmt(format_args!("test")).expect("no stderr"); + } +} diff --git a/tests/ui/explicit_write.stderr b/tests/ui/explicit_write.stderr new file mode 100644 index 000000000000..9a813e897934 --- /dev/null +++ b/tests/ui/explicit_write.stderr @@ -0,0 +1,38 @@ +error: use of `write!(stdout(), ...).unwrap()`. Consider using `print!` instead + --> $DIR/explicit_write.rs:16:9 + | +16 | write!(std::io::stdout(), "test").unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D explicit-write` implied by `-D warnings` + +error: use of `write!(stderr(), ...).unwrap()`. Consider using `eprint!` instead + --> $DIR/explicit_write.rs:17:9 + | +17 | write!(std::io::stderr(), "test").unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: use of `writeln!(stdout(), ...).unwrap()`. Consider using `println!` instead + --> $DIR/explicit_write.rs:18:9 + | +18 | writeln!(std::io::stdout(), "test").unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: use of `writeln!(stderr(), ...).unwrap()`. Consider using `eprintln!` instead + --> $DIR/explicit_write.rs:19:9 + | +19 | writeln!(std::io::stderr(), "test").unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: use of `stdout().write_fmt(...).unwrap()`. Consider using `print!` instead + --> $DIR/explicit_write.rs:20:9 + | +20 | std::io::stdout().write_fmt(format_args!("test")).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: use of `stderr().write_fmt(...).unwrap()`. Consider using `eprint!` instead + --> $DIR/explicit_write.rs:21:9 + | +21 | std::io::stderr().write_fmt(format_args!("test")).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +