Skip to content

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>`
@Jarcho Jarcho force-pushed the len_without_is_empty_sig branch from 88bf10e to 12985af Compare March 27, 2021 13:05
@llogiq
Copy link
Contributor

llogiq commented Mar 27, 2021

Thank you! @bors r+

@bors
Copy link
Contributor

bors commented Mar 27, 2021

📌 Commit 12985af has been approved by llogiq

@bors
Copy link
Contributor

bors commented Mar 27, 2021

⌛ Testing commit 12985af with merge dcee00d...

@bors
Copy link
Contributor

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