From a7fed03bbf1bef545c8afcbf6c906ac93b34c876 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 17 Dec 2022 13:30:04 +0100 Subject: [PATCH] Fix readonly properties bugs and infinite recursion --- src/Analyser/MutatingScope.php | 27 +++++++++------- .../Analyser/NodeScopeResolverTest.php | 5 +++ tests/PHPStan/Analyser/data/bug-8543.php | 31 +++++++++++++++++++ .../data/dependent-expression-certainty.php | 2 +- .../data/dependent-variable-certainty.php | 2 +- 5 files changed, 54 insertions(+), 13 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-8543.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 619be8020b..eae844279c 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3465,7 +3465,7 @@ public function assignExpression(Expr $expr, Type $type, ?Type $nativeType = nul } elseif ($expr instanceof Expr\StaticPropertyFetch) { $scope = $this->invalidateExpression($expr); } elseif ($expr instanceof Variable) { - $scope = $this->invalidateExpression($expr, true); + $scope = $this->invalidateExpression($expr); } return $scope->specifyExpressionType($expr, $type, $nativeType); @@ -3538,15 +3538,6 @@ private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr if ($requireMoreCharacters && $exprStringToInvalidate === $this->getNodeKey($expr)) { return false; } - if ($expr instanceof PropertyFetch) { - $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); - if ($propertyReflection !== null) { - $nativePropertyReflection = $propertyReflection->getNativeReflection(); - if ($nativePropertyReflection !== null && $nativePropertyReflection->isReadOnly()) { - return false; - } - } - } $nodeFinder = new NodeFinder(); $expressionToInvalidateClass = get_class($exprToInvalidate); @@ -3560,7 +3551,21 @@ private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr return $nodeString === $exprStringToInvalidate; }); - return $found !== null; + if ($found === null) { + return false; + } + + if ($this->phpVersion->supportsReadOnlyProperties() && $expr instanceof PropertyFetch && $expr->name instanceof Node\Identifier && $requireMoreCharacters) { + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); + if ($propertyReflection !== null) { + $nativePropertyReflection = $propertyReflection->getNativeReflection(); + if ($nativePropertyReflection !== null && $nativePropertyReflection->isReadOnly()) { + return false; + } + } + } + + return true; } private function invalidateMethodsOnExpression(Expr $expressionToInvalidate): self diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index 0e8198e765..67e916c0b6 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -1142,6 +1142,11 @@ public function dataFileAsserts(): iterable yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-82.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4565.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3789.php'); + + if (PHP_VERSION_ID >= 80100) { + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8543.php'); + } + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8520.php'); } diff --git a/tests/PHPStan/Analyser/data/bug-8543.php b/tests/PHPStan/Analyser/data/bug-8543.php new file mode 100644 index 0000000000..01d7b93c45 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-8543.php @@ -0,0 +1,31 @@ += 8.1 + +namespace Bug8543; + +use function PHPStan\Testing\assertType; + +class HelloWorld +{ + public readonly int $i; + + public int $j; + + public function invalidate(): void + { + } +} + +function (HelloWorld $hw): void { + $hw->i = 1; + $hw->j = 2; + assertType('1', $hw->i); + assertType('2', $hw->j); + + $hw->invalidate(); + assertType('1', $hw->i); + assertType('int', $hw->j); + + $hw = new HelloWorld(); + assertType('int', $hw->i); + assertType('int', $hw->j); +}; diff --git a/tests/PHPStan/Analyser/data/dependent-expression-certainty.php b/tests/PHPStan/Analyser/data/dependent-expression-certainty.php index d8596c0a21..302d82b609 100644 --- a/tests/PHPStan/Analyser/data/dependent-expression-certainty.php +++ b/tests/PHPStan/Analyser/data/dependent-expression-certainty.php @@ -153,7 +153,7 @@ function (bool $a, bool $b) { assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); if (returnsBool($b)) { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); + assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); // could be Yes } if (returnsBool($a)) { diff --git a/tests/PHPStan/Analyser/data/dependent-variable-certainty.php b/tests/PHPStan/Analyser/data/dependent-variable-certainty.php index 8e34c91b91..f84857da66 100644 --- a/tests/PHPStan/Analyser/data/dependent-variable-certainty.php +++ b/tests/PHPStan/Analyser/data/dependent-variable-certainty.php @@ -149,7 +149,7 @@ function (bool $a, bool $b) { assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); if ($b) { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); + assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); // could be Yes } if ($a) {