Skip to content

Recurse into generic objects parameters#306

Merged
ondrejmirtes merged 4 commits intophpstan:masterfrom
pmmp:recurse-generic-objects
Aug 30, 2020
Merged

Recurse into generic objects parameters#306
ondrejmirtes merged 4 commits intophpstan:masterfrom
pmmp:recurse-generic-objects

Conversation

@dktapps
Copy link
Contributor

@dktapps dktapps commented Aug 25, 2020

This fixes phpstan/phpstan#3767: types like GenericType<OtherGenericType> with invalid nested generic parameters not being analysed and reported.

I'm aware that this breaks some iterable special-cases such as intersection of iterable&Traversable. I'm not entirely sure how to address this, so I could use some advice.

I also don't know if I missed any cases, although I don't think I did.

I'm also not sure how this should be tested. There doesn't seem to be any test unit dedicated to MissingTypehintCheck.

this breaks some iterable special-cases such as intersection of iterable<int>&Traversable. Needs to be investigated.
@ondrejmirtes
Copy link
Member

Look at from which places are the Check classes called - these rules have unit tests.

And I guess the $traverse($type); inside $type->isIterable()->yes() shouldn't be added.

I'm not sure this is right and gut feeling tells me it will cause some issues not to be reported, but let's see if this flies.
@dktapps
Copy link
Contributor Author

dktapps commented Aug 25, 2020

Look at from which places are the Check classes called - these rules have unit tests.

Do you want me to repeat the test cases 5 times (one for each of the places MissingTypehintCheck is used)? I hoped there was a better way :<

@ondrejmirtes
Copy link
Member

@dktapps It's fine to test it in a single rule.

@ondrejmirtes ondrejmirtes merged commit c8a9b2f into phpstan:master Aug 30, 2020
@ondrejmirtes
Copy link
Member

Thank you!

@dktapps
Copy link
Contributor Author

dktapps commented Aug 30, 2020

Thanks. Also, the original post referenced the wrong issue, I meant to link this one: phpstan/phpstan#3764

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.

Redl

2 participants