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

len_without_is_empty improvements #6980

Merged
merged 1 commit into from
Mar 27, 2021

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Mar 26, 2021

fixes: #6958
fixes: #6972

changelog: Check the return type of len. Only integral types, or an Option or Result wrapping one.
changelog: Ensure the return type of is_empty matches. e.g. Option<usize> -> Option<bool>

@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 Mar 26, 2021
}

impl LenOutput<'_> {
fn matches_is_empty_output(self, ty: Ty<'_>) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

That's a cool idea I hadn't even though of, to check if the output types match with either Option or Result. 👍

@llogiq
Copy link
Contributor

llogiq commented Mar 27, 2021

Just one possible improvement, otherwise this looks good to me.

Check the return type of `len`. Only integral types, or an `Option` or `Result` wrapping one.
Ensure the return type of `is_empty` matches. e.g. `Option<usize>` -> `Option<bool>`
@llogiq
Copy link
Contributor

llogiq commented Mar 27, 2021

Thank you! @bors r+

@bors
Copy link
Collaborator

bors commented Mar 27, 2021

📌 Commit 12985af has been approved by llogiq

@bors
Copy link
Collaborator

bors commented Mar 27, 2021

⌛ Testing commit 12985af with merge dcee00d...

@bors
Copy link
Collaborator

bors commented Mar 27, 2021

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

@bors bors merged commit dcee00d into rust-lang:master Mar 27, 2021
@Fishrock123
Copy link
Contributor

Fishrock123 commented Mar 27, 2021

Oh, thanks for fixing this quick. :)

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
5 participants