Skip to content

Add print_in_format_impl lint #8253

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 25, 2022
Merged

Add print_in_format_impl lint #8253

merged 2 commits into from
Feb 25, 2022

Conversation

@Alexendoo
Copy link
Member

@Alexendoo Alexendoo commented Jan 9, 2022 •
edited
Loading

changelog: new lint: [print_in_format_impl]

Lints the use of print-like macros in manual Display/Debug impls. I feel like I make this mistake every time I write one 😄

r? @camsteffen

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 9, 2022
@camsteffen camsteffen mentioned this pull request Jan 11, 2022
Merged
@bors
Copy link
Contributor

bors commented Jan 11, 2022

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

@bors
Copy link
Contributor

bors commented Jan 17, 2022

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

@camsteffen
Copy link
Contributor

camsteffen commented Jan 17, 2022 •
edited
Loading

@Alexendoo I am thinking we'll let #8188 be merged first, and then this lint may use the same LintPass struct. Also this lint should be named print_in_format_traitprint_in_format_impl for consistency.

@jamesmcm
Copy link
Contributor

jamesmcm commented Jan 17, 2022

I just renamed the other one, so maybe print_in_format is okay?

@camsteffen
Copy link
Contributor

camsteffen commented Jan 17, 2022

Oh whoops I meant to say print_in_format_impl.

@Alexendoo
Copy link
Member Author

Alexendoo commented Jan 17, 2022

Sounds good to me, I'll rework this on top of the other when it's merged

@Alexendoo Alexendoo changed the title Add print_in_fmt lint Add print_in_format_impl lint Feb 17, 2022
@Alexendoo
Copy link
Member Author

Alexendoo commented Feb 17, 2022

Reworked it on top of #8188

@Alexendoo
Copy link
Member Author

Alexendoo commented Feb 25, 2022

Split the diagnostic item change out into #8474

camsteffen
camsteffen reviewed Feb 25, 2022
View reviewed changes
Copy link
Contributor

@camsteffen camsteffen left a comment

Choose a reason for hiding this comment

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

Just a nit on the docs. Otherwise looks great!

@camsteffen
Copy link
Contributor

camsteffen commented Feb 25, 2022

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Feb 25, 2022

📌 Commit 52f3d61 has been approved by camsteffen

@bors
Copy link
Contributor

bors commented Feb 25, 2022

⌛ Testing commit 52f3d61 with merge 8bba6b7...

@bors
Copy link
Contributor

bors commented Feb 25, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: camsteffen
Pushing 8bba6b7 to master...

@bors bors merged commit 8bba6b7 into rust-lang:master Feb 25, 2022
@Alexendoo Alexendoo deleted the print-in-fmt branch February 25, 2022 21:31
@unvalley unvalley mentioned this pull request Jul 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@camsteffen camsteffen camsteffen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@camsteffen camsteffen

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Alexendoo @bors @camsteffen @jamesmcm @rust-highfive