[staabm]

with https://github.com/phpstan/phpstan-src/pull/1383/files#r887647098 I was running into this bug.

the problem is, that phpstan should dump the type string but reports bool|float|int|string for the dumpType call seen here:

$ php bin/phpstan analyze src/Analyser/TypeSpecifier.php
Note: Using configuration file C:\dvl\Workspace\phpstan-src-staabm\phpstan.neon.dist.
 1/1 [============================] 100%

 ------ ------------------------------------
  Line   TypeSpecifier.php
 ------ ------------------------------------
  274    Dumped type: bool|float|int|string
 ------ ------------------------------------

I am not yet sure how to debug this issue further. I also tried reproducing the case in a smaler repro script, but this did not work out yet.

any ideas/hints?

[herndlm]

I guess I'd try using xdebug here, in case you didn't do that already.

[staabm]

I tried, but wasn't sure where to look tbh

[staabm]

after a bit more trying, I was able to reduce the repro further

this PR now includes a proper NodeScopeResolverTest for the problem

[staabm]

it seems this nullcheck is somehow messing it up

maybe @rvanvelzen has an idea?

[herndlm]

That's interesting. Null isn't even a scalar, right? Is this ever triggered and is there even a ConstantType that implements returning null via getValue?
Might be unrelated to the bug though.. 😅 nevermind NullType implements this of course..

[herndlm]

Ah one more thing came to mind: could it be related to the fact that phpstan remembers what the method returned? Maybe this needs to he invalidated below in the if scope.
The bug might then be that method return types are falsely remembered / not invalidated, even if the class instance changes. Or smth like that

[staabm]

@rajyan do you have an idea on how to approach this problem? maybe you even have time for a fix?

[rajyan]

https://phpstan.org/r/a7200b8c-8482-4fd2-b782-ecdb8052a28a

This bug looks very interesting, the commented out line changes the result. Rough guess, the type specification with instanceof is not working or moreSpecificType $var->getValue() remembered by !== null is breaking something.
I don’t have time this week (and this weekend) but I think I can fix it next week.

[rajyan]

Another guess is that the moreSpecificType is not correctly invalidated correctly on type specification by instanceof.

[rvanvelzen]

Check this strange behavior as well:

• https://phpstan.org/r/889e3c66-972b-4ace-af00-e9beb3280918 (with dumpType($constantType))
• https://phpstan.org/r/1e85e658-425f-44ef-afeb-7243c3df5221 (without dumpType($constantType))

[staabm]

@rvanvelzen @rajyan might one of you guys has time to look into this problem?

[rvanvelzen]

I've looked into this recently but it didn't look trivial to solve. I'm a bit swamped at work right now but I'll look into it more when that settles down again :)

[rajyan]

@staabm
This was harder to solve than I thought.
You can see #1495
when $var is specified, you might narrow down $var->call() more.
In this case, $var->call() is specified as non-null at first, and the next instanceof ConstantString type can specify $var->call() more. Currently, these types of type specification is not handled.

[staabm]

after working thru this I implemented several different solutions, which made the test pass, but regressed other things.

by doing this I got a better understanding how things work and how a possible solution could look like.

atm I think we need to adjust this block:

phpstan-src/src/Analyser/MutatingScope.php

Lines 3551 to 3579 in b0babd0

	foreach ($moreSpecificTypeHolders as $specifiedExprString => $specificTypeHolder) {
	if (!$specificTypeHolder->getCertainty()->yes()) {
	continue;
	}
	$specifiedExprString = (string) $specifiedExprString;
	$specifiedExpr = $this->exprStringToExpr($specifiedExprString);
	if ($specifiedExpr === null) {
	continue;
	}
	if (!$specifiedExpr instanceof Expr\ArrayDimFetch) {
	continue;
	}
	if (!$specifiedExpr->dim instanceof Variable) {
	continue;
	}
	if (!is_string($specifiedExpr->dim->name)) {
	continue;
	}
	if ($specifiedExpr->dim->name !== $variableName) {
	continue;
	}
	$moreSpecificTypeHolders[$specifiedExprString] = VariableTypeHolder::createYes($this->getType($specifiedExpr->var)->getOffsetValueType($type));
	unset($nativeTypes[$specifiedExprString]);
	}

.. so it takes into account when a variable get specified to a new ObjectType - e.g. via $constantType instanceof ConstantStringType.
We need to update all moreSpecificTypeHolders which depend on $constantType , e.g. $constantType->getValue() and merge the existing type of $constantType->getValue() with the new knowledge that $var was specified to ConstantStringType and therefore we have a better idea of $constantType->getValue().

class HelloWorld
{
	public function doFoo(
		ConstantScalarType $constantType
	)
	{
		if ($constantType->getValue() === null) {
			return;
		}
		assertType('bool|float|int|string', $constantType->getValue());

		if (
			$constantType instanceof ConstantStringType
		) {
			assertType('string', $constantType->getValue());
		}
	}
}

in the end I think this is a similar thing, this line achieves for array-offset-types:

phpstan-src/src/Analyser/MutatingScope.php

Line 3577 in b0babd0

$moreSpecificTypeHolders[$specifiedExprString] = VariableTypeHolder::createYes($this->getType($specifiedExpr->var)->getOffsetValueType($type));

[staabm]

finally, this one is ready for review.

[ondrejmirtes]

Can you please explain what the bug is and why this change fixes it? It seems too specific and too hardcoded for this specific test case...

[staabm]

I tried explaining it here: #1396 (comment)

[staabm]

it boils down to

• phpstan knows a moreSpecificType for a method call $constant->getValue(), based on $constant beeing of type ConstantScalarType
• after that the $constant object-type get narrowed to ConstantStringType via instanceof
• phpstans already known moreSpecificType is not changed, and therefore phpstan doesn't realize that $constant->getValue() can only return string after the instanceof ConstantStringType case

[staabm]

the fix should unblock

• infer type on strict comparison with get_class() #1616
• Specify types on get_parent_class() #1383

[ondrejmirtes]

Is this still a problem?

[staabm]

The underlying bug should still be present, but in phpstan-src I worked arround it in a related PR, so the phpstan-src codebase is no longer blocked by it right now

[ondrejmirtes]

It'd be better to reproduce this on phpstan.org/try and open an issue.