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

feat: conditional return types for Eloquent\Collection::find() #1419

Merged
merged 4 commits into from
Nov 4, 2022

Conversation

sebdesign
Copy link
Contributor

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

This was originally submitted here: laravel/framework#44459

Changes

Thanks to PHPStan's conditional return types, we can now infer what does the find method of the Eloquent Collection returns.
When the $key argument is an array or an Arrayable object, it returns a new collection with the found items.
In all other cases, it returns a single item, or the $default value.

@szepeviktor
Copy link
Collaborator

we can now infer what does the find method of the Eloquent Collection returns

Oh yes.
Thank you for this PR.

Copy link
Member

@canvural canvural left a comment

Choose a reason for hiding this comment

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

Thank you.

But also can you add tests to this file? (of course not for the support collection case) So we'll make sure it also works fine with the custom eloquent collections. If the tests doesn't pass, you might need to add find to the array here.

@sebdesign
Copy link
Contributor Author

Hey @canvural, I've tested the return type for custom Eloquent collections.

Instead of adding the method name on the list for the base collection class, we need to check the method name on the eloquent collection class, because that's where the method is declared.

@canvural
Copy link
Member

Thank you! Can you also resolve the conflict? Then if the CI is green, we can merge!

Sébastien Nikolaou and others added 2 commits October 29, 2022 13:27

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@canvural canvural changed the title Conditional return types for Eloquent\Collection::find() feat: conditional return types for Eloquent\Collection::find() Nov 4, 2022
@canvural canvural merged commit 333e791 into larastan:master Nov 4, 2022
@canvural
Copy link
Member

canvural commented Nov 4, 2022

Thank you!

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.

None yet

3 participants