From 3b371a3249c28c9f641b20d600cec131834d09ff Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Fri, 22 Apr 2022 09:51:30 +0900 Subject: [PATCH 1/6] add false positive test case for checkDynamicProperties --- .../PHPStan/Rules/Properties/AccessPropertiesRuleTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index 1ee7b1aec8..5dfc37d7a1 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -840,4 +840,12 @@ public function testBug3171(): void $this->analyse([__DIR__ . '/data/bug-3171.php'], []); } + public function testBug3171OnDynamicProperties(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = true; + $this->checkDynamicProperties = true; + $this->analyse([__DIR__ . '/data/bug-3171.php'], []); + } + } From d4505361914a181b3595a703a25105b0d876cf5c Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Fri, 22 Apr 2022 09:52:12 +0900 Subject: [PATCH 2/6] traverse on ArrayDimFetch too --- src/Analyser/NodeScopeResolver.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index c1e8b62bd6..6ebeea7471 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1587,7 +1587,9 @@ private function ensureNonNullability(MutatingScope $scope, Expr $expr, bool $fi $specifiedExpressions[] = $specifiedExpression; } - if ($exprToSpecify instanceof PropertyFetch) { + if ($exprToSpecify instanceof ArrayDimFetch && $exprToSpecify->dim !== null) { + $exprToSpecify = $exprToSpecify->var; + } elseif ($exprToSpecify instanceof PropertyFetch) { $exprToSpecify = $exprToSpecify->var; } elseif ($exprToSpecify instanceof StaticPropertyFetch && $exprToSpecify->class instanceof Expr) { $exprToSpecify = $exprToSpecify->class; From 7829af940fb81ea3a45c797371fa66dfd63c5b58 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Fri, 22 Apr 2022 10:26:44 +0900 Subject: [PATCH 3/6] fix solved false positives --- tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index 5dfc37d7a1..614b5aef6a 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -510,10 +510,6 @@ public function testAccessPropertiesOnDynamicProperties(): void 'Cannot access property $selfOrNull on TestAccessProperties\RevertNonNullabilityForIsset|null.', 402, ], - [ - 'Access to an undefined property stdClass|null::$array.', - 412, - ], ], ); } From b8a6d7fddbb3351b789ae96014a91b3853f2290e Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Fri, 22 Apr 2022 10:29:08 +0900 Subject: [PATCH 4/6] make coalesce non-null scope consistent with isset and empty --- src/Analyser/NodeScopeResolver.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 6ebeea7471..ed14e6b379 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1576,7 +1576,7 @@ private function ensureShallowNonNullability(MutatingScope $scope, Expr $exprToS return new EnsuredNonNullabilityResult($scope, []); } - private function ensureNonNullability(MutatingScope $scope, Expr $expr, bool $findMethods): EnsuredNonNullabilityResult + private function ensureNonNullability(MutatingScope $scope, Expr $expr): EnsuredNonNullabilityResult { $exprToSpecify = $expr; $specifiedExpressions = []; @@ -2306,7 +2306,7 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void { static fn (): MutatingScope => $rightResult->getScope()->filterByFalseyValue($expr), ); } elseif ($expr instanceof Coalesce) { - $nonNullabilityResult = $this->ensureNonNullability($scope, $expr->left, false); + $nonNullabilityResult = $this->ensureNonNullability($scope, $expr->left); $condScope = $this->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $expr->left); $condResult = $this->processExprNode($expr->left, $condScope, $nodeCallback, $context->enterDeep()); $scope = $this->revertNonNullability($condResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions()); @@ -2380,7 +2380,7 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void { $throwPoints = $result->getThrowPoints(); } } elseif ($expr instanceof Expr\Empty_) { - $nonNullabilityResult = $this->ensureNonNullability($scope, $expr->expr, true); + $nonNullabilityResult = $this->ensureNonNullability($scope, $expr->expr); $scope = $this->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $expr->expr); $result = $this->processExprNode($expr->expr, $scope, $nodeCallback, $context->enterDeep()); $scope = $result->getScope(); @@ -2393,7 +2393,7 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void { $throwPoints = []; $nonNullabilityResults = []; foreach ($expr->vars as $var) { - $nonNullabilityResult = $this->ensureNonNullability($scope, $var, true); + $nonNullabilityResult = $this->ensureNonNullability($scope, $var); $scope = $this->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $var); $result = $this->processExprNode($var, $scope, $nodeCallback, $context->enterDeep()); $scope = $result->getScope(); From 5aacc1fd107d2c10aa8354b93bec1226dcf7bb4a Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Fri, 22 Apr 2022 10:36:17 +0900 Subject: [PATCH 5/6] refactoring. Use consistent logic with undefined expression allowed --- src/Analyser/NodeScopeResolver.php | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index ed14e6b379..36e10416d5 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1536,10 +1536,8 @@ private function lookForExpressionCallback(MutatingScope $scope, Expr $expr, Clo $scope = $this->lookForExpressionCallback($scope, $expr->var, $callback); } elseif ($expr instanceof PropertyFetch || $expr instanceof Expr\NullsafePropertyFetch) { $scope = $this->lookForExpressionCallback($scope, $expr->var, $callback); - } elseif ($expr instanceof StaticPropertyFetch) { - if ($expr->class instanceof Expr) { - $scope = $this->lookForExpressionCallback($scope, $expr->class, $callback); - } + } elseif ($expr instanceof StaticPropertyFetch && $expr->class instanceof Expr) { + $scope = $this->lookForExpressionCallback($scope, $expr->class, $callback); } elseif ($expr instanceof Array_ || $expr instanceof List_) { foreach ($expr->items as $item) { if ($item === null) { @@ -1578,29 +1576,14 @@ private function ensureShallowNonNullability(MutatingScope $scope, Expr $exprToS private function ensureNonNullability(MutatingScope $scope, Expr $expr): EnsuredNonNullabilityResult { - $exprToSpecify = $expr; $specifiedExpressions = []; - while (true) { - $result = $this->ensureShallowNonNullability($scope, $exprToSpecify); - $scope = $result->getScope(); + $scope = $this->lookForExpressionCallback($scope, $expr, function($scope, $expr) use (&$specifiedExpressions) { + $result = $this->ensureShallowNonNullability($scope, $expr); foreach ($result->getSpecifiedExpressions() as $specifiedExpression) { $specifiedExpressions[] = $specifiedExpression; } - - if ($exprToSpecify instanceof ArrayDimFetch && $exprToSpecify->dim !== null) { - $exprToSpecify = $exprToSpecify->var; - } elseif ($exprToSpecify instanceof PropertyFetch) { - $exprToSpecify = $exprToSpecify->var; - } elseif ($exprToSpecify instanceof StaticPropertyFetch && $exprToSpecify->class instanceof Expr) { - $exprToSpecify = $exprToSpecify->class; - } elseif ($findMethods && $exprToSpecify instanceof MethodCall) { - $exprToSpecify = $exprToSpecify->var; - } elseif ($findMethods && $exprToSpecify instanceof StaticCall && $exprToSpecify->class instanceof Expr) { - $exprToSpecify = $exprToSpecify->class; - } else { - break; - } - } + return $result->getScope(); + }); return new EnsuredNonNullabilityResult($scope, $specifiedExpressions); } From 1c9ae2952d824008ece8646f88317ec2d6d901b5 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Fri, 22 Apr 2022 11:28:20 +0900 Subject: [PATCH 6/6] fix cs --- src/Analyser/NodeScopeResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 36e10416d5..4a7a75af33 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1577,7 +1577,7 @@ private function ensureShallowNonNullability(MutatingScope $scope, Expr $exprToS private function ensureNonNullability(MutatingScope $scope, Expr $expr): EnsuredNonNullabilityResult { $specifiedExpressions = []; - $scope = $this->lookForExpressionCallback($scope, $expr, function($scope, $expr) use (&$specifiedExpressions) { + $scope = $this->lookForExpressionCallback($scope, $expr, function ($scope, $expr) use (&$specifiedExpressions) { $result = $this->ensureShallowNonNullability($scope, $expr); foreach ($result->getSpecifiedExpressions() as $specifiedExpression) { $specifiedExpressions[] = $specifiedExpression;