Auto merge of #3450 - phansch:structured_sugg_for_explicit_write, r=flip1995

Add suggestion for explicit_write lint

Closes #2083

4 files changed, 109 insertions, 38 deletions

diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs
index 94bd0ab209c..a0db3ae8df3 100644
--- a/clippy_lints/src/explicit_write.rs
+++ b/clippy_lints/src/explicit_write.rs
@@ -10,8 +10,9 @@
 use crate::rustc::hir::*;
 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use crate::rustc::{declare_tool_lint, lint_array};
-use crate::utils::opt_def_id;
-use crate::utils::{is_expn_of, match_def_path, resolve_node, span_lint};
+use crate::rustc_errors::Applicability;
+use crate::syntax::ast::LitKind;
+use crate::utils::{is_expn_of, match_def_path, opt_def_id, resolve_node, span_lint, span_lint_and_sugg};
 use if_chain::if_chain;
 
 /// **What it does:** Checks for usage of `write!()` / `writeln()!` which can be
@@ -81,32 +82,85 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                 } 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")
-                        )
-                    );
+
+                // We need to remove the last trailing newline from the string because the
+                // underlying `fmt::write` function doesn't know whether `println!` or `print!` was
+                // used.
+                if let Some(mut write_output) = write_output_string(write_args) {
+                    if write_output.ends_with('\n') {
+                        write_output.pop();
+                    }
+
+                    if let Some(macro_name) = calling_macro {
+                        span_lint_and_sugg(
+                            cx,
+                            EXPLICIT_WRITE,
+                            expr.span,
+                            &format!(
+                                "use of `{}!({}(), ...).unwrap()`",
+                                macro_name,
+                                dest_name
+                            ),
+                            "try this",
+                            format!("{}{}!(\"{}\")", prefix, macro_name.replace("write", "print"), write_output.escape_default()),
+                            Applicability::MachineApplicable
+                        );
+                    } else {
+                        span_lint_and_sugg(
+                            cx,
+                            EXPLICIT_WRITE,
+                            expr.span,
+                            &format!("use of `{}().write_fmt(...).unwrap()`", dest_name),
+                            "try this",
+                            format!("{}print!(\"{}\")", prefix, write_output.escape_default()),
+                            Applicability::MachineApplicable
+                        );
+                    }
                 } else {
-                    span_lint(
-                        cx,
-                        EXPLICIT_WRITE,
-                        expr.span,
-                        &format!(
-                            "use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead",
-                            dest_name,
-                            prefix,
-                        )
-                    );
+                    // We don't have a proper suggestion
+                    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),
+                        );
+                    }
                 }
+
             }
         }
     }
 }
+
+// Extract the output string from the given `write_args`.
+fn write_output_string(write_args: &HirVec<Expr>) -> Option<String> {
+    if_chain! {
+        // Obtain the string that should be printed
+        if write_args.len() > 1;
+        if let ExprKind::Call(_, ref output_args) = write_args[1].node;
+        if output_args.len() > 0;
+        if let ExprKind::AddrOf(_, ref output_string_expr) = output_args[0].node;
+        if let ExprKind::Array(ref string_exprs) = output_string_expr.node;
+        if string_exprs.len() > 0;
+        if let ExprKind::Lit(ref lit) = string_exprs[0].node;
+        if let LitKind::Str(ref write_output, _) = lit.node;
+        then {
+            return Some(write_output.to_string())
+        }
+    }
+    None
+}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index ee41c632077..a862d774174 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -14,6 +14,7 @@
 #![feature(slice_patterns)]
 #![feature(stmt_expr_attributes)]
 #![feature(range_contains)]
+#![feature(str_escape)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![recursion_limit = "256"]
 #![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)]
diff --git a/tests/ui/explicit_write.rs b/tests/ui/explicit_write.rs
index 10a4bca9f49..01a63b3a95f 100644
--- a/tests/ui/explicit_write.rs
+++ b/tests/ui/explicit_write.rs
@@ -27,6 +27,10 @@ fn main() {
         writeln!(std::io::stderr(), "test").unwrap();
         std::io::stdout().write_fmt(format_args!("test")).unwrap();
         std::io::stderr().write_fmt(format_args!("test")).unwrap();
+
+        // including newlines
+        writeln!(std::io::stdout(), "test\ntest").unwrap();
+        writeln!(std::io::stderr(), "test\ntest").unwrap();
     }
     // these should not warn, different destination
     {
diff --git a/tests/ui/explicit_write.stderr b/tests/ui/explicit_write.stderr
index 171bf312a9b..1a11dbc169b 100644
--- a/tests/ui/explicit_write.stderr
+++ b/tests/ui/explicit_write.stderr
@@ -1,40 +1,52 @@
-error: use of `write!(stdout(), ...).unwrap()`. Consider using `print!` instead
+error: use of `write!(stdout(), ...).unwrap()`
   --> $DIR/explicit_write.rs:24:9
    |
 24 |         write!(std::io::stdout(), "test").unwrap();
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")`
    |
    = note: `-D clippy::explicit-write` implied by `-D warnings`
 
-error: use of `write!(stderr(), ...).unwrap()`. Consider using `eprint!` instead
+error: use of `write!(stderr(), ...).unwrap()`
   --> $DIR/explicit_write.rs:25:9
    |
 25 |         write!(std::io::stderr(), "test").unwrap();
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")`
 
-error: use of `writeln!(stdout(), ...).unwrap()`. Consider using `println!` instead
+error: use of `writeln!(stdout(), ...).unwrap()`
   --> $DIR/explicit_write.rs:26:9
    |
 26 |         writeln!(std::io::stdout(), "test").unwrap();
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test")`
 
-error: use of `writeln!(stderr(), ...).unwrap()`. Consider using `eprintln!` instead
+error: use of `writeln!(stderr(), ...).unwrap()`
   --> $DIR/explicit_write.rs:27:9
    |
 27 |         writeln!(std::io::stderr(), "test").unwrap();
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test")`
 
-error: use of `stdout().write_fmt(...).unwrap()`. Consider using `print!` instead
+error: use of `stdout().write_fmt(...).unwrap()`
   --> $DIR/explicit_write.rs:28:9
    |
 28 |         std::io::stdout().write_fmt(format_args!("test")).unwrap();
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")`
 
-error: use of `stderr().write_fmt(...).unwrap()`. Consider using `eprint!` instead
+error: use of `stderr().write_fmt(...).unwrap()`
   --> $DIR/explicit_write.rs:29:9
    |
 29 |         std::io::stderr().write_fmt(format_args!("test")).unwrap();
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")`
 
-error: aborting due to 6 previous errors
+error: use of `writeln!(stdout(), ...).unwrap()`
+  --> $DIR/explicit_write.rs:32:9
+   |
+32 |         writeln!(std::io::stdout(), "test/ntest").unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test/ntest")`
+
+error: use of `writeln!(stderr(), ...).unwrap()`
+  --> $DIR/explicit_write.rs:33:9
+   |
+33 |         writeln!(std::io::stderr(), "test/ntest").unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test/ntest")`
+
+error: aborting due to 8 previous errors