diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8c070bb040..18ea0112e9 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5392,6 +5392,7 @@ private function processAssignVar( } } + $scopeBeforeAssignEval = $scope; $scope = $result->getScope(); $truthySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createTruthy()); $falseySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createFalsey()); @@ -5404,7 +5405,7 @@ private function processAssignVar( $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType); $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType); - $nodeCallback(new VariableAssignNode($var, $assignedExpr), $result->getScope()); + $nodeCallback(new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval); $scope = $scope->assignVariable($var->name, $type, $scope->getNativeType($assignedExpr), TrinaryLogic::createYes()); foreach ($conditionalExpressions as $exprString => $holders) { $scope = $scope->addConditionalExpressions($exprString, $holders); @@ -5487,6 +5488,7 @@ private function processAssignVar( $nativeValueToWrite = $scope->getNativeType($assignedExpr); $originalValueToWrite = $valueToWrite; $originalNativeValueToWrite = $nativeValueToWrite; + $scopeBeforeAssignEval = $scope; // 3. eval assigned expr $result = $processExprCallback($scope); @@ -5542,11 +5544,11 @@ private function processAssignVar( if ($varType->isArray()->yes() || !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->yes()) { if ($var instanceof Variable && is_string($var->name)) { - $nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr), $scope); + $nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr), $scopeBeforeAssignEval); $scope = $scope->assignVariable($var->name, $valueToWrite, $nativeValueToWrite, TrinaryLogic::createYes()); } else { if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) { - $nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope); + $nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scopeBeforeAssignEval); if ($var instanceof PropertyFetch && $var->name instanceof Node\Identifier && !$isAssignOp) { $scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString()); } @@ -5574,9 +5576,9 @@ private function processAssignVar( } } else { if ($var instanceof Variable) { - $nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr), $scope); + $nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr), $scopeBeforeAssignEval); } elseif ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) { - $nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope); + $nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scopeBeforeAssignEval); if ($var instanceof PropertyFetch && $var->name instanceof Node\Identifier && !$isAssignOp) { $scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString()); } @@ -5611,6 +5613,7 @@ static function (): void { $scope = $propertyNameResult->getScope(); } + $scopeBeforeAssignEval = $scope; $result = $processExprCallback($scope); $hasYield = $hasYield || $result->hasYield(); $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); @@ -5625,7 +5628,7 @@ static function (): void { if ($propertyName !== null && $propertyHolderType->hasProperty($propertyName)->yes()) { $propertyReflection = $propertyHolderType->getProperty($propertyName, $scope); $assignedExprType = $scope->getType($assignedExpr); - $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scope); + $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval); if ($propertyReflection->canChangeTypeAfterAssignment()) { if ($propertyReflection->hasNativeType()) { $propertyNativeType = $propertyReflection->getNativeType(); @@ -5671,7 +5674,7 @@ static function (): void { } else { // fallback $assignedExprType = $scope->getType($assignedExpr); - $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scope); + $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval); $scope = $scope->assignExpression($var, $assignedExprType, $scope->getNativeType($assignedExpr)); // simulate dynamic property assign by __set to get throw points if (!$propertyHolderType->hasMethod('__set')->no()) { @@ -5705,6 +5708,7 @@ static function (): void { $scope = $propertyNameResult->getScope(); } + $scopeBeforeAssignEval = $scope; $result = $processExprCallback($scope); $hasYield = $hasYield || $result->hasYield(); $throwPoints = array_merge($throwPoints, $result->getThrowPoints()); @@ -5714,7 +5718,7 @@ static function (): void { if ($propertyName !== null) { $propertyReflection = $scope->getPropertyReflection($propertyHolderType, $propertyName); $assignedExprType = $scope->getType($assignedExpr); - $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scope); + $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval); if ($propertyReflection !== null && $propertyReflection->canChangeTypeAfterAssignment()) { if ($propertyReflection->hasNativeType()) { $propertyNativeType = $propertyReflection->getNativeType(); @@ -5745,7 +5749,7 @@ static function (): void { } else { // fallback $assignedExprType = $scope->getType($assignedExpr); - $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scope); + $nodeCallback(new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval); $scope = $scope->assignExpression($var, $assignedExprType, $scope->getNativeType($assignedExpr)); } } elseif ($var instanceof List_) { diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index 8d050e7636..4260da643c 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -15,9 +15,11 @@ class TypesAssignedToPropertiesRuleTest extends RuleTestCase private bool $checkExplicitMixed = false; + private bool $checkImplicitMixed = false; + protected function getRule(): Rule { - return new TypesAssignedToPropertiesRule(new RuleLevelHelper($this->createReflectionProvider(), true, false, true, $this->checkExplicitMixed, false, false, true), new PropertyReflectionFinder()); + return new TypesAssignedToPropertiesRule(new RuleLevelHelper($this->createReflectionProvider(), true, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, false, true), new PropertyReflectionFinder()); } public function testTypesAssignedToProperties(): void @@ -779,4 +781,35 @@ public function testPropertyHooks(): void ]); } + public function testBug13093c(): void + { + $this->analyse([__DIR__ . '/data/bug-13093c.php'], []); + } + + public function testBug13093d(): void + { + $this->analyse([__DIR__ . '/data/bug-13093d.php'], []); + } + + public function testBug8825(): void + { + $this->checkImplicitMixed = true; + $this->analyse([__DIR__ . '/data/bug-8825.php'], []); + } + + public function testBug7844(): void + { + $this->analyse([__DIR__ . '/data/bug-7844.php'], []); + } + + public function testBug7844b(): void + { + $this->analyse([__DIR__ . '/data/bug-7844b.php'], []); + } + + public function testBug12675(): void + { + $this->analyse([__DIR__ . '/data/bug-12675.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-12675.php b/tests/PHPStan/Rules/Properties/data/bug-12675.php new file mode 100644 index 0000000000..ea4674dd3f --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-12675.php @@ -0,0 +1,37 @@ +username = array_shift($pieces); + $this->domain = array_shift($pieces); + + echo "{$this->username}@{$this->domain}"; + } + + public function with_pop(string $email): void + { + $pieces = explode("@", $email); + if (2 !== count($pieces)) { + + throw new \Exception("Bad, very bad..."); + } + + $this->domain = array_pop($pieces); + $this->username = array_pop($pieces); + + echo "{$this->username}@{$this->domain}"; + } +} diff --git a/tests/PHPStan/Rules/Properties/data/bug-13093c.php b/tests/PHPStan/Rules/Properties/data/bug-13093c.php new file mode 100644 index 0000000000..3fecc5f91d --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-13093c.php @@ -0,0 +1,49 @@ + + */ + private array $nextMutantProcessKillerContainer = []; + + private string $prop; + + public function fillBucketOnce(array &$killer): int + { + if ($this->nextMutantProcessKillerContainer !== []) { + $this->prop = array_shift($this->nextMutantProcessKillerContainer); + } + + return 0; + } + +} + +final class ParallelProcessRunner2 +{ + /** + * @var array + */ + private array $nextMutantProcessKillerContainer = []; + + private string $prop; + + public function fillBucketOnce(array &$killer): int + { + $name = 'prop'; + if ($this->nextMutantProcessKillerContainer !== []) { + $this->{$name} = array_shift($this->nextMutantProcessKillerContainer); + } + + return 0; + } + +} + diff --git a/tests/PHPStan/Rules/Properties/data/bug-13093d.php b/tests/PHPStan/Rules/Properties/data/bug-13093d.php new file mode 100644 index 0000000000..1682c7c22a --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-13093d.php @@ -0,0 +1,50 @@ + + */ + private array $nextMutantProcessKillerContainer = []; + + static private string $prop; + + public function fillBucketOnce(array &$killer): int + { + if ($this->nextMutantProcessKillerContainer !== []) { + self::$prop = array_shift($this->nextMutantProcessKillerContainer); + } + + return 0; + } + +} + +final class ParallelProcessRunner2 +{ + /** + * @var array + */ + private array $nextMutantProcessKillerContainer = []; + + static private string $prop; + + public function fillBucketOnce(array &$killer): int + { + $name = 'prop'; + if ($this->nextMutantProcessKillerContainer !== []) { + self::${$name} = array_shift($this->nextMutantProcessKillerContainer); + } + + return 0; + } + +} + + diff --git a/tests/PHPStan/Rules/Properties/data/bug-7844.php b/tests/PHPStan/Rules/Properties/data/bug-7844.php new file mode 100644 index 0000000000..b202edd3f2 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-7844.php @@ -0,0 +1,20 @@ + */ + public $data = array(); + + public function foo(): void + { + if (count($this->data) > 0) { + $this->val = array_shift($this->data); + } + } +} + diff --git a/tests/PHPStan/Rules/Properties/data/bug-7844b.php b/tests/PHPStan/Rules/Properties/data/bug-7844b.php new file mode 100644 index 0000000000..c423628f8c --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-7844b.php @@ -0,0 +1,39 @@ + $objs */ + public function __construct(array $objs) + { + \assert($objs !== []); + $this->p1 = $objs[0]; + + \assert($objs !== []); + $this->p2 = $objs[array_key_last($objs)]; + + \assert($objs !== []); + $this->p3 = \array_pop($objs); + + \assert($objs !== []); + $this->p4 = \array_shift($objs); + + \assert($objs !== []); + $p = \array_shift($objs); + $this->p5 = $p; + + \assert($objs !== []); + $this->doSomething(\array_pop($objs)); + } + + private function doSomething(Obj $obj): void {} +} diff --git a/tests/PHPStan/Rules/Properties/data/bug-8825.php b/tests/PHPStan/Rules/Properties/data/bug-8825.php new file mode 100644 index 0000000000..e1aa5c372d --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-8825.php @@ -0,0 +1,24 @@ +isBool = $actionParameters['my_key'] ?? false; + } + + public function use(): void + { + $this->isBool->someMethod(); + } +} diff --git a/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php b/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php index f8268f8fcd..1e651b6b48 100644 --- a/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php +++ b/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php @@ -64,4 +64,14 @@ public function testBenevolentArrayKey(): void $this->analyse([__DIR__ . '/data/benevolent-array-key.php'], []); } + public function testBug13093(): void + { + $this->analyse([__DIR__ . '/data/bug-13093.php'], []); + } + + public function testBug13093b(): void + { + $this->analyse([__DIR__ . '/data/bug-13093b.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-13093.php b/tests/PHPStan/Rules/Variables/data/bug-13093.php new file mode 100644 index 0000000000..0d780ce1a0 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-13093.php @@ -0,0 +1,40 @@ + + */ + private array $nextMutantProcessKillerContainer = []; + + /** + * @param MutantProcessContainer[] $bucket + * @param Generator $input + */ + public function fillBucketOnce(array &$bucket, Generator $input, int $threadCount): int + { + if (count($bucket) >= $threadCount || !$input->valid()) { + if ($this->nextMutantProcessKillerContainer !== []) { + $bucket[] = array_shift($this->nextMutantProcessKillerContainer); + } + + return 0; + } + + return 1; + } + +} + diff --git a/tests/PHPStan/Rules/Variables/data/bug-13093b.php b/tests/PHPStan/Rules/Variables/data/bug-13093b.php new file mode 100644 index 0000000000..5e462a1cbe --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-13093b.php @@ -0,0 +1,26 @@ + + */ + private array $nextMutantProcessKillerContainer = []; + + public function fillBucketOnce(string &$killer): int + { + if ($this->nextMutantProcessKillerContainer !== []) { + $killer = array_shift($this->nextMutantProcessKillerContainer); + } + + return 0; + } + +} +