Merge pull request #2131 from devonhollowood/suggest-print

Suggest print

Author: oli-obk

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,
+                    )
+                );
+            }
+        }}
+    }
+}

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,

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");
+    }
+}

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();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+