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

Specify iterable type #1140

Merged
merged 3 commits into from
Jul 9, 2023
Merged

Specify iterable type #1140

merged 3 commits into from
Jul 9, 2023

Conversation

franmomu
Copy link
Contributor

This will help static analysis tools to understand the iterable type of Collection

This will help static analysis tools to understand the iterable type of `Collection`
@norkunas
Copy link
Member

norkunas commented May 3, 2023

What's missing here to merge? :)

@jbelien
Copy link
Member

jbelien commented May 5, 2023

What's missing here to merge? :)

I must admit I don't know that @template-extends and don't seem to find documentation about it.

Could someone give some documentation and context about what this PR actually does ?

@norkunas
Copy link
Member

norkunas commented May 9, 2023

In my understanding: IteratorAggregate can contain any element in the collection, but the geocoder stores the Location, so it extends and says to SA tools that this collection must store the Location implementing objects so we get the proper typehinting after calling iterator_to_array.

@franmomu
Copy link
Contributor Author

franmomu commented May 9, 2023

In my understanding: IteratorAggregate can contain any element in the collection, but the geocoder stores the Location, so it extends and says to SA tools that this collection must store the Location implementing objects so we get the proper typehinting after calling iterator_to_array.

yes, IteratorAggregate is defined as generic by Static Analysis tools, about generics there is this article: https://phpstan.org/blog/generics-in-php-using-phpdocs#class-level-generics

So what this PR does is to tell SA tools (and PHPStorm) that Collection contains Location objects.

Here it's an example using this:

https://phpstan.org/r/ba1d6a3e-ba61-42db-9dde-a14169708c8b

so when you are iterating it for example, you know that each item is a Location object and can warn you if you are calling a method that does't exist in Location in this case.

PS: It would be nice to add a phpstan or psalm to the repository, but that's another PR.

Copy link
Member

@jbelien jbelien 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 for the explanation!
I'll merge it and add PHPStan in the next few days.

@jbelien jbelien merged commit 2346d59 into geocoder-php:master Jul 9, 2023
154 checks passed
@jbelien
Copy link
Member

jbelien commented Jul 9, 2023

Working on adding PHPStan in #1193
I'll increase level bit by bit because there are 4783 errors for level 8. 😄

@franmomu franmomu deleted the patch-1 branch July 10, 2023 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants