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

needless_return suggest return unit type on void returns #8185

Closed
wants to merge 3 commits into from

Conversation

dswij
Copy link
Member

@dswij dswij commented Dec 28, 2021

closes #8177

previously, needless_return suggests an empty block {} to replace void return on match arms, this PR improve the suggestion by suggesting a unit instead.

changelog: needless_return suggests () instead of {} on match arms

Add test for nested match arms and update test to suggest replacing void
return with `()`
@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 28, 2021
@llogiq
Copy link
Contributor

llogiq commented Dec 28, 2021

I'm not completely happy with calling an empty tuple a "unit type" – you're returning a value, not a type. Please change to either "unit value", "unit expression" or "empty tuple".

Otherwise this looks good to merge.

@llogiq
Copy link
Contributor

llogiq commented Dec 28, 2021

Thank you! @bors r+ squash

@bors
Copy link
Contributor

bors commented Dec 28, 2021

📌 Commit 020998a has been approved by llogiq

@bors
Copy link
Contributor

bors commented Dec 28, 2021

⌛ Testing commit 020998a with merge 526fb6b...

bors added a commit that referenced this pull request Dec 28, 2021
`needless_return` suggest return unit type on void returns

closes #8177

previously, `needless_return` suggests an empty block `{}` to replace void `return` on match arms, this PR improve the suggestion by suggesting a unit instead.

changelog: `needless_return` suggests `()` instead of `{}` on match arms
@bors
Copy link
Contributor

bors commented Dec 28, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 526fb6b to master...

@llogiq
Copy link
Contributor

llogiq commented Dec 28, 2021

This was actually merged, but github UI seems to have some problems.

@xFrednet
Copy link
Member

It would be interesting to see what happens with a retry, but I think the best solution would be to just close it 😅

@llogiq
Copy link
Contributor

llogiq commented Dec 28, 2021

I did this on another PR, it was merged twice. So as it's merged I'm going to close this PR manually. Thanks @dswij and sorry for the inconvenience.

@llogiq llogiq closed this Dec 28, 2021
@dswij
Copy link
Member Author

dswij commented Dec 28, 2021

I did this on another PR, it was merged twice. So as it's merged I'm going to close this PR manually. Thanks @dswij and sorry for the inconvenience.

Nothing to be sorry about here, thanks for the review 🙂

@dswij dswij deleted the 8177 branch December 28, 2021 14:26
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.

needless_return suggests empty block instead of void return
5 participants