Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint print_stderr #6367

Merged
merged 5 commits into from
Dec 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2006,6 +2006,7 @@ Released 2018-09-13
[`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
[`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
[`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal
[`print_stderr`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_stderr
[`print_stdout`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_stdout
[`print_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline
[`println_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#println_empty_string
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&wildcard_imports::WILDCARD_IMPORTS,
&write::PRINTLN_EMPTY_STRING,
&write::PRINT_LITERAL,
&write::PRINT_STDERR,
&write::PRINT_STDOUT,
&write::PRINT_WITH_NEWLINE,
&write::USE_DEBUG,
Expand Down Expand Up @@ -1247,6 +1248,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::RC_BUFFER),
LintId::of(&unwrap_in_result::UNWRAP_IN_RESULT),
LintId::of(&verbose_file_reads::VERBOSE_FILE_READS),
LintId::of(&write::PRINT_STDERR),
LintId::of(&write::PRINT_STDOUT),
LintId::of(&write::USE_DEBUG),
]);
Expand Down
107 changes: 70 additions & 37 deletions clippy_lints/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,24 @@ declare_clippy_lint! {
"printing on stdout"
}

declare_clippy_lint! {
/// **What it does:** Checks for printing on *stderr*. The purpose of this lint
/// is to catch debugging remnants.
///
/// **Why is this bad?** People often print on *stderr* while debugging an
/// application and might forget to remove those prints afterward.
///
/// **Known problems:** Only catches `eprint!` and `eprintln!` calls.
///
/// **Example:**
/// ```rust
/// eprintln!("Hello world!");
/// ```
pub PRINT_STDERR,
restriction,
"printing on stderr"
}

declare_clippy_lint! {
/// **What it does:** Checks for use of `Debug` formatting. The purpose of this
/// lint is to catch debugging remnants.
Expand Down Expand Up @@ -201,6 +219,7 @@ impl_lint_pass!(Write => [
PRINT_WITH_NEWLINE,
PRINTLN_EMPTY_STRING,
PRINT_STDOUT,
PRINT_STDERR,
USE_DEBUG,
PRINT_LITERAL,
WRITE_WITH_NEWLINE,
Expand Down Expand Up @@ -243,47 +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!(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) {
Expand Down Expand Up @@ -487,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.
Expand Down
49 changes: 49 additions & 0 deletions tests/ui/eprint_with_newline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#![allow(clippy::print_literal)]
#![warn(clippy::print_with_newline)]

fn main() {
eprint!("Hello\n");
eprint!("Hello {}\n", "world");
eprint!("Hello {} {}\n", "world", "#2");
eprint!("{}\n", 1265);
eprint!("\n");

// these are all fine
eprint!("");
eprint!("Hello");
eprintln!("Hello");
eprintln!("Hello\n");
eprintln!("Hello {}\n", "world");
eprint!("Issue\n{}", 1265);
eprint!("{}", 1265);
eprint!("\n{}", 1275);
eprint!("\n\n");
eprint!("like eof\n\n");
eprint!("Hello {} {}\n\n", "world", "#2");
eprintln!("\ndon't\nwarn\nfor\nmultiple\nnewlines\n"); // #3126
eprintln!("\nbla\n\n"); // #3126

// Escaping
eprint!("\\n"); // #3514
eprint!("\\\n"); // should fail
eprint!("\\\\n");

// Raw strings
eprint!(r"\n"); // #3778

// Literal newlines should also fail
eprint!(
"
"
);
eprint!(
r"
"
);

// Don't warn on CRLF (#4208)
eprint!("\r\n");
eprint!("foo\r\n");
eprint!("\\r\n"); //~ ERROR
eprint!("foo\rbar\n") // ~ ERROR
}
121 changes: 121 additions & 0 deletions tests/ui/eprint_with_newline.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:5:5
|
LL | eprint!("Hello/n");
| ^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::print-with-newline` implied by `-D warnings`
help: use `eprintln!` instead
|
LL | eprintln!("Hello");
| ^^^^^^^^ --

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:6:5
|
LL | eprint!("Hello {}/n", "world");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `eprintln!` instead
|
LL | eprintln!("Hello {}", "world");
| ^^^^^^^^ --

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:7:5
|
LL | eprint!("Hello {} {}/n", "world", "#2");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `eprintln!` instead
|
LL | eprintln!("Hello {} {}", "world", "#2");
| ^^^^^^^^ --

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:8:5
|
LL | eprint!("{}/n", 1265);
| ^^^^^^^^^^^^^^^^^^^^^
|
help: use `eprintln!` instead
|
LL | eprintln!("{}", 1265);
| ^^^^^^^^ --

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:9:5
|
LL | eprint!("/n");
| ^^^^^^^^^^^^^
|
help: use `eprintln!` instead
|
LL | eprintln!();
| ^^^^^^^^ --

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:28:5
|
LL | eprint!("//n"); // should fail
| ^^^^^^^^^^^^^^^
|
help: use `eprintln!` instead
|
LL | eprintln!("/"); // should fail
| ^^^^^^^^ --

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:35:5
|
LL | / eprint!(
LL | | "
LL | | "
LL | | );
| |_____^
|
help: use `eprintln!` instead
|
LL | eprintln!(
LL | ""
|

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:39:5
|
LL | / eprint!(
LL | | r"
LL | | "
LL | | );
| |_____^
|
help: use `eprintln!` instead
|
LL | eprintln!(
LL | r""
|

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:47:5
|
LL | eprint!("/r/n"); //~ ERROR
| ^^^^^^^^^^^^^^^^
|
help: use `eprintln!` instead
|
LL | eprintln!("/r"); //~ ERROR
| ^^^^^^^^ --

error: using `eprint!()` with a format string that ends in a single newline
--> $DIR/eprint_with_newline.rs:48:5
|
LL | eprint!("foo/rbar/n") // ~ ERROR
| ^^^^^^^^^^^^^^^^^^^^^
|
help: use `eprintln!` instead
|
LL | eprintln!("foo/rbar") // ~ ERROR
| ^^^^^^^^ --

error: aborting due to 10 previous errors

8 changes: 8 additions & 0 deletions tests/ui/print_stderr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![warn(clippy::print_stderr)]

fn main() {
eprintln!("Hello");
println!("This should not do anything");
eprint!("World");
print!("Nor should this");
}
16 changes: 16 additions & 0 deletions tests/ui/print_stderr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: use of `eprintln!`
--> $DIR/print_stderr.rs:4:5
|
LL | eprintln!("Hello");
| ^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::print-stderr` implied by `-D warnings`

error: use of `eprint!`
--> $DIR/print_stderr.rs:6:5
|
LL | eprint!("World");
| ^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

7 changes: 7 additions & 0 deletions tests/ui/println_empty_string.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,11 @@ fn main() {
match "a" {
_ => println!(),
}

eprintln!();
eprintln!();

match "a" {
_ => eprintln!(),
}
}
7 changes: 7 additions & 0 deletions tests/ui/println_empty_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,11 @@ fn main() {
match "a" {
_ => println!(""),
}

eprintln!();
eprintln!("");

match "a" {
_ => eprintln!(""),
}
}
Loading