Skip to content

Use isArray instead of array supertype checks#1634

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

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

Conversation

@herndlm

@herndlm herndlm commented Aug 18, 2022

Copy link
Copy Markdown
Contributor

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

UPDATE: added another commit that does the same for isString

@herndlm

herndlm commented Aug 18, 2022

Copy link
Copy Markdown
Contributor Author

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

@staabm

staabm commented Aug 21, 2022

Copy link
Copy Markdown
Contributor

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
Comment thread src/Analyser/MutatingScope.php Outdated
@ondrejmirtes

Copy link
Copy Markdown
Member

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

@herndlm

herndlm commented Aug 24, 2022

Copy link
Copy Markdown
Contributor Author

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

herndlm commented Aug 25, 2022

Copy link
Copy Markdown
Contributor Author

@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

herndlm commented Aug 25, 2022

Copy link
Copy Markdown
Contributor Author

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

herndlm commented Aug 26, 2022

Copy link
Copy Markdown
Contributor Author

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

herndlm commented Aug 26, 2022

Copy link
Copy Markdown
Contributor Author

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