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 isA class-string types for falsey context #1040

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Feb 27, 2022

The title is misleading already, sorry. Should be more like "Extract type determination logic from is_subclass_of and use it in is_a" :)

Closes phpstan/phpstan#6704

While I initially was working on another solution, the changes in #1039 were finished and I could just completely take them over here. Thank you @arnaud-lb!

is_a and is_subclass_of seem to be having only 2 differences

Because there is still phpstan/phpstan#6305 open, the second one is fine here. For the first one I just changed the default when taking over the changes.

@herndlm herndlm force-pushed the specify-is-a-class-string-types-for-falsey-context branch from 57b9406 to c33c748 Compare February 27, 2022 23:08
@herndlm
Copy link
Contributor Author

herndlm commented Feb 28, 2022

I'm gonna extract the common logic and re-use that

@ondrejmirtes ondrejmirtes force-pushed the specify-is-a-class-string-types-for-falsey-context branch from c33c748 to 7ff5dfd Compare February 28, 2022 08:01
@ondrejmirtes
Copy link
Member

Rebased it to see the CI results :)

@herndlm
Copy link
Contributor Author

herndlm commented Feb 28, 2022

Should I push a change extracting the type determination? I basically moved the TypeTraverse::map into IsAFunctionTypeSpecifyingHelper::determineType(Type $classType, bool $allowString). What do you think? Unsure about the name still..
The idea is to later add another boolean argument or so that helps fixing phpstan/phpstan#6305

I'm just worried that either one of those extensions receives fixes/changes but the other is not udpated :/

@ondrejmirtes
Copy link
Member

Yes, common logic extraction 👍

When fixing something in the common logic, regression tests for both is_a and is_subclass_of should still be added, it doesn't hurt.

@ondrejmirtes ondrejmirtes force-pushed the specify-is-a-class-string-types-for-falsey-context branch from 4fbf5a8 to 83a0028 Compare February 28, 2022 08:52
@ondrejmirtes
Copy link
Member

Please mark this as ready, we're good to go :)

@herndlm herndlm marked this pull request as ready for review February 28, 2022 09:29
@ondrejmirtes ondrejmirtes merged commit 6f9749a into phpstan:master Feb 28, 2022
@ondrejmirtes
Copy link
Member

Thank you!

@herndlm herndlm deleted the specify-is-a-class-string-types-for-falsey-context branch February 28, 2022 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants