diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 36f7fee969b9..012c34e9b535 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -5,7 +5,7 @@ use rustc_errors::Applicability; use std::borrow::Cow; use syntax::ast::*; use syntax::parse::{parser, token}; -use syntax::tokenstream::TokenStream; +use syntax::tokenstream::{TokenStream, TokenTree}; /// **What it does:** This lint warns when you use `println!("")` to /// print a newline. @@ -200,8 +200,8 @@ impl EarlyLintPass for Pass { } } else if mac.node.path == "print" { span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`"); - if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false).0 { - if check_newlines(&fmtstr) { + if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, false) { + if check_newlines(&fmtstr, is_raw) { span_lint( cx, PRINT_WITH_NEWLINE, @@ -212,8 +212,8 @@ impl EarlyLintPass for Pass { } } } else if mac.node.path == "write" { - if let Some(fmtstr) = check_tts(cx, &mac.node.tts, true).0 { - if check_newlines(&fmtstr) { + if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, true) { + if check_newlines(&fmtstr, is_raw) { span_lint( cx, WRITE_WITH_NEWLINE, @@ -252,8 +252,9 @@ impl EarlyLintPass for Pass { } /// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two -/// options. 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. +/// 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. /// /// Example: /// @@ -263,34 +264,49 @@ impl EarlyLintPass for Pass { /// ``` /// will return /// ```rust,ignore -/// (Some("string to write: {}"), Some(buf)) +/// (Some("string to write: {}"), Some(buf), false) /// ``` -fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option, Option) { +fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option, Option, bool) { use fmt_macros::*; let tts = tts.clone(); + let mut is_raw = false; + if let TokenStream(Some(tokens)) = &tts { + for token in tokens.iter() { + if let (TokenTree::Token(_, token::Token::Literal(lit, _)), _) = token { + match lit { + token::Lit::Str_(_) => break, + token::Lit::StrRaw(_, _) => { + is_raw = true; + break; + }, + _ => {}, + } + } + } + } let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false); let mut expr: Option = None; if is_write { expr = match parser.parse_expr().map_err(|mut err| err.cancel()) { Ok(p) => Some(p.into_inner()), - Err(_) => return (None, None), + Err(_) => return (None, None, is_raw), }; // might be `writeln!(foo)` if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() { - return (None, expr); + return (None, expr, is_raw); } } let fmtstr = match parser.parse_str().map_err(|mut err| err.cancel()) { Ok(token) => token.0.to_string(), - Err(_) => return (None, expr), + Err(_) => return (None, expr, is_raw), }; let tmp = fmtstr.clone(); let mut args = vec![]; let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false); while let Some(piece) = fmt_parser.next() { if !fmt_parser.errors.is_empty() { - return (None, expr); + return (None, expr, is_raw); } if let Piece::NextArgument(arg) = piece { if arg.format.ty == "?" { @@ -312,11 +328,11 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O ty: "", }; if !parser.eat(&token::Comma) { - return (Some(fmtstr), expr); + return (Some(fmtstr), expr, is_raw); } let token_expr = match parser.parse_expr().map_err(|mut err| err.cancel()) { Ok(expr) => expr, - Err(_) => return (Some(fmtstr), None), + Err(_) => return (Some(fmtstr), None, is_raw), }; match &token_expr.node { ExprKind::Lit(_) => { @@ -366,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 991cd7311e5f..1c219ecb325b 100644 --- a/tests/ui/print_with_newline.rs +++ b/tests/ui/print_with_newline.rs @@ -21,4 +21,22 @@ fn main() { print!("Hello {} {}\n\n", "world", "#2"); println!("\ndon't\nwarn\nfor\nmultiple\nnewlines\n"); // #3126 println!("\nbla\n\n"); // #3126 + + // Escaping + print!("\\n"); // #3514 + print!("\\\n"); // should fail + print!("\\\\n"); + + // 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 a731212be87c..ff89b0d3fd44 100644 --- a/tests/ui/print_with_newline.stderr +++ b/tests/ui/print_with_newline.stderr @@ -24,5 +24,29 @@ error: using `print!()` with a format string that ends in a single newline, cons LL | print!("{}/n", 1265); | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: using `print!()` with a format string that ends in a single newline, consider using `println!()` instead + --> $DIR/print_with_newline.rs:27:5 + | +LL | print!("//n"); // 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 + | +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 2f53d4561af8..dd80dc0cf9c5 100644 --- a/tests/ui/write_with_newline.rs +++ b/tests/ui/write_with_newline.rs @@ -29,6 +29,21 @@ fn main() { // Escaping write!(&mut v, "\\n"); // #3514 - write!(&mut v, "\\\n"); + write!(&mut v, "\\\n"); // should fail write!(&mut v, "\\\\n"); + + // 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 1f4395c26213..3a31f61a277c 100644 --- a/tests/ui/write_with_newline.stderr +++ b/tests/ui/write_with_newline.stderr @@ -27,8 +27,28 @@ LL | write!(&mut v, "{}/n", 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 | -LL | write!(&mut v, "//n"); +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