Factor out some code in write.rs

Get rid of the too-many-lines error.
rust-lang · Dec 8, 2020 · 3187cad · 3187cad
1 parent 7063c36
commit 3187cad
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 61 deletions.
diff --git 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 mac.path == sym!(print) {
             if !is_build_script(cx) {
-                span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`");
+                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 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,
-                    );
-                }
+            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!`");
             }
-        } 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!(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 !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!(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
@@ -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
@@ -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
@@ -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