Skip to content

Commit

Permalink
Auto merge of #7183 - th1000s:write_nl_hint, r=flip1995
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed May 7, 2021
2 parents 4756667 + 5f3aae6 commit af8cf94
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
23 changes: 14 additions & 9 deletions clippy_lints/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
);
},
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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,
);
},
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/write_with_newline.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit af8cf94

Please sign in to comment.