From ef72b2cac0c9e0715d384711b3f08567c013300d Mon Sep 17 00:00:00 2001 From: phil Date: Mon, 18 Feb 2019 11:39:23 -0500 Subject: [PATCH] Check {print,write}_with_newline for literal newline Both regular strings and raw strings can contain literal newlines. This commit extends the lint to also warn about terminating strings with these. Behaviour handling for raw strings is also moved into `check_newlines` by passing in the `is_raw` boolean from `check_tts` as [suggested](https://github.com/rust-lang/rust-clippy/pull/3781#pullrequestreview-204663732) --- clippy_lints/src/write.rs | 13 ++++++++++--- tests/ui/print_with_newline.rs | 10 ++++++++++ tests/ui/print_with_newline.stderr | 20 +++++++++++++++++++- tests/ui/write_with_newline.rs | 12 ++++++++++++ tests/ui/write_with_newline.stderr | 22 +++++++++++++++++++++- 5 files changed, 72 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 4f771b2af55..012c34e9b53 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -201,7 +201,7 @@ fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) { } else if mac.node.path == "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 !is_raw && check_newlines(&fmtstr) { + if check_newlines(&fmtstr, is_raw) { span_lint( cx, PRINT_WITH_NEWLINE, @@ -213,7 +213,7 @@ fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) { } } else if mac.node.path == "write" { if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, true) { - if !is_raw && check_newlines(&fmtstr) { + if check_newlines(&fmtstr, is_raw) { span_lint( cx, WRITE_WITH_NEWLINE, @@ -382,7 +382,14 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O } // Checks if `s` constains a single newline that terminates it -fn check_newlines(s: &str) -> bool { +// Literal and escaped newlines are both checked (only literal for raw strings) +fn check_newlines(s: &str, is_raw: bool) -> bool { + if s.ends_with('\n') { + return true; + } else if is_raw { + return false; + } + if s.len() < 2 { return false; } diff --git a/tests/ui/print_with_newline.rs b/tests/ui/print_with_newline.rs index 9df4b9052de..1c219ecb325 100644 --- a/tests/ui/print_with_newline.rs +++ b/tests/ui/print_with_newline.rs @@ -29,4 +29,14 @@ fn main() { // Raw strings print!(r"\n"); // #3778 + + // Literal newlines should also fail + print!( + " +" + ); + print!( + r" +" + ); } diff --git a/tests/ui/print_with_newline.stderr b/tests/ui/print_with_newline.stderr index 1d89a16e090..ff89b0d3fd4 100644 --- a/tests/ui/print_with_newline.stderr +++ b/tests/ui/print_with_newline.stderr @@ -30,5 +30,23 @@ error: using `print!()` with a format string that ends in a single newline, cons LL | print!("//n"); // should fail | ^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead + --> $DIR/print_with_newline.rs:34:5 + | +LL | / print!( +LL | | " +LL | | " +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 + | +LL | / print!( +LL | | r" +LL | | " +LL | | ); + | |_____^ + +error: aborting due to 7 previous errors diff --git a/tests/ui/write_with_newline.rs b/tests/ui/write_with_newline.rs index 3575dd6da80..dd80dc0cf9c 100644 --- a/tests/ui/write_with_newline.rs +++ b/tests/ui/write_with_newline.rs @@ -34,4 +34,16 @@ fn main() { // Raw strings write!(&mut v, r"\n"); // #3778 + + // Literal newlines should also fail + write!( + &mut v, + " +" + ); + write!( + &mut v, + r" +" + ); } diff --git a/tests/ui/write_with_newline.stderr b/tests/ui/write_with_newline.stderr index b0761f3b081..3a31f61a277 100644 --- a/tests/ui/write_with_newline.stderr +++ b/tests/ui/write_with_newline.stderr @@ -30,5 +30,25 @@ error: using `write!()` with a format string that ends in a single newline, cons LL | write!(&mut v, "//n"); // should fail | ^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: using `write!()` with a format string that ends in a single newline, consider using `writeln!()` instead + --> $DIR/write_with_newline.rs:39:5 + | +LL | / write!( +LL | | &mut v, +LL | | " +LL | | " +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 + | +LL | / write!( +LL | | &mut v, +LL | | r" +LL | | " +LL | | ); + | |_____^ + +error: aborting due to 7 previous errors