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

Don't trigger use_debug lint in Debug impl #5047

Merged
merged 2 commits into from
Mar 3, 2020
Merged

Conversation

flip1995
Copy link
Member

Fixes #5039

changelog: Don't trigger [use_debug] lint in Debug impl

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 14, 2020
.ident
.name
.as_str();
if trait_name == "Debug" {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also stop linting in user defined traits, named Debug. But I guess this is a FN, we can live with.

clippy_lints/src/write.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 force-pushed the use_debug branch 2 times, most recently from 2a3aa5e to b335e8d Compare February 2, 2020 17:04
Comment on lines 361 to 388
if !self.in_debug_impl && arg.format.ty == "?" {
// FIXME: modify rustc's fmt string parser to give us the current span
span_lint(cx, USE_DEBUG, parser.prev_span, "use of `Debug`-based formatting");
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change in this function, everything else is just indented once more.

@flip1995 flip1995 force-pushed the use_debug branch 3 times, most recently from 2b36bd4 to 8dd3e16 Compare February 6, 2020 18:27
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one nit (this PR is just waiting for review, I'm not a reviewer though.)

clippy_lints/src/write.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Feb 21, 2020

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

@flip1995 flip1995 force-pushed the use_debug branch 2 times, most recently from 8bba9d1 to 4987e65 Compare February 21, 2020 12:26
.expect("path has at least one segment")
.ident
.name;
if trait_name == sym!(Debug) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, to avoid the FN, couldn't we compare the full path here instead of just the last segment? i.e if it's core::fmt::Debug or std::fmt::Debug?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would need to turn it in a LatePassLint for that. I think we can live with this FN though, since it shouldn't be that common that someone defines a Debug trait

@phansch phansch 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 Feb 27, 2020
@bors
Copy link
Contributor

bors commented Mar 1, 2020

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

@phansch
Copy link
Member

phansch commented Mar 3, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Mar 3, 2020

📌 Commit a540b5c has been approved by phansch

@phansch phansch removed the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Mar 3, 2020
@bors
Copy link
Contributor

bors commented Mar 3, 2020

⌛ Testing commit a540b5c with merge f44181e...

@bors
Copy link
Contributor

bors commented Mar 3, 2020

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

@bors bors merged commit f44181e into rust-lang:master Mar 3, 2020
@flip1995 flip1995 deleted the use_debug branch March 3, 2020 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use_debug in Debug implementation
5 participants