add suggestions for print/write with newline lint

euclio · euclio · commit 01af71ef3af7 · 2019-05-24T14:55:13.000-04:00
diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs
@@ -1,12 +1,12 @@
-use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg};
+use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then};
 use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
 use rustc::{declare_lint_pass, declare_tool_lint};
 use rustc_errors::Applicability;
 use std::borrow::Cow;
 use syntax::ast::*;
 use syntax::parse::{parser, token};
 use syntax::tokenstream::{TokenStream, TokenTree};
-use syntax_pos::symbol::Symbol;
+use syntax_pos::{symbol::Symbol, BytePos, Span};
 
 declare_clippy_lint! {
     /// **What it does:** This lint warns when you use `println!("")` to
@@ -184,7 +184,7 @@ impl EarlyLintPass for Write {
     fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
         if mac.node.path == sym!(println) {
             span_lint(cx, PRINT_STDOUT, mac.span, "use of `println!`");
-            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false).0 {
+            if let Some((fmtstr, ..)) = check_tts(cx, &mac.node.tts, false).0 {
                 if fmtstr == "" {
                     span_lint_and_sugg(
                         cx,
@@ -199,32 +199,50 @@ impl EarlyLintPass for Write {
             }
         } else if mac.node.path == sym!(print) {
             span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`");
-            if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, false) {
+            if let (Some((fmtstr, fmtstyle, fmt_sp)), _, is_raw) = check_tts(cx, &mac.node.tts, false) {
                 if check_newlines(&fmtstr, is_raw) {
-                    span_lint(
+                    span_lint_and_then(
                         cx,
                         PRINT_WITH_NEWLINE,
                         mac.span,
-                        "using `print!()` with a format string that ends in a \
-                         single newline, consider using `println!()` instead",
+                        "using `print!()` with a format string that ends in a single newline",
+                        |err| {
+                            err.multipart_suggestion(
+                                "use `println!` instead",
+                                vec![
+                                    (mac.node.path.span, String::from("println")),
+                                    (newline_span(&fmtstr, fmtstyle, fmt_sp), String::new()),
+                                ],
+                                Applicability::MachineApplicable,
+                            );
+                        },
                     );
                 }
             }
         } else if mac.node.path == sym!(write) {
-            if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, true) {
+            if let (Some((fmtstr, fmtstyle, fmt_sp)), _, is_raw) = check_tts(cx, &mac.node.tts, true) {
                 if check_newlines(&fmtstr, is_raw) {
-                    span_lint(
+                    span_lint_and_then(
                         cx,
                         WRITE_WITH_NEWLINE,
                         mac.span,
-                        "using `write!()` with a format string that ends in a \
-                         single newline, consider using `writeln!()` instead",
-                    );
+                        "using `write!()` with a format string that ends in a single newline",
+                        |err| {
+                            err.multipart_suggestion(
+                                "use `writeln!()` instead",
+                                vec![
+                                    (mac.node.path.span, String::from("writeln")),
+                                    (newline_span(&fmtstr, fmtstyle, fmt_sp), String::new()),
+                                ],
+                                Applicability::MachineApplicable,
+                            );
+                        },
+                    )
                 }
             }
         } else if mac.node.path == sym!(writeln) {
             let check_tts = check_tts(cx, &mac.node.tts, true);
-            if let Some(fmtstr) = check_tts.0 {
+            if let Some((fmtstr, ..)) = check_tts.0 {
                 if fmtstr == "" {
                     let mut applicability = Applicability::MachineApplicable;
                     let suggestion = check_tts.1.map_or_else(
@@ -250,10 +268,28 @@ impl EarlyLintPass for Write {
     }
 }
 
+/// Given a format string that ends in a newline and its span, calculates the span of the newline.
+fn newline_span(fmt_str: &str, str_style: StrStyle, sp: Span) -> Span {
+    let newline_sp_hi = sp.hi()
+        - match str_style {
+            StrStyle::Cooked => BytePos(1),
+            StrStyle::Raw(hashes) => BytePos((1 + hashes).into()),
+        };
+
+    let newline_sp_len = if fmt_str.ends_with('\n') {
+        BytePos(1)
+    } else {
+        BytePos(2)
+    };
+
+    sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
+}
+
 /// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
-/// options and a bool. The first part of the tuple is `format_str` of the macros. The second part
-/// of the tuple is in the `write[ln]!` case the expression the `format_str` should be written to.
-/// The final part is a boolean flag indicating if the string is a raw string.
+/// `Option`s. The first `Option` of the tuple is the macro's `format_str`. It includes
+/// the contents of the string, whether it's a raw string, and the span of the literal in the
+/// source. The second `Option` in the tuple is, in the `write[ln]!` case, the expression the
+/// `format_str` should be written to.
 ///
 /// Example:
 ///
@@ -266,9 +302,13 @@ impl EarlyLintPass for Write {
 /// ```
 /// will return
 /// ```rust,ignore
-/// (Some("string to write: {}"), Some(buf), false)
+/// (Some(("string to write: {}"), StrStyle::Cooked, "1:15-1:36"), Some(buf))
 /// ```
-fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<String>, Option<Expr>, bool) {
+fn check_tts<'a>(
+    cx: &EarlyContext<'a>,
+    tts: &TokenStream,
+    is_write: bool,
+) -> (Option<(String, StrStyle, Span)>, Option<Expr>, bool) {
     use fmt_macros::*;
     let tts = tts.clone();
     let mut is_raw = false;
@@ -299,10 +339,11 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
         }
     }
 
-    let fmtstr = match parser.parse_str().map_err(|mut err| err.cancel()) {
-        Ok(token) => token.0.to_string(),
+    let (fmtstr, fmtstyle) = match parser.parse_str().map_err(|mut err| err.cancel()) {
+        Ok((fmtstr, fmtstyle)) => (fmtstr.to_string(), fmtstyle),
         Err(_) => return (None, expr, is_raw),
     };
+    let fmtspan = parser.prev_span;
     let tmp = fmtstr.clone();
     let mut args = vec![];
     let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
@@ -330,11 +371,11 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
             ty: "",
         };
         if !parser.eat(&token::Comma) {
-            return (Some(fmtstr), expr, is_raw);
+            return (Some((fmtstr, fmtstyle, fmtspan)), expr, is_raw);
         }
         let token_expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
             Ok(expr) => expr,
-            Err(_) => return (Some(fmtstr), None, is_raw),
+            Err(_) => return (Some((fmtstr, fmtstyle, fmtspan)), None, is_raw),
         };
         match &token_expr.node {
             ExprKind::Lit(_) => {
diff --git a/tests/ui/print_with_newline.rs b/tests/ui/print_with_newline.rs
@@ -1,3 +1,6 @@
+// FIXME: Ideally these suggestions would be fixed via rustfix. Blocked by rust-lang/rust#53934
+// // run-rustfix
+
 #![allow(clippy::print_literal)]
 #![warn(clippy::print_with_newline)]
 
diff --git a/tests/ui/print_with_newline.stderr b/tests/ui/print_with_newline.stderr
@@ -1,52 +1,82 @@
-error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead
-  --> $DIR/print_with_newline.rs:5:5
+error: using `print!()` with a format string that ends in a single newline
+  --> $DIR/print_with_newline.rs:8:5
    |
 LL |     print!("Hello/n");
    |     ^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::print-with-newline` implied by `-D warnings`
+help: use `println!` instead
+   |
+LL |     println!("Hello");
+   |     ^^^^^^^       --
 
-error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead
-  --> $DIR/print_with_newline.rs:6:5
+error: using `print!()` with a format string that ends in a single newline
+  --> $DIR/print_with_newline.rs:9:5
    |
 LL |     print!("Hello {}/n", "world");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: use `println!` instead
+   |
+LL |     println!("Hello {}", "world");
+   |     ^^^^^^^          --
 
-error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead
-  --> $DIR/print_with_newline.rs:7:5
+error: using `print!()` with a format string that ends in a single newline
+  --> $DIR/print_with_newline.rs:10:5
    |
 LL |     print!("Hello {} {}/n", "world", "#2");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: use `println!` instead
+   |
+LL |     println!("Hello {} {}", "world", "#2");
+   |     ^^^^^^^             --
 
-error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead
-  --> $DIR/print_with_newline.rs:8:5
+error: using `print!()` with a format string that ends in a single newline
+  --> $DIR/print_with_newline.rs:11:5
    |
 LL |     print!("{}/n", 1265);
    |     ^^^^^^^^^^^^^^^^^^^^
+help: use `println!` instead
+   |
+LL |     println!("{}", 1265);
+   |     ^^^^^^^    --
 
-error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead
-  --> $DIR/print_with_newline.rs:27:5
+error: using `print!()` with a format string that ends in a single newline
+  --> $DIR/print_with_newline.rs:30:5
    |
 LL |     print!("//n"); // should fail
    |     ^^^^^^^^^^^^^^
+help: use `println!` instead
+   |
+LL |     println!("/"); // should fail
+   |     ^^^^^^^    --
 
-error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead
-  --> $DIR/print_with_newline.rs:34:5
+error: using `print!()` with a format string that ends in a single newline
+  --> $DIR/print_with_newline.rs:37:5
    |
 LL | /     print!(
 LL | |         "
 LL | | "
 LL | |     );
    | |_____^
+help: use `println!` instead
+   |
+LL |     println!(
+LL |         ""
+   |
 
-error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead
-  --> $DIR/print_with_newline.rs:38:5
+error: using `print!()` with a format string that ends in a single newline
+  --> $DIR/print_with_newline.rs:41:5
    |
 LL | /     print!(
 LL | |         r"
 LL | | "
 LL | |     );
    | |_____^
+help: use `println!` instead
+   |
+LL |     println!(
+LL |         r""
+   |
 
 error: aborting due to 7 previous errors
 
diff --git a/tests/ui/write_with_newline.rs b/tests/ui/write_with_newline.rs
@@ -1,3 +1,6 @@
+// FIXME: Ideally these suggestions would be fixed via rustfix. Blocked by rust-lang/rust#53934
+// // run-rustfix
+
 #![allow(clippy::write_literal)]
 #![warn(clippy::write_with_newline)]
 
diff --git a/tests/ui/write_with_newline.stderr b/tests/ui/write_with_newline.stderr
@@ -1,54 +1,86 @@
-error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead
-  --> $DIR/write_with_newline.rs:10:5
+error: using `write!()` with a format string that ends in a single newline
+  --> $DIR/write_with_newline.rs:13:5
    |
 LL |     write!(&mut v, "Hello/n");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::write-with-newline` implied by `-D warnings`
+help: use `writeln!()` instead
+   |
+LL |     writeln!(&mut v, "Hello");
+   |     ^^^^^^^               --
 
-error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead
-  --> $DIR/write_with_newline.rs:11:5
+error: using `write!()` with a format string that ends in a single newline
+  --> $DIR/write_with_newline.rs:14:5
    |
 LL |     write!(&mut v, "Hello {}/n", "world");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: use `writeln!()` instead
+   |
+LL |     writeln!(&mut v, "Hello {}", "world");
+   |     ^^^^^^^                  --
 
-error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead
-  --> $DIR/write_with_newline.rs:12:5
+error: using `write!()` with a format string that ends in a single newline
+  --> $DIR/write_with_newline.rs:15:5
    |
 LL |     write!(&mut v, "Hello {} {}/n", "world", "#2");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: use `writeln!()` instead
+   |
+LL |     writeln!(&mut v, "Hello {} {}", "world", "#2");
+   |     ^^^^^^^                     --
 
-error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead
-  --> $DIR/write_with_newline.rs:13:5
+error: using `write!()` with a format string that ends in a single newline
+  --> $DIR/write_with_newline.rs:16:5
    |
 LL |     write!(&mut v, "{}/n", 1265);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: use `writeln!()` instead
+   |
+LL |     writeln!(&mut v, "{}", 1265);
+   |     ^^^^^^^            --
 
-error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead
-  --> $DIR/write_with_newline.rs:32:5
+error: using `write!()` with a format string that ends in a single newline
+  --> $DIR/write_with_newline.rs:35:5
    |
 LL |     write!(&mut v, "//n"); // should fail
    |     ^^^^^^^^^^^^^^^^^^^^^^
+help: use `writeln!()` instead
+   |
+LL |     writeln!(&mut v, "/"); // should fail
+   |     ^^^^^^^            --
 
-error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead
-  --> $DIR/write_with_newline.rs:39:5
+error: using `write!()` with a format string that ends in a single newline
+  --> $DIR/write_with_newline.rs:42:5
    |
 LL | /     write!(
 LL | |         &mut v,
 LL | |         "
 LL | | "
 LL | |     );
    | |_____^
+help: use `writeln!()` instead
+   |
+LL |     writeln!(
+LL |         &mut v,
+LL |         ""
+   |
 
-error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead
-  --> $DIR/write_with_newline.rs:44:5
+error: using `write!()` with a format string that ends in a single newline
+  --> $DIR/write_with_newline.rs:47:5
    |
 LL | /     write!(
 LL | |         &mut v,
 LL | |         r"
 LL | | "
 LL | |     );
    | |_____^
+help: use `writeln!()` instead
+   |
+LL |     writeln!(
+LL |         &mut v,
+LL |         r""
+   |
 
 error: aborting due to 7 previous errors
 
