phpstan
Implement TypeSpecifierComparisonContext
implement a POC for idea from https://github.com/phpstan/phpstan-src/pull/3178#issuecomment-2184949350
goal is to allow type-specifying extensions to narrow types of comparisons even in non true/truethy/false/falsey context, but e.g. from comparison against constant values.
similar as https://github.com/phpstan/phpstan-src/commit/d842380a1f07014d045dd42d6ccd4204a85e688a hardcoded it for preg_match but this time with support for extensions.
we need it for https://github.com/composer/pcre/pull/24/files#diff-8f259a63a4ab66c0f035859cea17176b75144dfb4c81db9981e04b66168edc4cR32-R50
this is an alternative implementation for https://github.com/phpstan/phpstan-src/pull/3178 and does not yet handle everything (MethodCall, StaticCall etc.).
as is, this consitutues a BC break (as can be seen in all the errors of the CI pipeline).
problem is, that extension can exist which just check for !$context->null(), e.g. https://github.com/phpstan/phpstan-src/blob/323315554048a22e6c01d635d6c5bb68f10d5aa1/src/Type/Php/CountFunctionTypeSpecifyingExtension.php#L22-L45
opened the draft so we can see how to progress.
one way could be: introduce a new interface - e.g. ComparisonContextAwareExtension - *TypeSpecifyingExtensions can implement optionally, so we can call a different method - e.g. specifyComparisonTypes. that way we could also use regular arguments to said method, instead of adding a new TypeSpecifierComparisonContext class
IMO we should merge the other open PRs before diving into this one
What's the BC break? The way I thought about it, there should be no BC break, as long as the extension currently properly checks the context.
What's the BC break?
after this PR extensions like CountFunctionTypeSpecifyingExtension are now getting called specifyTypes method for the comparison context and they now specify the type in said cases.
before this PR this extension was only invoked for true/truethy/false/falsey context, but it it now specifing types for the comparison case as well.
this happens because of isFunctionSupported just checks whether it is NOT null:
https://github.com/phpstan/phpstan-src/blob/323315554048a22e6c01d635d6c5bb68f10d5aa1/src/Type/Php/CountFunctionTypeSpecifyingExtension.php#L22-L31
Oh I see, so returning false from null() really didn't help us here.
I agree we need a new interface: ComparisonAwareTypeSpecifyingExtension - we only need one.
When an extension implements this interface (that's supposed to always be combined with *TypeSpecifyingExtension like FunctionTypeSpecifyingExtension), we know we can call it from the new places in code and that basically TypeSpecifierContext::comparison() will return something else than null in this scenario.
what do you think about TypeSpecifierComparisonContext ? should I drop that class and turn all the properties this object currently holds into parameters to the new method ComparisonAwareTypeSpecifyingExtension will enforce?
In https://github.com/phpstan/phpstan-src/pull/3185#issuecomment-2188691569 I suggested new interface ComparisonAwareTypeSpecifyingExtension but I don't think this interface will contain any methods. It's purely for instanceof check and whether we can call it with "comparison TypeSpecifierContext".
the POC works now equally to the pre-existing impl. IMO pluggin the TypeSpecifierComparisonContext into the TypeSpecifierContext makes the PR more complicated than it needs to be.
since we need the ComparisonAwareTypeSpecifyingExtension anyway, I think it would simplify this PR, if the interface would define e.g. a specifyComparisonTypes(FunctionReflection $functionReflection, FuncCall $node, Scope $scope, TypeSpecifierComparisonContext $comparisonContext) method and leave the TypeSpecifierContext untouched.
In the current form with just public function specifyTypes(FunctionReflection $functionReflection, FuncCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes its also confusing that when in comparison-context, the extension developer needs to make sure that the 2 available TypeSpecifierContext instances ($context and $comparisonContext->getTypeSpecifierContext()) are not mixed up
wdyt?
a better version of this PR is implemented in https://github.com/phpstan/phpstan-src/pull/3881