From add766493a580f2c5411d24540bf17032cafa06a Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Sun, 3 Nov 2019 00:41:22 -0400 Subject: [PATCH] don't warn on CRLF in `with_newline` lints --- clippy_lints/src/lib.rs | 2 ++ clippy_lints/src/write.rs | 54 ++++++++++++++---------------- tests/ui/print_with_newline.rs | 6 ++++ tests/ui/print_with_newline.stderr | 24 ++++++++++++- tests/ui/write_with_newline.rs | 6 ++++ tests/ui/write_with_newline.stderr | 24 ++++++++++++- 6 files changed, 85 insertions(+), 31 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 00028ec1965..c22693aee3b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -29,6 +29,8 @@ extern crate rustc_errors; #[allow(unused_extern_crates)] extern crate rustc_index; #[allow(unused_extern_crates)] +extern crate rustc_lexer; +#[allow(unused_extern_crates)] extern crate rustc_mir; #[allow(unused_extern_crates)] extern crate rustc_parse; diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index f53f8f016de..b8773b47a99 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -1,9 +1,12 @@ +use std::borrow::Cow; +use std::ops::Range; + use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then}; use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_errors::Applicability; +use rustc_lexer::unescape::{self, EscapeError}; use rustc_parse::parser; -use std::borrow::Cow; use syntax::ast::*; use syntax::token; use syntax::tokenstream::TokenStream; @@ -202,7 +205,7 @@ impl EarlyLintPass for Write { } else if mac.path == sym!(print) { span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`"); if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, false) { - if check_newlines(&fmt_str) { + if check_newlines(&fmt_str.contents, fmt_str.style) { span_lint_and_then( cx, PRINT_WITH_NEWLINE, @@ -223,7 +226,7 @@ impl EarlyLintPass for Write { } } else if mac.path == sym!(write) { if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, true) { - if check_newlines(&fmt_str) { + if check_newlines(&fmt_str.contents, fmt_str.style) { span_lint_and_then( cx, WRITE_WITH_NEWLINE, @@ -442,38 +445,31 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O } } -/// Checks if the format string constains a single newline that terminates it. +/// Checks if the format string contains a single newline that terminates it. /// /// Literal and escaped newlines are both checked (only literal for raw strings). -fn check_newlines(fmt_str: &FmtStr) -> bool { - let s = &fmt_str.contents; +fn check_newlines(contents: &str, style: StrStyle) -> bool { + let mut has_internal_newline = false; + let mut last_was_cr = false; + let mut should_lint = false; - if s.ends_with('\n') { - return true; - } else if let StrStyle::Raw(_) = fmt_str.style { - return false; - } + let mut cb = |r: Range, c: Result| { + let c = c.unwrap(); - if s.len() < 2 { - return false; - } - - let bytes = s.as_bytes(); - if bytes[bytes.len() - 2] != b'\\' || bytes[bytes.len() - 1] != b'n' { - return false; - } - - let mut escaping = false; - for (index, &byte) in bytes.iter().enumerate() { - if escaping { - if byte == b'n' { - return index == bytes.len() - 1; + if r.end == contents.len() && c == '\n' && !last_was_cr && !has_internal_newline { + should_lint = true; + } else { + last_was_cr = c == '\r'; + if c == '\n' { + has_internal_newline = true; } - escaping = false; - } else if byte == b'\\' { - escaping = true; } + }; + + match style { + StrStyle::Cooked => unescape::unescape_str(contents, &mut cb), + StrStyle::Raw(_) => unescape::unescape_raw_str(contents, &mut cb), } - false + should_lint } diff --git a/tests/ui/print_with_newline.rs b/tests/ui/print_with_newline.rs index 111a29faa41..3f710540e90 100644 --- a/tests/ui/print_with_newline.rs +++ b/tests/ui/print_with_newline.rs @@ -42,4 +42,10 @@ fn main() { r" " ); + + // Don't warn on CRLF (#4208) + print!("\r\n"); + print!("foo\r\n"); + print!("\\r\n"); //~ ERROR + print!("foo\rbar\n") // ~ ERROR } diff --git a/tests/ui/print_with_newline.stderr b/tests/ui/print_with_newline.stderr index c9e37eab6e1..05fe88915d6 100644 --- a/tests/ui/print_with_newline.stderr +++ b/tests/ui/print_with_newline.stderr @@ -84,5 +84,27 @@ LL | println!( LL | r"" | -error: aborting due to 7 previous errors +error: using `print!()` with a format string that ends in a single newline + --> $DIR/print_with_newline.rs:49:5 + | +LL | print!("/r/n"); //~ ERROR + | ^^^^^^^^^^^^^^^ + | +help: use `println!` instead + | +LL | println!("/r"); //~ ERROR + | ^^^^^^^ -- + +error: using `print!()` with a format string that ends in a single newline + --> $DIR/print_with_newline.rs:50:5 + | +LL | print!("foo/rbar/n") // ~ ERROR + | ^^^^^^^^^^^^^^^^^^^^ + | +help: use `println!` instead + | +LL | println!("foo/rbar") // ~ ERROR + | ^^^^^^^ -- + +error: aborting due to 9 previous errors diff --git a/tests/ui/write_with_newline.rs b/tests/ui/write_with_newline.rs index f0b13a69887..93afd73d111 100644 --- a/tests/ui/write_with_newline.rs +++ b/tests/ui/write_with_newline.rs @@ -49,4 +49,10 @@ fn main() { r" " ); + + // Don't warn on CRLF (#4208) + write!(&mut v, "\r\n"); + write!(&mut v, "foo\r\n"); + write!(&mut v, "\\r\n"); //~ ERROR + write!(&mut v, "foo\rbar\n"); } diff --git a/tests/ui/write_with_newline.stderr b/tests/ui/write_with_newline.stderr index 4d95bfeb17f..2473329ca72 100644 --- a/tests/ui/write_with_newline.stderr +++ b/tests/ui/write_with_newline.stderr @@ -88,5 +88,27 @@ LL | &mut v, LL | r"" | -error: aborting due to 7 previous errors +error: using `write!()` with a format string that ends in a single newline + --> $DIR/write_with_newline.rs:56:5 + | +LL | write!(&mut v, "/r/n"); //~ ERROR + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `writeln!()` instead + | +LL | writeln!(&mut v, "/r"); //~ ERROR + | ^^^^^^^ -- + +error: using `write!()` with a format string that ends in a single newline + --> $DIR/write_with_newline.rs:57:5 + | +LL | write!(&mut v, "foo/rbar/n"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `writeln!()` instead + | +LL | writeln!(&mut v, "foo/rbar"); + | ^^^^^^^ -- + +error: aborting due to 9 previous errors