Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix/issue 3171 dynamic properties #1237

Merged
merged 6 commits into from Apr 22, 2022
Merged

Fix/issue 3171 dynamic properties #1237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3b371a3
add false positive test case for checkDynamicProperties
rajyan Apr 22, 2022
d450536
traverse on ArrayDimFetch too
rajyan Apr 22, 2022
7829af9
fix solved false positives
rajyan Apr 22, 2022
b8a6d7f
make coalesce non-null scope consistent with isset and empty
rajyan Apr 22, 2022
5aacc1f
refactoring. Use consistent logic with undefined expression allowed
rajyan Apr 22, 2022
1c9ae29
fix cs
rajyan Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 10 additions & 25 deletions src/Analyser/NodeScopeResolver.php
View file
Open in desktop
Expand Up @@ -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) {
Expand Down Expand Up @@ -1576,29 +1574,16 @@ 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 = [];
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 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);
}
Expand Down Expand Up @@ -2304,7 +2289,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());
Expand Down Expand Up @@ -2378,7 +2363,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();
Expand All @@ -2391,7 +2376,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();
Expand Down
12 changes: 8 additions & 4 deletions tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php
View file
Open in desktop
Expand Up @@ -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,
],
],
);
}
Expand Down Expand Up @@ -840,4 +836,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'], []);
}

}