Add lint print_stderr #6367

justjosias · 2020-11-22T18:10:40Z

Resolves #6348
Almost identical to print_stdout, this lint applies to the eprintln! and eprint! macros rather than println! and print!.

changelog: Add new lint [print_stderr]. [println_empty_string] and [print_with_newline] now apply to eprint!() and eprintln!() respectively.

rust-highfive · 2020-11-22T18:10:43Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ebroto (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

bors · 2020-11-27T07:17:19Z

☔ The latest upstream changes (presumably #6389) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

ebroto

Looking at the phrasing in PRINT_STDOUT documentation, it seems targetting only print and println was intended, so I think it's OK to have a separate lint for eprint and eprintln.

tests/ui/print_stderr.rs

ebroto · 2020-11-28T22:27:43Z

clippy_lints/src/write.rs

@@ -260,6 +279,10 @@ impl EarlyLintPass for Write {
                    );
                }
            }
+        } else if mac.path == sym!(eprintln) {
+            span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`");


Looking at the surrounding code and the description of those lints, it seems PRINTLN_EMPTY_STRING and PRINT_WITH_NEWLINE could apply to eprintln and eprint respectively.

Would you mind doing that change in this PR? Alternatively we can open a different issue and leave it as a follow-up.

The application of PRINTLN_EMPTY_STRING and PRINT_WITH_NEWLINE to eprintln and eprint would also require giving different suggestions for each case (using eprint[ln] instead of print[ln]). Should these be divided into separate lints as well?

Thanks for adding this change!

No, I think we just need to change the suggestion depending on the case, but without adding new lints.

BTW it seems you need to rebase on top of the master branch, there are a lot of commits that are not related to this PR. Do not hesitate to ping me if you find any problem when doing that.

@ebroto It seems I have made a mess of the branch's history. I don't know enough git to deal with this. I've tried rebasing several times.

@justjosias it should be fixed now.

FYI I did an interactive rebase on top of the current master, which left 4 commits. 2 of them were duplicates, so I only kept the newest ones and fixed some conflicts during the rebase.

BTW do not forget to commit the changes to the stderr file; since we added some lines, the line numbers of the previous error messages has changed.

@ebroto It appears master no longer compiles, so I can't test anything or proceed with other changes. But I fixed the stderr file.

@justjosias FYI the problem you mention has been fixed, it should work if you 1) re-run setup-toolchain.sh and 2) rebase on top of the latest master.

I screwed up the history again. I'll figure out the interactive rebase to fix it.

EDIT: Fixed! Thanks for the advice. I'm now going to work on this last change.

Resolves rust-lang#6348 Almost identical to print_stdout, this lint applies to the `eprintln!` and `eprint!` macros rather than `println!` and `print!`.

Get rid of the too-many-lines error.

ebroto · 2020-12-08T22:19:28Z

FYI the CI error was related to a function having too many lines in write.rs. I've factored out some code to get rid of it.

Thanks for your contribution and don't hesitate to take another issue :)

ebroto · 2020-12-08T22:19:35Z

@bors r+

bors · 2020-12-08T22:19:37Z

📌 Commit 3187cad has been approved by ebroto

Add lint print_stderr Resolves #6348 Almost identical to print_stdout, this lint applies to the `eprintln!` and `eprint!` macros rather than `println!` and `print!`.

bors · 2020-12-08T22:19:45Z

⌛ Testing commit 3187cad with merge 02b0552...

bors · 2020-12-08T22:20:19Z

💔 Test failed - checks-action_test

ebroto · 2020-12-08T22:22:45Z

@bors retry (missing changelog)

bors · 2020-12-08T22:22:54Z

⌛ Testing commit 3187cad with merge b02b0c7...

bors · 2020-12-08T22:45:20Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: ebroto
Pushing b02b0c7 to master...

rust-highfive assigned ebroto Nov 22, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 22, 2020

ebroto suggested changes Nov 28, 2020

View reviewed changes

ebroto added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 28, 2020

ebroto force-pushed the 6348-print-stderr branch from b2380f6 to e080791 Compare November 30, 2020 22:59

justjosias force-pushed the 6348-print-stderr branch from c33fc5a to e59cbbb Compare December 4, 2020 08:43

justjosias and others added 5 commits December 7, 2020 23:38

Add lint print_stderr

b81141c

Resolves rust-lang#6348 Almost identical to print_stdout, this lint applies to the `eprintln!` and `eprint!` macros rather than `println!` and `print!`.

Add negative tests

51cee15

Fix print_stderr.stderr test

b04bfbd

Add eprint! to print_with_newline lint

7063c36

Factor out some code in write.rs

3187cad

Get rid of the too-many-lines error.

ebroto force-pushed the 6348-print-stderr branch from f2952db to 3187cad Compare December 8, 2020 22:18

ebroto approved these changes Dec 8, 2020

View reviewed changes

ebroto removed the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Dec 8, 2020

bors merged commit b02b0c7 into rust-lang:master Dec 8, 2020

justjosias deleted the 6348-print-stderr branch December 9, 2020 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lint print_stderr #6367

Add lint print_stderr #6367

justjosias commented Nov 22, 2020 •

edited by ebroto

Loading

rust-highfive commented Nov 22, 2020

bors commented Nov 27, 2020

ebroto left a comment

ebroto Nov 28, 2020

justjosias Nov 29, 2020

ebroto Nov 29, 2020

justjosias Nov 29, 2020

ebroto Nov 30, 2020

justjosias Dec 1, 2020

ebroto Dec 3, 2020

justjosias Dec 4, 2020 •

edited

Loading

ebroto commented Dec 8, 2020

ebroto commented Dec 8, 2020

bors commented Dec 8, 2020

bors commented Dec 8, 2020

bors commented Dec 8, 2020

ebroto commented Dec 8, 2020

bors commented Dec 8, 2020

bors commented Dec 8, 2020

Add lint print_stderr #6367

Add lint print_stderr #6367

Conversation

justjosias commented Nov 22, 2020 • edited by ebroto Loading

rust-highfive commented Nov 22, 2020

bors commented Nov 27, 2020

ebroto left a comment

Choose a reason for hiding this comment

ebroto Nov 28, 2020

Choose a reason for hiding this comment

justjosias Nov 29, 2020

Choose a reason for hiding this comment

ebroto Nov 29, 2020

Choose a reason for hiding this comment

justjosias Nov 29, 2020

Choose a reason for hiding this comment

ebroto Nov 30, 2020

Choose a reason for hiding this comment

justjosias Dec 1, 2020

Choose a reason for hiding this comment

ebroto Dec 3, 2020

Choose a reason for hiding this comment

justjosias Dec 4, 2020 • edited Loading

Choose a reason for hiding this comment

ebroto commented Dec 8, 2020

ebroto commented Dec 8, 2020

bors commented Dec 8, 2020

bors commented Dec 8, 2020

bors commented Dec 8, 2020

ebroto commented Dec 8, 2020

bors commented Dec 8, 2020

bors commented Dec 8, 2020

justjosias commented Nov 22, 2020 •

edited by ebroto

Loading

justjosias Dec 4, 2020 •

edited

Loading