Skip to content

Use isArray instead of array supertype checks#1634

Merged
ondrejmirtes merged 2 commits intophpstan:1.8.xfrom
herndlm:is-array-cleanups
Aug 25, 2022
Merged

Use isArray instead of array supertype checks#1634
ondrejmirtes merged 2 commits intophpstan:1.8.xfrom
herndlm:is-array-cleanups

Conversation

@herndlm
Copy link
Copy Markdown
Contributor

@herndlm herndlm commented Aug 18, 2022

another small (got bigger over time..) refactor. I'll stop now.. :)

UPDATE: added another commit that does the same for isString

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Aug 18, 2022

I was not expecting that one additional failure. weird. needs some debugging..

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Aug 21, 2022

one more here

($stringType->isSuperTypeOf($leftType)->yes() && $stringType->isSuperTypeOf($rightType)->yes())

@herndlm herndlm force-pushed the is-array-cleanups branch 3 times, most recently from 1388797 to ded6e19 Compare August 22, 2022 15:43
@ondrejmirtes
Copy link
Copy Markdown
Member

Please ping me once you mark this as ready, I'm looking forward to merge this.

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Aug 24, 2022

I think in order to be able to continue here I'd need #1644 to avoid weird behaviour or exceptions

@herndlm herndlm force-pushed the is-array-cleanups branch from bf929da to dc183ee Compare August 25, 2022 14:50
@herndlm herndlm marked this pull request as ready for review August 25, 2022 15:09
@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Aug 25, 2022

@ondrejmirtes this is ready now. I could create isInteger next and do the same if you like that

@herndlm herndlm requested a review from ondrejmirtes August 25, 2022 15:09
@ondrejmirtes
Copy link
Copy Markdown
Member

Yeah, you could be the guy who spends the next 6 months painstakingly creating methods on Type interface for all the use cases where we currently use instanceof *Type and TypeUtils methods :) That would be really awesome if it sounds interesting to you :)

@ondrejmirtes ondrejmirtes merged commit b157177 into phpstan:1.8.x Aug 25, 2022
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

@herndlm herndlm deleted the is-array-cleanups branch August 25, 2022 16:53
@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Aug 25, 2022

Yeah, you could be the guy who spends the next 6 months painstakingly creating methods on Type interface for all the use cases where we currently use instanceof *Type and TypeUtils methods :) That would be really awesome if it sounds interesting to you :)

you need to work on your sales pitch :D
I don't want to promise much, but I'll definitely continue with some primitive type methods. Feel free to mention any methods you're interested in

@ondrejmirtes
Copy link
Copy Markdown
Member

It's not my fault there's "pain" in "painstakingly" 🤣

@ondrejmirtes
Copy link
Copy Markdown
Member

Another easy one should be to replace stuff like instanceof TypeWithClassName, instanceof ObjectType, TypeUtils::getDirectClassNames().

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Aug 26, 2022

AFAIK the problems with instanceof replacements are then that methods are not guaranteed to exist on the Type interface, right? E.g. it's not safe anymore to call getClassName() on the potential union :/

ah and I misread "painstakingly" with "painfully" yesterday I guess. Both are fitting terms I think :D

@ondrejmirtes
Copy link
Copy Markdown
Member

This is the problem I'm trying to solve - there needs to be methods for EVERYTHING directly on Type. It nicely solves issues when the type is UnionType etc.

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Aug 26, 2022

definitely, yeah. it would also surely help with some constantarray operations I think and then greatly simplify the code that currently needs conditionals to deal with them with the downside of blowing up the Type interface I guess. but for the array/constantarray methods there could be a trait that implements the array-specific methods once for all non-array types or so and then maybe it's not a big deal after all

@ondrejmirtes
Copy link
Copy Markdown
Member

What we can do once the transformation is complete is to remove all specific Template*Type implementations and have just one TemplateType (class!) that supports any bound.

We can also stop inheriting type classes from each other.

And I guess we could also have a general SubtractableType that supports any type.

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