about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-05-07 12:13:35 +0000
committerbors <bors@rust-lang.org>2021-05-07 12:13:35 +0000
commitaf8cf9444c5aab898799c75b1db01afa0b28e2bd (patch)
treed2bb3bccef3aca397756fe65f5f1f180281abf5c
parent475666703f813a58c2b9df0b0395cb5439f3fe43 (diff)
parent5f3aae61af8ab129b743d13b14ec5519e97cc445 (diff)
downloadrust-af8cf9444c5aab898799c75b1db01afa0b28e2bd.tar.gz
rust-af8cf9444c5aab898799c75b1db01afa0b28e2bd.zip
Auto merge of #7183 - th1000s:write_nl_hint, r=flip1995
Handle write!(buf, "\n") case better

Make `write!(buf, "\n")` suggest `writeln!(buf)` by removing
the trailing comma from `writeln!(buf, )`.

changelog: [`write_with_newline`] suggestion on only "\n" improved
-rw-r--r--clippy_lints/src/write.rs23
-rw-r--r--tests/ui/write_with_newline.stderr4
2 files changed, 16 insertions, 11 deletions
diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs
index 7e962472c07..d0e79efa70d 100644
--- a/clippy_lints/src/write.rs
+++ b/clippy_lints/src/write.rs
@@ -279,8 +279,15 @@ impl EarlyLintPass for Write {
             span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`");
             self.lint_println_empty_string(cx, mac);
         } else if mac.path == sym!(write) {
-            if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), true) {
+            if let (Some(fmt_str), dest) = self.check_tts(cx, mac.args.inner_tokens(), true) {
                 if check_newlines(&fmt_str) {
+                    let (nl_span, only_nl) = newline_span(&fmt_str);
+                    let nl_span = match (dest, only_nl) {
+                        // Special case of `write!(buf, "\n")`: Mark everything from the end of
+                        // `buf` for removal so no trailing comma [`writeln!(buf, )`] remains.
+                        (Some(dest_expr), true) => Span::new(dest_expr.span.hi(), nl_span.hi(), nl_span.ctxt()),
+                        _ => nl_span,
+                    };
                     span_lint_and_then(
                         cx,
                         WRITE_WITH_NEWLINE,
@@ -289,10 +296,7 @@ impl EarlyLintPass for Write {
                         |err| {
                             err.multipart_suggestion(
                                 "use `writeln!()` instead",
-                                vec![
-                                    (mac.path.span, String::from("writeln")),
-                                    (newline_span(&fmt_str), String::new()),
-                                ],
+                                vec![(mac.path.span, String::from("writeln")), (nl_span, String::new())],
                                 Applicability::MachineApplicable,
                             );
                         },
@@ -329,12 +333,13 @@ impl EarlyLintPass for Write {
 
 /// Given a format string that ends in a newline and its span, calculates the span of the
 /// newline, or the format string itself if the format string consists solely of a newline.
-fn newline_span(fmtstr: &StrLit) -> Span {
+/// Return this and a boolean indicating whether it only consisted of a newline.
+fn newline_span(fmtstr: &StrLit) -> (Span, bool) {
     let sp = fmtstr.span;
     let contents = &fmtstr.symbol.as_str();
 
     if *contents == r"\n" {
-        return sp;
+        return (sp, true);
     }
 
     let newline_sp_hi = sp.hi()
@@ -351,7 +356,7 @@ fn newline_span(fmtstr: &StrLit) -> Span {
         panic!("expected format string to contain a newline");
     };
 
-    sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
+    (sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi), false)
 }
 
 /// Stores a list of replacement spans for each argument, but only if all the replacements used an
@@ -613,7 +618,7 @@ impl Write {
                     |err| {
                         err.multipart_suggestion(
                             &format!("use `{}!` instead", suggested),
-                            vec![(mac.path.span, suggested), (newline_span(&fmt_str), String::new())],
+                            vec![(mac.path.span, suggested), (newline_span(&fmt_str).0, String::new())],
                             Applicability::MachineApplicable,
                         );
                     },
diff --git a/tests/ui/write_with_newline.stderr b/tests/ui/write_with_newline.stderr
index a14e86122ee..cecc2ea9406 100644
--- a/tests/ui/write_with_newline.stderr
+++ b/tests/ui/write_with_newline.stderr
@@ -51,8 +51,8 @@ LL |     write!(&mut v, "/n");
    |
 help: use `writeln!()` instead
    |
-LL |     writeln!(&mut v, );
-   |     ^^^^^^^         --
+LL |     writeln!(&mut v);
+   |     ^^^^^^^       --
 
 error: using `write!()` with a format string that ends in a single newline
   --> $DIR/write_with_newline.rs:36:5