Feature/disallow dynamic property option

rajyan · web-flow · commit f13c2c7e9fa2 · 2022-04-21T15:01:26.000+02:00
diff --git a/conf/config.level0.neon b/conf/config.level0.neon
@@ -146,6 +146,7 @@ services:
 			- phpstan.rules.rule
 		arguments:
 			reportMagicProperties: %reportMagicProperties%
+			checkDynamicProperties: %checkDynamicProperties%
 
 	-
 		class: PHPStan\Rules\Properties\AccessStaticPropertiesRule
diff --git a/conf/config.neon b/conf/config.neon
@@ -62,6 +62,7 @@ parameters:
 	checkMissingTypehints: false
 	checkTooWideReturnTypesInProtectedAndPublicMethods: false
 	checkUninitializedProperties: false
+	checkDynamicProperties: false
 	inferPrivatePropertyTypeFromConstructor: false
 	reportMaybes: false
 	reportMaybesInMethodSignatures: false
@@ -252,6 +253,7 @@ parametersSchema:
 	checkMissingTypehints: bool()
 	checkTooWideReturnTypesInProtectedAndPublicMethods: bool()
 	checkUninitializedProperties: bool()
+	checkDynamicProperties: bool()
 	inferPrivatePropertyTypeFromConstructor: bool()
 
 	tipsOfTheDay: bool()
diff --git a/src/Analyser/DirectScopeFactory.php b/src/Analyser/DirectScopeFactory.php
@@ -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
 	 *
diff --git a/src/Analyser/LazyScopeFactory.php b/src/Analyser/LazyScopeFactory.php
@@ -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
 	 *
diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php
@@ -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
 	 */
@@ -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,
@@ -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
diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
@@ -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_) {
@@ -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;
@@ -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);
 			}
 		}
 
@@ -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());
@@ -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());
@@ -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());
diff --git a/src/Analyser/ScopeFactory.php b/src/Analyser/ScopeFactory.php
@@ -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
 	 *
diff --git a/src/Rules/Properties/AccessPropertiesRule.php b/src/Rules/Properties/AccessPropertiesRule.php
@@ -33,6 +33,7 @@ public function __construct(
 		private ReflectionProvider $reflectionProvider,
 		private RuleLevelHelper $ruleLevelHelper,
 		private bool $reportMagicProperties,
+		private bool $checkDynamicProperties,
 	)
 	{
 	}
@@ -88,7 +89,7 @@ private function processSingleProperty(Scope $scope, PropertyFetch $node, string
 			];
 		}
 
-		if ($scope->isUndefinedExpressionAllowed($node)) {
+		if ($this->canAccessUndefinedProperties($scope, $node)) {
 			return [];
 		}
 
@@ -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;
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php
@@ -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),
 		);
 	}
 
diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php
diff --git a/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php
diff --git a/tests/PHPStan/Rules/Properties/data/bug-3171.php b/tests/PHPStan/Rules/Properties/data/bug-3171.php

Original file line number	Diff line number	Diff line change
`@@ -146,6 +146,7 @@ services:`
`146`	`146`	`- phpstan.rules.rule`
`147`	`147`	`arguments:`
`148`	`148`	`reportMagicProperties: %reportMagicProperties%`
	`149`	`+ checkDynamicProperties: %checkDynamicProperties%`
`149`	`150`
`150`	`151`	`-`
`151`	`152`	`class: PHPStan\Rules\Properties\AccessStaticPropertiesRule`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ public function __construct(`
`46`	`46`	`* @param VariableTypeHolder[] $moreSpecificTypes`
`47`	`47`	`* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions`
`48`	`48`	`* @param array<string, true> $currentlyAssignedExpressions`
`49`		`- * @param array<string, bool> $currentlyAllowedUndefinedExpressions`
	`49`	`+ * @param array<string, true> $currentlyAllowedUndefinedExpressions`
`50`	`50`	`* @param array<string, Type> $nativeExpressionTypes`
`51`	`51`	`* @param array<(FunctionReflection\|MethodReflection)> $inFunctionCallsStack`
`52`	`52`	`*`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ public function __construct(`
`38`	`38`	`* @param VariableTypeHolder[] $moreSpecificTypes`
`39`	`39`	`* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions`
`40`	`40`	`* @param array<string, true> $currentlyAssignedExpressions`
`41`		`- * @param array<string, bool> $currentlyAllowedUndefinedExpressions`
	`41`	`+ * @param array<string, true> $currentlyAllowedUndefinedExpressions`
`42`	`42`	`* @param array<string, Type> $nativeExpressionTypes`
`43`	`43`	`* @param array<(FunctionReflection\|MethodReflection)> $inFunctionCallsStack`
`44`	`44`	`*`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ interface ScopeFactory`
`17`	`17`	`* @param VariableTypeHolder[] $moreSpecificTypes`
`18`	`18`	`* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions`
`19`	`19`	`* @param array<string, true> $currentlyAssignedExpressions`
`20`		`- * @param array<string, bool> $currentlyAllowedUndefinedExpressions`
	`20`	`+ * @param array<string, true> $currentlyAllowedUndefinedExpressions`
`21`	`21`	`* @param array<string, Type> $nativeExpressionTypes`
`22`	`22`	`* @param array<MethodReflection\|FunctionReflection> $inFunctionCallsStack`
`23`	`23`	`*`
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@ public function __construct(`
`33`	`33`	`private ReflectionProvider $reflectionProvider,`
`34`	`34`	`private RuleLevelHelper $ruleLevelHelper,`
`35`	`35`	`private bool $reportMagicProperties,`
	`36`	`+ private bool $checkDynamicProperties,`
`36`	`37`	`)`
`37`	`38`	`{`
`38`	`39`	`}`
`@@ -88,7 +89,7 @@ private function processSingleProperty(Scope $scope, PropertyFetch $node, string`
`88`	`89`	`];`
`89`	`90`	`}`
`90`	`91`
`91`		`- if ($scope->isUndefinedExpressionAllowed($node)) {`
	`92`	`+ if ($this->canAccessUndefinedProperties($scope, $node)) {`
`92`	`93`	`return [];`
`93`	`94`	`}`
`94`	`95`
`@@ -162,4 +163,9 @@ private function processSingleProperty(Scope $scope, PropertyFetch $node, string`
`162`	`163`	`return [];`
`163`	`164`	`}`
`164`	`165`
	`166`	`+ private function canAccessUndefinedProperties(Scope $scope, Node\Expr $node): bool`
	`167`	`+ {`
	`168`	`+ return $scope->isUndefinedExpressionAllowed($node) && !$this->checkDynamicProperties;`
	`169`	`+ }`
	`170`	`+`
`165`	`171`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ protected function getRule(): Rule`
`17`	`17`	`{`
`18`	`18`	`$reflectionProvider = $this->createReflectionProvider();`
`19`	`19`	`return new AccessPropertiesInAssignRule(`
`20`		`- new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false), true),`
	`20`	`+ new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false), true, true),`
`21`	`21`	`);`
`22`	`22`	`}`
`23`	`23`