Skip to content

Commit 3fa3bd8

Browse files
committed
Don't fail for raw string ending in \n
Pass tests for #3778, {print,write}_with_newline false positive This change guards the lint from checking newlines with a sort of complicated check to see if it's a raw string. Raw strings shouldn't be newline-checked, since r"\n" is literally \-n, not a newline. I think it's ok not to check for _literal_ newlines at the end of raw strings, but maybe that's debatable. I... don't think this code is that great. I wanted to write the check after `check_tts`, but that was too late -- raw string type info is lost (or I couldn't find it). Putting it inside `check_tts` feels heavy-duty and the check itself feels like a brittle reach possibly into places it shouldn't. Maybe someone can fix this up :)
1 parent cf5d4a1 commit 3fa3bd8

File tree

1 file changed

+31
-15
lines changed

1 file changed

+31
-15
lines changed

clippy_lints/src/write.rs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc_errors::Applicability;
55
use std::borrow::Cow;
66
use syntax::ast::*;
77
use syntax::parse::{parser, token};
8-
use syntax::tokenstream::TokenStream;
8+
use syntax::tokenstream::{TokenStream, TokenTree};
99

1010
/// **What it does:** This lint warns when you use `println!("")` to
1111
/// print a newline.
@@ -200,8 +200,8 @@ impl EarlyLintPass for Pass {
200200
}
201201
} else if mac.node.path == "print" {
202202
span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`");
203-
if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false).0 {
204-
if check_newlines(&fmtstr) {
203+
if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, false) {
204+
if !is_raw && check_newlines(&fmtstr) {
205205
span_lint(
206206
cx,
207207
PRINT_WITH_NEWLINE,
@@ -212,8 +212,8 @@ impl EarlyLintPass for Pass {
212212
}
213213
}
214214
} else if mac.node.path == "write" {
215-
if let Some(fmtstr) = check_tts(cx, &mac.node.tts, true).0 {
216-
if check_newlines(&fmtstr) {
215+
if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, true) {
216+
if !is_raw && check_newlines(&fmtstr) {
217217
span_lint(
218218
cx,
219219
WRITE_WITH_NEWLINE,
@@ -252,8 +252,9 @@ impl EarlyLintPass for Pass {
252252
}
253253

254254
/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
255-
/// options. The first part of the tuple is `format_str` of the macros. The second part of the tuple
256-
/// is in the `write[ln]!` case the expression the `format_str` should be written to.
255+
/// options and a bool. The first part of the tuple is `format_str` of the macros. The second part
256+
/// of the tuple is in the `write[ln]!` case the expression the `format_str` should be written to.
257+
/// The final part is a boolean flag indicating if the string is a raw string.
257258
///
258259
/// Example:
259260
///
@@ -263,34 +264,49 @@ impl EarlyLintPass for Pass {
263264
/// ```
264265
/// will return
265266
/// ```rust,ignore
266-
/// (Some("string to write: {}"), Some(buf))
267+
/// (Some("string to write: {}"), Some(buf), false)
267268
/// ```
268-
fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<String>, Option<Expr>) {
269+
fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<String>, Option<Expr>, bool) {
269270
use fmt_macros::*;
270271
let tts = tts.clone();
272+
let mut is_raw = false;
273+
if let TokenStream(Some(tokens)) = &tts {
274+
for token in tokens.iter() {
275+
if let (TokenTree::Token(_, token::Token::Literal(lit, _)), _) = token {
276+
match lit {
277+
token::Lit::Str_(_) => break,
278+
token::Lit::StrRaw(_, _) => {
279+
is_raw = true;
280+
break;
281+
},
282+
_ => {},
283+
}
284+
}
285+
}
286+
}
271287
let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false);
272288
let mut expr: Option<Expr> = None;
273289
if is_write {
274290
expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
275291
Ok(p) => Some(p.into_inner()),
276-
Err(_) => return (None, None),
292+
Err(_) => return (None, None, is_raw),
277293
};
278294
// might be `writeln!(foo)`
279295
if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
280-
return (None, expr);
296+
return (None, expr, is_raw);
281297
}
282298
}
283299

284300
let fmtstr = match parser.parse_str().map_err(|mut err| err.cancel()) {
285301
Ok(token) => token.0.to_string(),
286-
Err(_) => return (None, expr),
302+
Err(_) => return (None, expr, is_raw),
287303
};
288304
let tmp = fmtstr.clone();
289305
let mut args = vec![];
290306
let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
291307
while let Some(piece) = fmt_parser.next() {
292308
if !fmt_parser.errors.is_empty() {
293-
return (None, expr);
309+
return (None, expr, is_raw);
294310
}
295311
if let Piece::NextArgument(arg) = piece {
296312
if arg.format.ty == "?" {
@@ -312,11 +328,11 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
312328
ty: "",
313329
};
314330
if !parser.eat(&token::Comma) {
315-
return (Some(fmtstr), expr);
331+
return (Some(fmtstr), expr, is_raw);
316332
}
317333
let token_expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
318334
Ok(expr) => expr,
319-
Err(_) => return (Some(fmtstr), None),
335+
Err(_) => return (Some(fmtstr), None, is_raw),
320336
};
321337
match &token_expr.node {
322338
ExprKind::Lit(_) => {

0 commit comments

Comments
 (0)