From d3a2a92fcd612bf42bbfd19cd3a5625481ff7522 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 10 Sep 2024 13:44:44 +0200 Subject: [PATCH] Process expression assignments other than Variable in by-ref parameters --- src/Analyser/NodeScopeResolver.php | 33 +++++++++----- .../TypesAssignedToPropertiesRuleTest.php | 15 +++++++ .../data/properties-assigned-types.php | 45 +++++++++++++++++++ .../TooWidePropertyTypeRuleTest.php | 5 +++ .../Rules/TooWideTypehints/data/bug-11667.php | 40 +++++++++++++++++ 5 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 tests/PHPStan/Rules/TooWideTypehints/data/bug-11667.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index b22d651f99..f09e4a9bb6 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4767,18 +4767,29 @@ private function processArgs( } $argValue = $arg->value; - if ($argValue instanceof Variable && is_string($argValue->name)) { - if ($argValue->name !== 'this') { - $paramOutType = $this->getParameterOutExtensionsType($callLike, $calleeReflection, $currentParameter, $scope); - if ($paramOutType !== null) { - $byRefType = $paramOutType; - } - - $nodeCallback(new VariableAssignNode($argValue, new TypeExpr($byRefType), false), $scope); - $scope = $scope->assignVariable($argValue->name, $byRefType, new MixedType()); + if (!$argValue instanceof Variable || $argValue->name !== 'this') { + $paramOutType = $this->getParameterOutExtensionsType($callLike, $calleeReflection, $currentParameter, $scope); + if ($paramOutType !== null) { + $byRefType = $paramOutType; } - } else { - $scope = $scope->invalidateExpression($argValue); + + $result = $this->processAssignVar( + $scope, + $stmt, + $argValue, + new TypeExpr($byRefType), + static function (Node $node, Scope $scope) use ($nodeCallback): void { + if (!$node instanceof PropertyAssignNode && !$node instanceof VariableAssignNode) { + return; + } + + $nodeCallback($node, $scope); + }, + $context, + static fn (MutatingScope $scope): ExpressionResult => new ExpressionResult($scope, false, [], []), + true, + ); + $scope = $result->getScope(); } } elseif ($calleeReflection !== null && $calleeReflection->hasSideEffects()->yes()) { $argType = $scope->getType($arg->value); diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index 09f34d1d40..33551884d8 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -107,6 +107,21 @@ public function testTypesAssignedToProperties(): void 'Property PropertiesAssignedTypes\AppendToArrayAccess::$collection2 (ArrayAccess&Countable) does not accept Countable.', 376, ], + [ + 'Property PropertiesAssignedTypes\ParamOutAssign::$foo (list) does not accept string.', + 400, + 'string is not a list.', + ], + [ + 'Property PropertiesAssignedTypes\ParamOutAssign::$foo2 (list>) does not accept string.', + 410, + 'string is not a list.', + ], + [ + 'Property PropertiesAssignedTypes\ParamOutAssign::$foo2 (list>) does not accept non-empty-list|string>.', + 415, + 'list|string might not be a list.', + ], ]); } diff --git a/tests/PHPStan/Rules/Properties/data/properties-assigned-types.php b/tests/PHPStan/Rules/Properties/data/properties-assigned-types.php index 3686d4d244..14b3ad66e4 100644 --- a/tests/PHPStan/Rules/Properties/data/properties-assigned-types.php +++ b/tests/PHPStan/Rules/Properties/data/properties-assigned-types.php @@ -376,3 +376,48 @@ public function foo(): void $this->collection2[] = 2; } } + +class ParamOutAssign +{ + + /** @var list */ + private $foo; + + /** @var list> */ + private $foo2; + + /** + * @param mixed $a + * @param-out string $a + */ + public function paramOut(&$a): void + { + + } + + public function doFoo(): void + { + $this->paramOut($this->foo); + } + + public function doFoo2(): void + { + $this->paramOut($this->foo[0]); + } + + public function doBar(): void + { + $this->paramOut($this->foo2); + } + + public function doBar2(): void + { + $this->paramOut($this->foo2[0]); + } + + public function doBar3(): void + { + $this->paramOut($this->foo2[0][0]); + } + +} diff --git a/tests/PHPStan/Rules/TooWideTypehints/TooWidePropertyTypeRuleTest.php b/tests/PHPStan/Rules/TooWideTypehints/TooWidePropertyTypeRuleTest.php index f512d22fc7..1171abd564 100644 --- a/tests/PHPStan/Rules/TooWideTypehints/TooWidePropertyTypeRuleTest.php +++ b/tests/PHPStan/Rules/TooWideTypehints/TooWidePropertyTypeRuleTest.php @@ -56,4 +56,9 @@ public function testRule(): void ]); } + public function testBug11667(): void + { + $this->analyse([__DIR__ . '/data/bug-11667.php'], []); + } + } diff --git a/tests/PHPStan/Rules/TooWideTypehints/data/bug-11667.php b/tests/PHPStan/Rules/TooWideTypehints/data/bug-11667.php new file mode 100644 index 0000000000..09bd5a8919 --- /dev/null +++ b/tests/PHPStan/Rules/TooWideTypehints/data/bug-11667.php @@ -0,0 +1,40 @@ +|null */ + private $matches = null; + + public function match(string $string): void { + preg_match('/Hello (\w+)/', $string, $this->matches); + } + + /** @return list|null */ + public function get(): ?array { + return $this->matches; + } +} + +final class HelloWorld2 { + /** @var list|null */ + private $matches = null; + + public function match(string $string): void { + $this->paramOut($this->matches); + } + + /** + * @param mixed $a + * @param-out list $a + */ + public function paramOut(&$a): void + { + + } + + /** @return list|null */ + public function get(): ?array { + return $this->matches; + } +}