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

Elide unit variables linted by let_unit and use () directly instead #12603

Merged
merged 1 commit into from Apr 1, 2024

Conversation

m-rph
Copy link
Contributor

@m-rph m-rph commented Mar 31, 2024

Situation: let_unit lints when an expression binds a unit (()) to a variable. In some cases this binding may be passed down to another function. Currently, the lint removes the binding without considering usage.

fixes: #12594

changelog: Suggestion Fix [let_unit]. Clippy will remove unit bindings and replace all their instances in the body with ().

@rustbot
Copy link
Collaborator

rustbot commented Mar 31, 2024

r? @Manishearth

rustbot has assigned @Manishearth.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 31, 2024
@m-rph m-rph marked this pull request as ready for review March 31, 2024 16:24
@y21
Copy link
Member

y21 commented Mar 31, 2024

🤔 would it be worth it to continue linting but suggest replacing every use of the binding with ()?

@m-rph
Copy link
Contributor Author

m-rph commented Mar 31, 2024

I think we could do that. It could look be a bit silly buuuuuuut why not? :D

@m-rph m-rph changed the title Do not lint let_unit bindings that are used Elide unit variables linted by let_unit and use () directly instead Apr 1, 2024
Situation: `let_unit` lints when an expression binds a unit (`()`)
to a variable. In some cases this binding may be passed down to
another function. Currently, the lint removes the binding without
considering usage.

Change: All usages of the elided variable are now replaced with `()`.

fixes: rust-lang#12594
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

nicely done!

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 1, 2024

📌 Commit eee4db9 has been approved by Manishearth

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Apr 1, 2024

⌛ Testing commit eee4db9 with merge 95c45be...

@bors
Copy link
Collaborator

bors commented Apr 1, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 95c45be to master...

1 similar comment
@bors
Copy link
Collaborator

bors commented Apr 1, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 95c45be to master...

@bors
Copy link
Collaborator

bors commented Apr 1, 2024

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

@bors bors merged commit 95c45be into rust-lang:master Apr 1, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clippy removes variable bindings even if it's later being used.
5 participants