Implement TypeSpecifierComparisonContext

[staabm]

implement a POC for idea from #3178 (comment)

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 d842380 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 #3178 and does not yet handle everything (MethodCall, StaticCall etc.).

[staabm]

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.

phpstan-src/src/Type/Php/CountFunctionTypeSpecifyingExtension.php

Lines 22 to 45 in 3233155

	public function isFunctionSupported(
	FunctionReflection $functionReflection,
	FuncCall $node,
	TypeSpecifierContext $context,
	): bool
	{
	return !$context->null()
	&& count($node->getArgs()) >= 1
	&& in_array($functionReflection->getName(), ['sizeof', 'count'], true);
	}
	public function specifyTypes(
	FunctionReflection $functionReflection,
	FuncCall $node,
	Scope $scope,
	TypeSpecifierContext $context,
	): SpecifiedTypes
	{
	if (!$scope->getType($node->getArgs()[0]->value)->isArray()->yes()) {
	return new SpecifiedTypes([], []);
	}
	return $this->typeSpecifier->create($node->getArgs()[0]->value, new NonEmptyArrayType(), $context, false, $scope);
	}

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

[ondrejmirtes]

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.

[staabm]

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:

phpstan-src/src/Type/Php/CountFunctionTypeSpecifyingExtension.php

Lines 22 to 31 in 3233155

	public function isFunctionSupported(
	FunctionReflection $functionReflection,
	FuncCall $node,
	TypeSpecifierContext $context,
	): bool
	{
	return !$context->null()
	&& count($node->getArgs()) >= 1
	&& in_array($functionReflection->getName(), ['sizeof', 'count'], true);
	}

[ondrejmirtes]

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.

[staabm]

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?

[ondrejmirtes]

In #3185 (comment) 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".

[staabm]

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?