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

Narrow type of Collection::last() when using Collection::isEmpty() #173

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Feb 9, 2021

Now that we have #172 , we should do the same for last().

@ruudk ruudk force-pushed the doctrine-collection-last branch from 815a523 to 239135a Compare February 9, 2021 17:43
@ruudk ruudk force-pushed the doctrine-collection-last branch from 239135a to 22165cf Compare February 9, 2021 17:47
@ondrejmirtes ondrejmirtes merged commit 815dd47 into phpstan:master Feb 9, 2021
@ondrejmirtes
Copy link
Member

Thank you :)

@ruudk ruudk deleted the doctrine-collection-last branch February 9, 2021 19:17
@ruudk
Copy link
Contributor Author

ruudk commented Feb 9, 2021

⚡ Would be great to have this tagged too 🙏 Thanks!

@ondrejmirtes
Copy link
Member

Tagged 👌

@VincentLanglet
Copy link
Contributor

Hi @ruudk, would is be possible to do the same with the count value ?

image

@ruudk
Copy link
Contributor Author

ruudk commented Mar 5, 2021

Not sure if that's possible TBH. /cc @ondrejmirtes

@ondrejmirtes
Copy link
Member

Just assert that last() is not null instead...

@VincentLanglet
Copy link
Contributor

Just assert that last() is not null instead...

I know, and that's what 'im doing.
But the same argument could have be use for isEmpty, and still this great improvement was made.

If I assert that the count is > 0, I could avoid to add another assert that last/first is not false.

Would it be implementable ?

@ondrejmirtes
Copy link
Member

Nope, the TypeSpecifyingExtension API doesn’t allow it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants