Skip to content

Conversation

@herndlm
Copy link
Contributor

@herndlm herndlm commented Oct 7, 2022

This special handling for constant arrays should not be needed and the nested dim loop can break the outer type loop already if it found something to report ($report will not get any more true than true :)).

@herndlm
Copy link
Contributor Author

herndlm commented Oct 7, 2022

hmm it looks like this is reverting parts or all of the levels changes of the recently merged #1684. I was not entirely sure about this there and I'm not sure about it here..

oooh I think I know why now.. TypeUtils::map() basically aborts when it finds an unmatched type in https://github.com/phpstan/phpstan-src/blob/1.8.8/src/Type/TypeUtils.php#L212. The 2 new array type getters don't do that. So I guess this PR here should be fine, but that behaviour change is a bit dangerous

@herndlm herndlm force-pushed the simplify-nonexistent-offset-in-array-dim-fetch-check branch from 5b6e817 to cb7f705 Compare October 7, 2022 21:00
@herndlm
Copy link
Contributor Author

herndlm commented Oct 8, 2022

I'm leaning more and more to adding very specific methods to the two array types that can be used in the return type extensions for the array functions. E.g. filter(). I added some recently already anyways, like ConstantArrayType::pop() or so. I think we could get rid of most of the getArrays() and getConstantArrays() calls with all the conditional logic then and slim down the extensions but blow up the types. I can try it out maybe tomorrow with one if you're interested. And for rules like this one we shouldn't even need them preferably Maybe that's a good way to move forward. Not sure what to do with the two new methods then and if they really should be part of the API :)

Anyways, not much related to this PR which should be ready

@ondrejmirtes
Copy link
Member

I'm leaning more and more

Yes please :)

@ondrejmirtes ondrejmirtes merged commit 330209f into phpstan:1.9.x Oct 12, 2022
@ondrejmirtes
Copy link
Member

Thank you.

@herndlm herndlm deleted the simplify-nonexistent-offset-in-array-dim-fetch-check branch October 12, 2022 18:46
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.

2 participants