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.

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

Feature/disallow dynamic property option #1234

Merged
1 change: 1 addition & 0 deletions conf/config.level0.neon
Expand Up @@ -146,6 +146,7 @@ services:
- phpstan.rules.rule
arguments:
reportMagicProperties: %reportMagicProperties%
checkDynamicProperties: %checkDynamicProperties%

-
class: PHPStan\Rules\Properties\AccessStaticPropertiesRule
Expand Down
2 changes: 2 additions & 0 deletions conf/config.neon
Expand Up @@ -62,6 +62,7 @@ parameters:
checkMissingTypehints: false
checkTooWideReturnTypesInProtectedAndPublicMethods: false
checkUninitializedProperties: false
checkDynamicProperties: false
inferPrivatePropertyTypeFromConstructor: false
reportMaybes: false
reportMaybesInMethodSignatures: false
Expand Down Expand Up @@ -252,6 +253,7 @@ parametersSchema:
checkMissingTypehints: bool()
checkTooWideReturnTypesInProtectedAndPublicMethods: bool()
checkUninitializedProperties: bool()
checkDynamicProperties: bool()
inferPrivatePropertyTypeFromConstructor: bool()

tipsOfTheDay: bool()
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/DirectScopeFactory.php
Expand Up @@ -46,7 +46,7 @@ public function __construct(
* @param VariableTypeHolder[] $moreSpecificTypes
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, bool> $currentlyAllowedUndefinedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param array<string, Type> $nativeExpressionTypes
* @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack
*
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/LazyScopeFactory.php
Expand Up @@ -38,7 +38,7 @@ public function __construct(
* @param VariableTypeHolder[] $moreSpecificTypes
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, bool> $currentlyAllowedUndefinedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param array<string, Type> $nativeExpressionTypes
* @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack
*
Expand Down
8 changes: 4 additions & 4 deletions src/Analyser/MutatingScope.php
Expand Up @@ -167,7 +167,7 @@ class MutatingScope implements Scope
* @param VariableTypeHolder[] $moreSpecificTypes
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, bool> $currentlyAllowedUndefinedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param array<string, Type> $nativeExpressionTypes
* @param array<MethodReflection|FunctionReflection> $inFunctionCallsStack
*/
Expand Down Expand Up @@ -3906,11 +3906,11 @@ public function isInExpressionAssign(Expr $expr): bool
return array_key_exists($exprString, $this->currentlyAssignedExpressions);
}

public function setAllowedUndefinedExpression(Expr $expr, bool $isAllowed): self
public function setAllowedUndefinedExpression(Expr $expr): self
{
$exprString = $this->getNodeKey($expr);
$currentlyAllowedUndefinedExpressions = $this->currentlyAllowedUndefinedExpressions;
$currentlyAllowedUndefinedExpressions[$exprString] = $isAllowed;
$currentlyAllowedUndefinedExpressions[$exprString] = true;

return $this->scopeFactory->create(
$this->context,
Expand Down Expand Up @@ -3964,7 +3964,7 @@ public function unsetAllowedUndefinedExpression(Expr $expr): self
public function isUndefinedExpressionAllowed(Expr $expr): bool
{
$exprString = $this->getNodeKey($expr);
return array_key_exists($exprString, $this->currentlyAllowedUndefinedExpressions) && $this->currentlyAllowedUndefinedExpressions[$exprString];
return array_key_exists($exprString, $this->currentlyAllowedUndefinedExpressions);
}

public function assignVariable(string $variableName, Type $type, ?TrinaryLogic $certainty = null): self
Expand Down
62 changes: 23 additions & 39 deletions src/Analyser/NodeScopeResolver.php
Expand Up @@ -1336,9 +1336,9 @@ private function processStmtNode(
$hasYield = false;
$throwPoints = [];
foreach ($stmt->vars as $var) {
$scope = $this->lookForEnterAllowedUndefinedVariable($scope, $var, true);
$scope = $this->lookForSetAllowedUndefinedExpressions($scope, $var);
$scope = $this->processExprNode($var, $scope, $nodeCallback, ExpressionContext::createDeep())->getScope();
$scope = $this->lookForExitAllowedUndefinedVariable($scope, $var);
$scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var);
$scope = $scope->unsetExpression($var);
}
} elseif ($stmt instanceof Node\Stmt\Use_) {
Expand All @@ -1355,9 +1355,9 @@ private function processStmtNode(
if (!$var instanceof Variable) {
throw new ShouldNotHappenException();
}
$scope = $this->lookForEnterAllowedUndefinedVariable($scope, $var, true);
$scope = $this->lookForSetAllowedUndefinedExpressions($scope, $var);
$this->processExprNode($var, $scope, $nodeCallback, ExpressionContext::createDeep());
$scope = $this->lookForExitAllowedUndefinedVariable($scope, $var);
$scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var);

if (!is_string($var->name)) {
continue;
Expand Down Expand Up @@ -1513,56 +1513,40 @@ private function createAstClassReflection(Node\Stmt\ClassLike $stmt, string $cla
);
}

private function lookForEnterAllowedUndefinedVariable(MutatingScope $scope, Expr $expr, bool $isAllowed): MutatingScope
private function lookForSetAllowedUndefinedExpressions(MutatingScope $scope, Expr $expr): MutatingScope
{
if (!$expr instanceof ArrayDimFetch || $expr->dim !== null) {
$scope = $scope->setAllowedUndefinedExpression($expr, $isAllowed);
}
if (!$expr instanceof Variable) {
return $this->lookForVariableCallback($scope, $expr, static fn (MutatingScope $scope, Expr $expr): MutatingScope => $scope->setAllowedUndefinedExpression($expr, $isAllowed));
}

return $scope;
return $this->lookForExpressionCallback($scope, $expr, static fn (MutatingScope $scope, Expr $expr): MutatingScope => $scope->setAllowedUndefinedExpression($expr));
}

private function lookForExitAllowedUndefinedVariable(MutatingScope $scope, Expr $expr): MutatingScope
private function lookForUnsetAllowedUndefinedExpressions(MutatingScope $scope, Expr $expr): MutatingScope
{
if (!$expr instanceof ArrayDimFetch || $expr->dim !== null) {
$scope = $scope->unsetAllowedUndefinedExpression($expr);
}
if (!$expr instanceof Variable) {
return $this->lookForVariableCallback($scope, $expr, static fn (MutatingScope $scope, Expr $expr): MutatingScope => $scope->unsetAllowedUndefinedExpression($expr));
}

return $scope;
return $this->lookForExpressionCallback($scope, $expr, static fn (MutatingScope $scope, Expr $expr): MutatingScope => $scope->unsetAllowedUndefinedExpression($expr));
}

/**
* @param Closure(MutatingScope $scope, Expr $expr): MutatingScope $callback
*/
private function lookForVariableCallback(MutatingScope $scope, Expr $expr, Closure $callback): MutatingScope
private function lookForExpressionCallback(MutatingScope $scope, Expr $expr, Closure $callback): MutatingScope
{
if ($expr instanceof Variable) {
if (!$expr instanceof ArrayDimFetch || $expr->dim !== null) {
$scope = $callback($scope, $expr);
} elseif ($expr instanceof ArrayDimFetch) {
if ($expr->dim !== null) {
$scope = $callback($scope, $expr);
}
}

$scope = $this->lookForVariableCallback($scope, $expr->var, $callback);
if ($expr instanceof ArrayDimFetch) {
$scope = $this->lookForExpressionCallback($scope, $expr->var, $callback);
} elseif ($expr instanceof PropertyFetch || $expr instanceof Expr\NullsafePropertyFetch) {
$scope = $this->lookForVariableCallback($scope, $expr->var, $callback);
$scope = $this->lookForExpressionCallback($scope, $expr->var, $callback);
} elseif ($expr instanceof StaticPropertyFetch) {
if ($expr->class instanceof Expr) {
$scope = $this->lookForVariableCallback($scope, $expr->class, $callback);
$scope = $this->lookForExpressionCallback($scope, $expr->class, $callback);
}
} elseif ($expr instanceof Array_ || $expr instanceof List_) {
foreach ($expr->items as $item) {
if ($item === null) {
continue;
}

$scope = $this->lookForVariableCallback($scope, $item->value, $callback);
$scope = $this->lookForExpressionCallback($scope, $item->value, $callback);
}
}

Expand Down Expand Up @@ -2321,10 +2305,10 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void {
);
} elseif ($expr instanceof Coalesce) {
$nonNullabilityResult = $this->ensureNonNullability($scope, $expr->left, false);
$condScope = $this->lookForEnterAllowedUndefinedVariable($nonNullabilityResult->getScope(), $expr->left, true);
$condScope = $this->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $expr->left);
$condResult = $this->processExprNode($expr->left, $condScope, $nodeCallback, $context->enterDeep());
$scope = $this->revertNonNullability($condResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions());
$scope = $this->lookForExitAllowedUndefinedVariable($scope, $expr->left);
$scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $expr->left);

$rightScope = $scope->filterByFalseyValue(new Expr\Isset_([$expr->left]));
$rightResult = $this->processExprNode($expr->right, $rightScope, $nodeCallback, $context->enterDeep());
Expand Down Expand Up @@ -2395,26 +2379,26 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void {
}
} elseif ($expr instanceof Expr\Empty_) {
$nonNullabilityResult = $this->ensureNonNullability($scope, $expr->expr, true);
$scope = $this->lookForEnterAllowedUndefinedVariable($nonNullabilityResult->getScope(), $expr->expr, true);
$scope = $this->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $expr->expr);
$result = $this->processExprNode($expr->expr, $scope, $nodeCallback, $context->enterDeep());
$scope = $result->getScope();
$hasYield = $result->hasYield();
$throwPoints = $result->getThrowPoints();
$scope = $this->revertNonNullability($scope, $nonNullabilityResult->getSpecifiedExpressions());
$scope = $this->lookForExitAllowedUndefinedVariable($scope, $expr->expr);
$scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $expr->expr);
} elseif ($expr instanceof Expr\Isset_) {
$hasYield = false;
$throwPoints = [];
$nonNullabilityResults = [];
foreach ($expr->vars as $var) {
$nonNullabilityResult = $this->ensureNonNullability($scope, $var, true);
$scope = $this->lookForEnterAllowedUndefinedVariable($nonNullabilityResult->getScope(), $var, true);
$scope = $this->lookForSetAllowedUndefinedExpressions($nonNullabilityResult->getScope(), $var);
$result = $this->processExprNode($var, $scope, $nodeCallback, $context->enterDeep());
$scope = $result->getScope();
$hasYield = $hasYield || $result->hasYield();
$throwPoints = array_merge($throwPoints, $result->getThrowPoints());
$nonNullabilityResults[] = $nonNullabilityResult;
$scope = $this->lookForExitAllowedUndefinedVariable($scope, $var);
$scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var);
}
foreach (array_reverse($nonNullabilityResults) as $nonNullabilityResult) {
$scope = $this->revertNonNullability($scope, $nonNullabilityResult->getSpecifiedExpressions());
Expand Down Expand Up @@ -3466,7 +3450,7 @@ static function (): void {
if ($arrayItem->value instanceof ArrayDimFetch && $arrayItem->value->dim === null) {
$itemScope = $itemScope->enterExpressionAssign($arrayItem->value);
}
$itemScope = $this->lookForEnterAllowedUndefinedVariable($itemScope, $arrayItem->value, true);
$itemScope = $this->lookForSetAllowedUndefinedExpressions($itemScope, $arrayItem->value);
$itemResult = $this->processExprNode($arrayItem, $itemScope, $nodeCallback, $context->enterDeep());
$hasYield = $hasYield || $itemResult->hasYield();
$throwPoints = array_merge($throwPoints, $itemResult->getThrowPoints());
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/ScopeFactory.php
Expand Up @@ -17,7 +17,7 @@ interface ScopeFactory
* @param VariableTypeHolder[] $moreSpecificTypes
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, bool> $currentlyAllowedUndefinedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param array<string, Type> $nativeExpressionTypes
* @param array<MethodReflection|FunctionReflection> $inFunctionCallsStack
*
Expand Down
8 changes: 7 additions & 1 deletion src/Rules/Properties/AccessPropertiesRule.php
Expand Up @@ -33,6 +33,7 @@ public function __construct(
private ReflectionProvider $reflectionProvider,
private RuleLevelHelper $ruleLevelHelper,
private bool $reportMagicProperties,
private bool $checkDynamicProperties,
)
{
}
Expand Down Expand Up @@ -88,7 +89,7 @@ private function processSingleProperty(Scope $scope, PropertyFetch $node, string
];
}

if ($scope->isUndefinedExpressionAllowed($node)) {
if ($this->canAccessUndefinedProperties($scope, $node)) {
return [];
}

Expand Down Expand Up @@ -162,4 +163,9 @@ private function processSingleProperty(Scope $scope, PropertyFetch $node, string
return [];
}

private function canAccessUndefinedProperties(Scope $scope, Node\Expr $node): bool
{
return $scope->isUndefinedExpressionAllowed($node) && !$this->checkDynamicProperties;
}

}
Expand Up @@ -17,7 +17,7 @@ protected function getRule(): Rule
{
$reflectionProvider = $this->createReflectionProvider();
return new AccessPropertiesInAssignRule(
new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false), true),
new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false), true, true),
);
}

Expand Down