From 3187cad8ec0e9c32272341028526d6f1e208a704 Mon Sep 17 00:00:00 2001 From: Eduardo Broto Date: Tue, 8 Dec 2020 23:17:12 +0100 Subject: [PATCH] Factor out some code in write.rs Get rid of the too-many-lines error. --- clippy_lints/src/write.rs | 112 ++++++++++++--------------- tests/ui/println_empty_string.fixed | 7 ++ tests/ui/println_empty_string.rs | 7 ++ tests/ui/println_empty_string.stderr | 14 +++- 4 files changed, 78 insertions(+), 62 deletions(-) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 0fd56f785..337f7a229 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -262,71 +262,22 @@ impl EarlyLintPass for Write { .map_or(false, |crate_name| crate_name == "build_script_build") } - if mac.path == sym!(println) { - if !is_build_script(cx) { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); - } - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { - if fmt_str.symbol == Symbol::intern("") { - span_lint_and_sugg( - cx, - PRINTLN_EMPTY_STRING, - mac.span(), - "using `println!(\"\")`", - "replace it with", - "println!()".to_string(), - Applicability::MachineApplicable, - ); - } - } - } else if mac.path == sym!(eprintln) { - span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`"); - } else if mac.path == sym!(eprint) { - span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprint!`"); - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { - if check_newlines(&fmt_str) { - span_lint_and_then( - cx, - PRINT_WITH_NEWLINE, - mac.span(), - "using `eprint!()` with a format string that ends in a single newline", - |err| { - err.multipart_suggestion( - "use `eprintln!` instead", - vec![ - (mac.path.span, String::from("eprintln")), - (newline_span(&fmt_str), String::new()), - ], - Applicability::MachineApplicable, - ); - }, - ); - } - } - } else if mac.path == sym!(print) { + if mac.path == sym!(print) { if !is_build_script(cx) { span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); } - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { - if check_newlines(&fmt_str) { - span_lint_and_then( - cx, - PRINT_WITH_NEWLINE, - mac.span(), - "using `print!()` with a format string that ends in a single newline", - |err| { - err.multipart_suggestion( - "use `println!` instead", - vec![ - (mac.path.span, String::from("println")), - (newline_span(&fmt_str), String::new()), - ], - Applicability::MachineApplicable, - ); - }, - ); - } + self.lint_print_with_newline(cx, mac); + } else if mac.path == sym!(println) { + if !is_build_script(cx) { + span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); } + self.lint_println_empty_string(cx, mac); + } else if mac.path == sym!(eprint) { + span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprint!`"); + self.lint_print_with_newline(cx, mac); + } else if mac.path == sym!(eprintln) { + 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 check_newlines(&fmt_str) { @@ -530,6 +481,45 @@ impl Write { } } } + + fn lint_println_empty_string(&self, cx: &EarlyContext<'_>, mac: &MacCall) { + if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { + if fmt_str.symbol == Symbol::intern("") { + let name = mac.path.segments[0].ident.name; + span_lint_and_sugg( + cx, + PRINTLN_EMPTY_STRING, + mac.span(), + &format!("using `{}!(\"\")`", name), + "replace it with", + format!("{}!()", name), + Applicability::MachineApplicable, + ); + } + } + } + + fn lint_print_with_newline(&self, cx: &EarlyContext<'_>, mac: &MacCall) { + if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { + if check_newlines(&fmt_str) { + let name = mac.path.segments[0].ident.name; + let suggested = format!("{}ln", name); + span_lint_and_then( + cx, + PRINT_WITH_NEWLINE, + mac.span(), + &format!("using `{}!()` with a format string that ends in a single newline", name), + |err| { + err.multipart_suggestion( + &format!("use `{}!` instead", suggested), + vec![(mac.path.span, suggested), (newline_span(&fmt_str), String::new())], + Applicability::MachineApplicable, + ); + }, + ); + } + } + } } /// Checks if the format string contains a single newline that terminates it. diff --git a/tests/ui/println_empty_string.fixed b/tests/ui/println_empty_string.fixed index 2b889b62e..976068092 100644 --- a/tests/ui/println_empty_string.fixed +++ b/tests/ui/println_empty_string.fixed @@ -8,4 +8,11 @@ fn main() { match "a" { _ => println!(), } + + eprintln!(); + eprintln!(); + + match "a" { + _ => eprintln!(), + } } diff --git a/tests/ui/println_empty_string.rs b/tests/ui/println_empty_string.rs index 890f5f684..80fdb3e6e 100644 --- a/tests/ui/println_empty_string.rs +++ b/tests/ui/println_empty_string.rs @@ -8,4 +8,11 @@ fn main() { match "a" { _ => println!(""), } + + eprintln!(); + eprintln!(""); + + match "a" { + _ => eprintln!(""), + } } diff --git a/tests/ui/println_empty_string.stderr b/tests/ui/println_empty_string.stderr index 23112b881..17fe4ea74 100644 --- a/tests/ui/println_empty_string.stderr +++ b/tests/ui/println_empty_string.stderr @@ -12,5 +12,17 @@ error: using `println!("")` LL | _ => println!(""), | ^^^^^^^^^^^^ help: replace it with: `println!()` -error: aborting due to 2 previous errors +error: using `eprintln!("")` + --> $DIR/println_empty_string.rs:13:5 + | +LL | eprintln!(""); + | ^^^^^^^^^^^^^ help: replace it with: `eprintln!()` + +error: using `eprintln!("")` + --> $DIR/println_empty_string.rs:16:14 + | +LL | _ => eprintln!(""), + | ^^^^^^^^^^^^^ help: replace it with: `eprintln!()` + +error: aborting due to 4 previous errors