Failing test for #4657

[ruudk]

See phpstan/phpstan#4657

While using the debugger I found that the problem occurs while resolving is_null($value).

This code:

phpstan-src/src/Rules/Comparison/ImpossibleCheckTypeHelper.php

Lines 177 to 186 in 6d52302

	$sureType = reset($sureTypes);
	if ($isSpecified($sureType[0])) {
	return null;
	}
	if ($this->treatPhpDocTypesAsCertain) {
	$argumentType = $scope->getType($sureType[0]);
	} else {
	$argumentType = $scope->getNativeType($sureType[0]);
	}

gets $sureTypes = [ Node\Expr\Variable , Type\NullType ].

When treatPhpDocTypesAsCertain = false, it will call getNativeType on the variable. That resolves to null.

It's probably because the $value = null; declaration on the top.

It does not know that the value is also passed as reference to a callback.

Any advice how to debug this further?

[ruudk]

Interesting: When I remove this:

phpstan-src/src/Rules/Comparison/ImpossibleCheckTypeHelper.php

Lines 182 to 186 in 6d52302

	if ($this->treatPhpDocTypesAsCertain) {
	$argumentType = $scope->getType($sureType[0]);
	} else {
	$argumentType = $scope->getNativeType($sureType[0]);
	}

with
$argumentType = $scope->getType($sureType[0]);

my test passes, but a lot other tests fail (obviously).

I wonder, my bug file does not contain any PHPDoc, so why does getNativeType give other results than getType?
https://github.com/phpstan/phpstan-src/blob/7c232be3b8bafda3f0be8bf1f791f79cbe03ebc1/tests/PHPStan/Rules/Comparison/data/bug-4657.php

[ondrejmirtes]

@ruudk That's about wrong information being in MutatingScope::$nativeExpressionTypes.

This is all the code that's related to processing closures:

• phpstan-src/src/Analyser/NodeScopeResolver.php

  Lines 2308 to 2428 in 2b30830

  	private function processClosureNode(
  	Expr\Closure $expr,
  	MutatingScope $scope,
  	callable $nodeCallback,
  	ExpressionContext $context,
  	?Type $passedToType
  	): ExpressionResult
  	{
  	foreach ($expr->params as $param) {
  	$this->processParamNode($param, $scope, $nodeCallback);
  	}
  	$byRefUses = [];
  	if ($passedToType !== null && !$passedToType->isCallable()->no()) {
  	$callableParameters = null;
  	$acceptors = $passedToType->getCallableParametersAcceptors($scope);
  	if (count($acceptors) === 1) {
  	$callableParameters = $acceptors[0]->getParameters();
  	}
  	} else {
  	$callableParameters = null;
  	}
  	$useScope = $scope;
  	foreach ($expr->uses as $use) {
  	if ($use->byRef) {
  	$byRefUses[] = $use;
  	$useScope = $useScope->enterExpressionAssign($use->var);
  	$inAssignRightSideVariableName = $context->getInAssignRightSideVariableName();
  	$inAssignRightSideType = $context->getInAssignRightSideType();
  	if (
  	$inAssignRightSideVariableName === $use->var->name
  	&& $inAssignRightSideType !== null
  	) {
  	if ($inAssignRightSideType instanceof ClosureType) {
  	$variableType = $inAssignRightSideType;
  	} else {
  	$alreadyHasVariableType = $scope->hasVariableType($inAssignRightSideVariableName);
  	if ($alreadyHasVariableType->no()) {
  	$variableType = TypeCombinator::union(new NullType(), $inAssignRightSideType);
  	} else {
  	$variableType = TypeCombinator::union($scope->getVariableType($inAssignRightSideVariableName), $inAssignRightSideType);
  	}
  	}
  	$scope = $scope->assignVariable($inAssignRightSideVariableName, $variableType);
  	}
  	}
  	$this->processExprNode($use, $useScope, $nodeCallback, $context);
  	if (!$use->byRef) {
  	continue;
  	}
  	$useScope = $useScope->exitExpressionAssign($use->var);
  	}
  	if ($expr->returnType !== null) {
  	$nodeCallback($expr->returnType, $scope);
  	}
  	$closureScope = $scope->enterAnonymousFunction($expr, $callableParameters);
  	$closureScope = $closureScope->processClosureScope($scope, null, $byRefUses);
  	$nodeCallback(new InClosureNode($expr), $closureScope);
  	$gatheredReturnStatements = [];
  	$gatheredYieldStatements = [];
  	$closureStmtsCallback = static function (\PhpParser\Node $node, Scope $scope) use ($nodeCallback, &$gatheredReturnStatements, &$gatheredYieldStatements, &$closureScope): void {
  	$nodeCallback($node, $scope);
  	if ($scope->getAnonymousFunctionReflection() !== $closureScope->getAnonymousFunctionReflection()) {
  	return;
  	}
  	if ($node instanceof Expr\Yield_ || $node instanceof Expr\YieldFrom) {
  	$gatheredYieldStatements[] = $node;
  	}
  	if (!$node instanceof Return_) {
  	return;
  	}
  	$gatheredReturnStatements[] = new ReturnStatement($scope, $node);
  	};
  	if (count($byRefUses) === 0) {
  	$statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback);
  	$nodeCallback(new ClosureReturnStatementsNode(
  	$expr,
  	$gatheredReturnStatements,
  	$gatheredYieldStatements,
  	$statementResult
  	), $closureScope);
  	return new ExpressionResult($scope, false);
  	}
  	$count = 0;
  	do {
  	$prevScope = $closureScope;
  	$intermediaryClosureScopeResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, static function (): void {
  	});
  	$intermediaryClosureScope = $intermediaryClosureScopeResult->getScope();
  	foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) {
  	$intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope());
  	}
  	$closureScope = $scope->enterAnonymousFunction($expr, $callableParameters);
  	$closureScope = $closureScope->processClosureScope($intermediaryClosureScope, $prevScope, $byRefUses);
  	if ($closureScope->equals($prevScope)) {
  	break;
  	}
  	$count++;
  	} while ($count < self::LOOP_SCOPE_ITERATIONS);
  	$statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback);
  	$nodeCallback(new ClosureReturnStatementsNode(
  	$expr,
  	$gatheredReturnStatements,
  	$gatheredYieldStatements,
  	$statementResult
  	), $closureScope);
  	return new ExpressionResult($scope->processClosureScope($closureScope, null, $byRefUses), false);
  	}

• phpstan-src/src/Analyser/MutatingScope.php

  Lines 2758 to 2881 in 2b30830

  	public function enterAnonymousFunction(
  	Expr\Closure $closure,
  	?array $callableParameters = null
  	): self
  	{
  	$anonymousFunctionReflection = $this->getType($closure);
  	if (!$anonymousFunctionReflection instanceof ClosureType) {
  	throw new \PHPStan\ShouldNotHappenException();
  	}
  	$scope = $this->enterAnonymousFunctionWithoutReflection($closure, $callableParameters);
  	return $this->scopeFactory->create(
  	$scope->context,
  	$scope->isDeclareStrictTypes(),
  	$scope->constantTypes,
  	$scope->getFunction(),
  	$scope->getNamespace(),
  	$scope->variableTypes,
  	$scope->moreSpecificTypes,
  	[],
  	$scope->inClosureBindScopeClass,
  	$anonymousFunctionReflection,
  	true,
  	[],
  	$scope->nativeExpressionTypes,
  	[],
  	false,
  	$this
  	);
  	}
  	/**
  	* @param \PhpParser\Node\Expr\Closure $closure
  	* @param \PHPStan\Reflection\ParameterReflection[]|null $callableParameters
  	* @return self
  	*/
  	private function enterAnonymousFunctionWithoutReflection(
  	Expr\Closure $closure,
  	?array $callableParameters = null
  	): self
  	{
  	$variableTypes = [];
  	foreach ($closure->params as $i => $parameter) {
  	$isNullable = $this->isParameterValueNullable($parameter);
  	$parameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
  	if ($callableParameters !== null) {
  	if (isset($callableParameters[$i])) {
  	$parameterType = TypehintHelper::decideType($parameterType, $callableParameters[$i]->getType());
  	} elseif (count($callableParameters) > 0) {
  	$lastParameter = $callableParameters[count($callableParameters) - 1];
  	if ($lastParameter->isVariadic()) {
  	$parameterType = TypehintHelper::decideType($parameterType, $lastParameter->getType());
  	} else {
  	$parameterType = TypehintHelper::decideType($parameterType, new MixedType());
  	}
  	} else {
  	$parameterType = TypehintHelper::decideType($parameterType, new MixedType());
  	}
  	}
  	if (!$parameter->var instanceof Variable || !is_string($parameter->var->name)) {
  	throw new \PHPStan\ShouldNotHappenException();
  	}
  	$variableTypes[$parameter->var->name] = VariableTypeHolder::createYes(
  	$parameterType
  	);
  	}
  	$nativeTypes = [];
  	$moreSpecificTypes = [];
  	foreach ($closure->uses as $use) {
  	if (!is_string($use->var->name)) {
  	throw new \PHPStan\ShouldNotHappenException();
  	}
  	if ($use->byRef) {
  	continue;
  	}
  	$variableName = $use->var->name;
  	if ($this->hasVariableType($variableName)->no()) {
  	$variableType = new ErrorType();
  	} else {
  	$variableType = $this->getVariableType($variableName);
  	$nativeTypes[sprintf('$%s', $variableName)] = $this->getNativeType($use->var);
  	}
  	$variableTypes[$variableName] = VariableTypeHolder::createYes($variableType);
  	foreach ($this->moreSpecificTypes as $exprString => $moreSpecificType) {
  	$matches = \Nette\Utils\Strings::matchAll((string) $exprString, '#^\$([a-zA-Z_\x7f-\xff][a-zA-Z_0-9\x7f-\xff]*)#');
  	if ($matches === []) {
  	continue;
  	}
  	$matches = array_column($matches, 1);
  	if (!in_array($variableName, $matches, true)) {
  	continue;
  	}
  	$moreSpecificTypes[$exprString] = $moreSpecificType;
  	}
  	}
  	if ($this->hasVariableType('this')->yes() && !$closure->static) {
  	$variableTypes['this'] = VariableTypeHolder::createYes($this->getVariableType('this'));
  	}
  	return $this->scopeFactory->create(
  	$this->context,
  	$this->isDeclareStrictTypes(),
  	$this->constantTypes,
  	$this->getFunction(),
  	$this->getNamespace(),
  	$variableTypes,
  	$moreSpecificTypes,
  	[],
  	$this->inClosureBindScopeClass,
  	new TrivialParametersAcceptor(),
  	true,
  	[],
  	$nativeTypes,
  	[],
  	false,
  	$this
  	);
  	}

• phpstan-src/src/Analyser/MutatingScope.php

  Lines 3997 to 4051 in 2b30830

  	public function processClosureScope(
  	self $closureScope,
  	?self $prevScope,
  	array $byRefUses
  	): self
  	{
  	$variableTypes = $this->variableTypes;
  	if (count($byRefUses) === 0) {
  	return $this;
  	}
  	foreach ($byRefUses as $use) {
  	if (!is_string($use->var->name)) {
  	throw new \PHPStan\ShouldNotHappenException();
  	}
  	$variableName = $use->var->name;
  	if (!$closureScope->hasVariableType($variableName)->yes()) {
  	$variableTypes[$variableName] = VariableTypeHolder::createYes(new NullType());
  	continue;
  	}
  	$variableType = $closureScope->getVariableType($variableName);
  	if ($prevScope !== null) {
  	$prevVariableType = $prevScope->getVariableType($variableName);
  	if (!$variableType->equals($prevVariableType)) {
  	$variableType = TypeCombinator::union($variableType, $prevVariableType);
  	$variableType = self::generalizeType($variableType, $prevVariableType);
  	}
  	}
  	$variableTypes[$variableName] = VariableTypeHolder::createYes($variableType);
  	}
  	return $this->scopeFactory->create(
  	$this->context,
  	$this->isDeclareStrictTypes(),
  	$this->constantTypes,
  	$this->getFunction(),
  	$this->getNamespace(),
  	$variableTypes,
  	$this->moreSpecificTypes,
  	$this->conditionalExpressions,
  	$this->inClosureBindScopeClass,
  	$this->anonymousFunctionReflection,
  	$this->inFirstLevelStatement,
  	[],
  	$this->nativeExpressionTypes,
  	$this->inFunctionCallsStack,
  	$this->afterExtractCall,
  	$this->parentScope
  	);
  	}

[ruudk]

@ondrejmirtes Thanks for pointing this out. I believe I solved it. Is it the way to go?

[ondrejmirtes]

Yeah, looks fine, just do all the necessary assertNativeType() tests to verify it :)

[ruudk]

Added them to my test file, are there more places where I should add it?

[ondrejmirtes]

@ruudk If you write some nonsense there you'll realized those asserts aren't executed. You need to add the file as a dataProvider in NodeScopeResolver::testFileAsserts().

[ondrejmirtes]

Thank you!