From e7d204c1051e7a1e71183639e40c3cefed9ef210 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Wed, 20 Apr 2022 20:37:41 +0900 Subject: [PATCH 01/10] add failing test for issue-3171 --- .../Properties/AccessPropertiesRuleTest.php | 7 +++++++ .../Rules/Properties/data/bug-3171.php | 20 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 tests/PHPStan/Rules/Properties/data/bug-3171.php diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index ddfabe3eaf..c715dc4426 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -578,4 +578,11 @@ public function testBug4559(): void $this->analyse([__DIR__ . '/data/bug-4559.php'], []); } + public function testBug3171(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/data/bug-3171.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-3171.php b/tests/PHPStan/Rules/Properties/data/bug-3171.php new file mode 100644 index 0000000000..f42c794a5b --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-3171.php @@ -0,0 +1,20 @@ +property->someArray['test'] ?? 'test'; + } +} + From 36a99ed0e1c56a414f2752393a17b0edbb81efc1 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Wed, 20 Apr 2022 20:57:40 +0900 Subject: [PATCH 02/10] recursively allow undefined expressions in coalesce, isset, empty --- src/Analyser/NodeScopeResolver.php | 62 +++++++------------ .../Properties/AccessPropertiesRuleTest.php | 16 ----- .../AccessStaticPropertiesRuleTest.php | 4 -- 3 files changed, 23 insertions(+), 59 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 48cb5a518f..8367c7ce9f 100644 --- 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, true); $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, true); $this->processExprNode($var, $scope, $nodeCallback, ExpressionContext::createDeep()); - $scope = $this->lookForExitAllowedUndefinedVariable($scope, $var); + $scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var); if (!is_string($var->name)) { continue; @@ -1513,48 +1513,32 @@ 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, bool $isAllowed): 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, $isAllowed)); } - 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) { @@ -1562,7 +1546,7 @@ private function lookForVariableCallback(MutatingScope $scope, Expr $expr, Closu 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, true); $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, true); $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, true); $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, true); $itemResult = $this->processExprNode($arrayItem, $itemScope, $nodeCallback, $context->enterDeep()); $hasYield = $hasYield || $itemResult->hasYield(); $throwPoints = array_merge($throwPoints, $itemResult->getThrowPoints()); diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index c715dc4426..1d3e03741e 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -145,18 +145,10 @@ public function testAccessProperties(): void 'Access to an undefined property class@anonymous/tests/PHPStan/Rules/Properties/data/access-properties.php:294::$barProperty.', 299, ], - [ - 'Access to an undefined property TestAccessProperties\AccessInIsset::$foo.', - 386, - ], [ 'Cannot access property $selfOrNull on TestAccessProperties\RevertNonNullabilityForIsset|null.', 402, ], - [ - 'Cannot access property $array on stdClass|null.', - 412, - ], ], ); } @@ -267,10 +259,6 @@ public function testAccessPropertiesWithoutUnionTypes(): void 'Access to an undefined property class@anonymous/tests/PHPStan/Rules/Properties/data/access-properties.php:294::$barProperty.', 299, ], - [ - 'Access to an undefined property TestAccessProperties\AccessInIsset::$foo.', - 386, - ], ], ); } @@ -305,10 +293,6 @@ public function testAccessPropertiesOnThisOnly(): void 'Access to private property $foo of parent class TestAccessProperties\FooAccessProperties.', 24, ], - [ - 'Access to an undefined property TestAccessProperties\AccessInIsset::$foo.', - 386, - ], ], ); } diff --git a/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php index e5bdc4d0b7..409bcd4812 100644 --- a/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php @@ -164,10 +164,6 @@ public function testAccessStaticProperties(): void 'Static access to instance property ClassOrString::$instanceProperty.', 152, ], - [ - 'Access to an undefined static property AccessInIsset::$foo.', - 185, - ], [ 'Access to static property $foo on an unknown class TraitWithStaticProperty.', 209, From da04ef74deb279f6767191580f9ff630b318e2e4 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Wed, 20 Apr 2022 22:00:55 +0900 Subject: [PATCH 03/10] add checkDynamicProperties feature --- conf/config.neon | 3 +++ src/Analyser/NodeScopeResolver.php | 6 +++++- src/Testing/RuleTestCase.php | 1 + src/Testing/TypeInferenceTestCase.php | 1 + tests/PHPStan/Analyser/AnalyserTest.php | 1 + 5 files changed, 11 insertions(+), 1 deletion(-) diff --git a/conf/config.neon b/conf/config.neon index 03dda3c747..2177ac737d 100644 --- 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() @@ -498,6 +500,7 @@ services: earlyTerminatingMethodCalls: %earlyTerminatingMethodCalls% earlyTerminatingFunctionCalls: %earlyTerminatingFunctionCalls% implicitThrows: %exceptions.implicitThrows% + checkDynamicProperties: %checkDynamicProperties% - class: PHPStan\Analyser\ConstantResolver diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8367c7ce9f..80a0e88542 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -193,6 +193,7 @@ public function __construct( private array $earlyTerminatingMethodCalls, private array $earlyTerminatingFunctionCalls, private bool $implicitThrows, + private bool $checkDynamicProperties, ) { $earlyTerminatingMethodNames = []; @@ -1528,7 +1529,10 @@ private function lookForUnsetAllowedUndefinedExpressions(MutatingScope $scope, E */ private function lookForExpressionCallback(MutatingScope $scope, Expr $expr, Closure $callback): MutatingScope { - if (!$expr instanceof ArrayDimFetch || $expr->dim !== null) { + if ( + (!$expr instanceof ArrayDimFetch || $expr->dim !== null) && + (!$expr instanceof PropertyFetch && !$expr instanceof Expr\NullsafePropertyFetch || !$this->checkDynamicProperties) + ) { $scope = $callback($scope, $expr); } diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index 9751ad1fb1..d4f7687652 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -66,6 +66,7 @@ private function getAnalyser(): Analyser [], [], true, + false ); $fileAnalyser = new FileAnalyser( $this->createScopeFactory($reflectionProvider, $typeSpecifier), diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index 948c4f0084..ca135b6c53 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -56,6 +56,7 @@ public function processFile( $this->getEarlyTerminatingMethodCalls(), $this->getEarlyTerminatingFunctionCalls(), true, + false ); $resolver->setAnalysedFiles(array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], $this->getAdditionalAnalysedFiles()))); diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 275a7849b6..01621dae56 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -486,6 +486,7 @@ private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser [], [], true, + false ); $lexer = new Lexer(['usedAttributes' => ['comments', 'startLine', 'endLine', 'startTokenPos', 'endTokenPos']]); $fileAnalyser = new FileAnalyser( From 93aad950551e1b1b2f2820c1b4933378f03bb4ce Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Wed, 20 Apr 2022 22:07:30 +0900 Subject: [PATCH 04/10] change `$currentlyAllowedUndefinedExpressions` to `array` because false will not be set anymore --- src/Analyser/DirectScopeFactory.php | 2 +- src/Analyser/LazyScopeFactory.php | 2 +- src/Analyser/MutatingScope.php | 8 ++++---- src/Analyser/NodeScopeResolver.php | 16 ++++++++-------- src/Analyser/ScopeFactory.php | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Analyser/DirectScopeFactory.php b/src/Analyser/DirectScopeFactory.php index 4364eb9ea0..15538eed25 100644 --- a/src/Analyser/DirectScopeFactory.php +++ b/src/Analyser/DirectScopeFactory.php @@ -46,7 +46,7 @@ public function __construct( * @param VariableTypeHolder[] $moreSpecificTypes * @param array $conditionalExpressions * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions + * @param array $currentlyAllowedUndefinedExpressions * @param array $nativeExpressionTypes * @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack * diff --git a/src/Analyser/LazyScopeFactory.php b/src/Analyser/LazyScopeFactory.php index e5522da25b..e4d34f7818 100644 --- a/src/Analyser/LazyScopeFactory.php +++ b/src/Analyser/LazyScopeFactory.php @@ -38,7 +38,7 @@ public function __construct( * @param VariableTypeHolder[] $moreSpecificTypes * @param array $conditionalExpressions * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions + * @param array $currentlyAllowedUndefinedExpressions * @param array $nativeExpressionTypes * @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack * diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index fa068bbb7e..2d00b525f2 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -167,7 +167,7 @@ class MutatingScope implements Scope * @param VariableTypeHolder[] $moreSpecificTypes * @param array $conditionalExpressions * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions + * @param array $currentlyAllowedUndefinedExpressions * @param array $nativeExpressionTypes * @param array $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 index 80a0e88542..4c564d61dd 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1337,7 +1337,7 @@ private function processStmtNode( $hasYield = false; $throwPoints = []; foreach ($stmt->vars as $var) { - $scope = $this->lookForSetAllowedUndefinedExpressions($scope, $var, true); + $scope = $this->lookForSetAllowedUndefinedExpressions($scope, $var); $scope = $this->processExprNode($var, $scope, $nodeCallback, ExpressionContext::createDeep())->getScope(); $scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var); $scope = $scope->unsetExpression($var); @@ -1356,7 +1356,7 @@ private function processStmtNode( if (!$var instanceof Variable) { throw new ShouldNotHappenException(); } - $scope = $this->lookForSetAllowedUndefinedExpressions($scope, $var, true); + $scope = $this->lookForSetAllowedUndefinedExpressions($scope, $var); $this->processExprNode($var, $scope, $nodeCallback, ExpressionContext::createDeep()); $scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var); @@ -1514,9 +1514,9 @@ private function createAstClassReflection(Node\Stmt\ClassLike $stmt, string $cla ); } - private function lookForSetAllowedUndefinedExpressions(MutatingScope $scope, Expr $expr, bool $isAllowed): MutatingScope + private function lookForSetAllowedUndefinedExpressions(MutatingScope $scope, Expr $expr): MutatingScope { - return $this->lookForExpressionCallback($scope, $expr, static fn (MutatingScope $scope, Expr $expr): MutatingScope => $scope->setAllowedUndefinedExpression($expr, $isAllowed)); + return $this->lookForExpressionCallback($scope, $expr, static fn (MutatingScope $scope, Expr $expr): MutatingScope => $scope->setAllowedUndefinedExpression($expr)); } private function lookForUnsetAllowedUndefinedExpressions(MutatingScope $scope, Expr $expr): MutatingScope @@ -2309,7 +2309,7 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void { ); } elseif ($expr instanceof Coalesce) { $nonNullabilityResult = $this->ensureNonNullability($scope, $expr->left, false); - $condScope = $this->lookForSetAllowedUndefinedExpressions($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->lookForUnsetAllowedUndefinedExpressions($scope, $expr->left); @@ -2383,7 +2383,7 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void { } } elseif ($expr instanceof Expr\Empty_) { $nonNullabilityResult = $this->ensureNonNullability($scope, $expr->expr, true); - $scope = $this->lookForSetAllowedUndefinedExpressions($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(); @@ -2396,7 +2396,7 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void { $nonNullabilityResults = []; foreach ($expr->vars as $var) { $nonNullabilityResult = $this->ensureNonNullability($scope, $var, true); - $scope = $this->lookForSetAllowedUndefinedExpressions($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(); @@ -3454,7 +3454,7 @@ static function (): void { if ($arrayItem->value instanceof ArrayDimFetch && $arrayItem->value->dim === null) { $itemScope = $itemScope->enterExpressionAssign($arrayItem->value); } - $itemScope = $this->lookForSetAllowedUndefinedExpressions($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 index 7360e69533..bfb0912e9a 100644 --- a/src/Analyser/ScopeFactory.php +++ b/src/Analyser/ScopeFactory.php @@ -17,7 +17,7 @@ interface ScopeFactory * @param VariableTypeHolder[] $moreSpecificTypes * @param array $conditionalExpressions * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions + * @param array $currentlyAllowedUndefinedExpressions * @param array $nativeExpressionTypes * @param array $inFunctionCallsStack * From 370b6497b88da44a19117cf138aea4954e17fb45 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Wed, 20 Apr 2022 22:08:47 +0900 Subject: [PATCH 05/10] cs fix --- src/Testing/RuleTestCase.php | 2 +- src/Testing/TypeInferenceTestCase.php | 2 +- tests/PHPStan/Analyser/AnalyserTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index d4f7687652..a9baed6595 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -66,7 +66,7 @@ private function getAnalyser(): Analyser [], [], true, - false + false, ); $fileAnalyser = new FileAnalyser( $this->createScopeFactory($reflectionProvider, $typeSpecifier), diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index ca135b6c53..19300427f8 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -56,7 +56,7 @@ public function processFile( $this->getEarlyTerminatingMethodCalls(), $this->getEarlyTerminatingFunctionCalls(), true, - false + false, ); $resolver->setAnalysedFiles(array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], $this->getAdditionalAnalysedFiles()))); diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 01621dae56..5fa6afc894 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -486,7 +486,7 @@ private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser [], [], true, - false + false, ); $lexer = new Lexer(['usedAttributes' => ['comments', 'startLine', 'endLine', 'startTokenPos', 'endTokenPos']]); $fileAnalyser = new FileAnalyser( From a876c5a339b5f6648b953dd1249fc3eb6910ead9 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Wed, 20 Apr 2022 22:21:09 +0900 Subject: [PATCH 06/10] fix duplicate class name --- tests/PHPStan/Rules/Properties/data/bug-3171.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Properties/data/bug-3171.php b/tests/PHPStan/Rules/Properties/data/bug-3171.php index f42c794a5b..e7e90f7fdd 100644 --- a/tests/PHPStan/Rules/Properties/data/bug-3171.php +++ b/tests/PHPStan/Rules/Properties/data/bug-3171.php @@ -7,7 +7,7 @@ class PropertyClass { public $someArray; } -class HelloWorld +class Foo { /** @var PropertyClass|null */ private $property; From f12c7ca4aaa13c2ac53bb113a6c1cf47f4c8fa1c Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Thu, 21 Apr 2022 09:00:55 +0900 Subject: [PATCH 07/10] move checkDynamicProperties to AccessPropertiesRule --- conf/config.level0.neon | 1 + conf/config.neon | 1 - src/Analyser/NodeScopeResolver.php | 6 +----- src/Rules/Properties/AccessPropertiesRule.php | 10 ++++++++-- src/Testing/RuleTestCase.php | 1 - src/Testing/TypeInferenceTestCase.php | 1 - tests/PHPStan/Analyser/AnalyserTest.php | 1 - .../Properties/AccessPropertiesInAssignRuleTest.php | 2 +- .../Rules/Properties/AccessPropertiesRuleTest.php | 2 +- 9 files changed, 12 insertions(+), 13 deletions(-) diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 45fbd90efd..2da06d326a 100644 --- 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 index 2177ac737d..cafbbebe0b 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -500,7 +500,6 @@ services: earlyTerminatingMethodCalls: %earlyTerminatingMethodCalls% earlyTerminatingFunctionCalls: %earlyTerminatingFunctionCalls% implicitThrows: %exceptions.implicitThrows% - checkDynamicProperties: %checkDynamicProperties% - class: PHPStan\Analyser\ConstantResolver diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 4c564d61dd..c1e8b62bd6 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -193,7 +193,6 @@ public function __construct( private array $earlyTerminatingMethodCalls, private array $earlyTerminatingFunctionCalls, private bool $implicitThrows, - private bool $checkDynamicProperties, ) { $earlyTerminatingMethodNames = []; @@ -1529,10 +1528,7 @@ private function lookForUnsetAllowedUndefinedExpressions(MutatingScope $scope, E */ private function lookForExpressionCallback(MutatingScope $scope, Expr $expr, Closure $callback): MutatingScope { - if ( - (!$expr instanceof ArrayDimFetch || $expr->dim !== null) && - (!$expr instanceof PropertyFetch && !$expr instanceof Expr\NullsafePropertyFetch || !$this->checkDynamicProperties) - ) { + if (!$expr instanceof ArrayDimFetch || $expr->dim !== null) { $scope = $callback($scope, $expr); } diff --git a/src/Rules/Properties/AccessPropertiesRule.php b/src/Rules/Properties/AccessPropertiesRule.php index ad713ac7c8..7c389eff86 100644 --- 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, ) { } @@ -78,7 +79,7 @@ private function processSingleProperty(Scope $scope, PropertyFetch $node, string return []; } - if ($type->canAccessProperties()->no() || $type->canAccessProperties()->maybe() && !$scope->isUndefinedExpressionAllowed($node)) { + if ($type->canAccessProperties()->no() || $type->canAccessProperties()->maybe() && !$this->canAccessUndefinedProperties($scope, $node)) { return [ RuleErrorBuilder::message(sprintf( 'Cannot access property $%s on %s.', @@ -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/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index a9baed6595..9751ad1fb1 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -66,7 +66,6 @@ private function getAnalyser(): Analyser [], [], true, - false, ); $fileAnalyser = new FileAnalyser( $this->createScopeFactory($reflectionProvider, $typeSpecifier), diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index 19300427f8..948c4f0084 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -56,7 +56,6 @@ public function processFile( $this->getEarlyTerminatingMethodCalls(), $this->getEarlyTerminatingFunctionCalls(), true, - false, ); $resolver->setAnalysedFiles(array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], $this->getAdditionalAnalysedFiles()))); diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 5fa6afc894..275a7849b6 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -486,7 +486,6 @@ private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser [], [], true, - false, ); $lexer = new Lexer(['usedAttributes' => ['comments', 'startLine', 'endLine', 'startTokenPos', 'endTokenPos']]); $fileAnalyser = new FileAnalyser( diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php index e1a4f4b73e..861bc158be 100644 --- 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 index 1d3e03741e..a3c98704cb 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -20,7 +20,7 @@ class AccessPropertiesRuleTest extends RuleTestCase protected function getRule(): Rule { $reflectionProvider = $this->createReflectionProvider(); - return new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, $this->checkThisOnly, $this->checkUnionTypes, false), true); + return new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, $this->checkThisOnly, $this->checkUnionTypes, false), true, true); } public function testAccessProperties(): void From 0f5a31caa1a5172a0d6ccf92e0ca2be71efe77aa Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Thu, 21 Apr 2022 09:06:39 +0900 Subject: [PATCH 08/10] configure checkDynamicProperties in test --- .../Properties/AccessPropertiesRuleTest.php | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index a3c98704cb..d4b789f348 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -17,16 +17,19 @@ class AccessPropertiesRuleTest extends RuleTestCase private bool $checkUnionTypes; + private bool $checkDynamicProperties; + protected function getRule(): Rule { $reflectionProvider = $this->createReflectionProvider(); - return new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, $this->checkThisOnly, $this->checkUnionTypes, false), true, true); + return new AccessPropertiesRule($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, $this->checkThisOnly, $this->checkUnionTypes, false), true, $this->checkDynamicProperties); } public function testAccessProperties(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse( [__DIR__ . '/data/access-properties.php'], [ @@ -157,6 +160,7 @@ public function testAccessPropertiesWithoutUnionTypes(): void { $this->checkThisOnly = false; $this->checkUnionTypes = false; + $this->checkDynamicProperties = false; $this->analyse( [__DIR__ . '/data/access-properties.php'], [ @@ -270,6 +274,7 @@ public function testRuleAssignOp(): void } $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/access-properties-assign-op.php'], [ [ 'Access to an undefined property TestAccessProperties\AssignOpNonexistentProperty::$flags.', @@ -282,6 +287,7 @@ public function testAccessPropertiesOnThisOnly(): void { $this->checkThisOnly = true; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse( [__DIR__ . '/data/access-properties.php'], [ @@ -301,6 +307,7 @@ public function testAccessPropertiesAfterIsNullInBooleanOr(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/access-properties-after-isnull.php'], [ [ 'Cannot access property $fooProperty on null.', @@ -341,6 +348,7 @@ public function testDateIntervalChildProperties(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/date-interval-child-properties.php'], [ [ 'Access to an undefined property AccessPropertiesDateIntervalChild\DateIntervalChild::$nonexistent.', @@ -353,6 +361,7 @@ public function testClassExists(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/access-properties-class-exists.php'], [ [ @@ -382,6 +391,7 @@ public function testMixin(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/mixin.php'], [ [ 'Access to an undefined property MixinProperties\GenericFoo::$namee.', @@ -394,6 +404,7 @@ public function testBug3947(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-3947.php'], []); } @@ -405,6 +416,7 @@ public function testNullSafe(): void $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/nullsafe-property-fetch.php'], [ [ @@ -434,6 +446,7 @@ public function testBug3371(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-3371.php'], []); } @@ -445,6 +458,7 @@ public function testBug4527(): void $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-4527.php'], []); } @@ -452,6 +466,7 @@ public function testBug4808(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-4808.php'], []); } @@ -463,6 +478,7 @@ public function testBug5868(): void } $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-5868.php'], [ [ 'Cannot access property $child on Bug5868PropertyFetch\Foo|null.', @@ -491,6 +507,7 @@ public function testBug6385(): void $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-6385.php'], [ [ 'Access to an undefined property UnitEnum::$value.', @@ -510,6 +527,7 @@ public function testBug6566(): void } $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-6566.php'], []); } @@ -517,6 +535,7 @@ public function testBug6899(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-6899.php'], [ [ 'Cannot access property $prop on string.', @@ -537,6 +556,7 @@ public function testBug6026(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-6026.php'], []); } @@ -544,6 +564,7 @@ public function testBug3659(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-3659.php'], []); } @@ -551,14 +572,15 @@ public function testDynamicProperties(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/dynamic-properties.php'], []); } - public function testBug4559(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-4559.php'], []); } @@ -566,6 +588,7 @@ public function testBug3171(): void { $this->checkThisOnly = false; $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; $this->analyse([__DIR__ . '/data/bug-3171.php'], []); } From e64bc9a87f6b7b30722b853c2ff8cf5a78e24013 Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Thu, 21 Apr 2022 09:33:01 +0900 Subject: [PATCH 09/10] add tests for checkDynamicProperties --- .../Properties/AccessPropertiesRuleTest.php | 248 ++++++++++++++++++ 1 file changed, 248 insertions(+) diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index d4b789f348..902127910b 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -303,6 +303,221 @@ public function testAccessPropertiesOnThisOnly(): void ); } + public function testAccessPropertiesOnDynamicProperties(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = true; + $this->checkDynamicProperties = true; + $this->analyse( + [__DIR__ . '/data/access-properties.php'], + [ + [ + 'Access to an undefined property TestAccessProperties\BarAccessProperties::$loremipsum.', + 23, + ], + [ + 'Access to private property $foo of parent class TestAccessProperties\FooAccessProperties.', + 24, + ], + [ + 'Cannot access property $propertyOnString on string.', + 31, + ], + [ + 'Access to private property TestAccessProperties\FooAccessProperties::$foo.', + 42, + ], + [ + 'Access to protected property TestAccessProperties\FooAccessProperties::$bar.', + 43, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$baz.', + 45, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$baz.', + 48, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$baz.', + 49, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$nonexistent.', + 51, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$nonexistent.', + 52, + ], + [ + 'Access to private property TestAccessProperties\FooAccessProperties::$foo.', + 58, + ], + [ + 'Access to protected property TestAccessProperties\FooAccessProperties::$bar.', + 59, + ], + [ + 'Access to property $foo on an unknown class TestAccessProperties\UnknownClass.', + 63, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$emptyBaz.', + 65, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$emptyBaz.', + 68, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$emptyNonexistent.', + 69, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$emptyNonexistent.', + 70, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherNonexistent.', + 75, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherNonexistent.', + 76, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherNonexistent.', + 76, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherNonexistent.', + 77, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherNonexistent.', + 77, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherNonexistent.', + 78, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherEmptyNonexistent.', + 80, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherEmptyNonexistent.', + 80, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherEmptyNonexistent.', + 81, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherEmptyNonexistent.', + 82, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherEmptyNonexistent.', + 83, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$anotherEmptyNonexistent.', + 83, + ], + [ + 'Access to property $test on an unknown class TestAccessProperties\FirstUnknownClass.', + 146, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'Access to property $test on an unknown class TestAccessProperties\SecondUnknownClass.', + 146, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'Access to an undefined property TestAccessProperties\WithFooAndBarProperty|TestAccessProperties\WithFooProperty::$bar.', + 176, + ], + [ + 'Access to an undefined property TestAccessProperties\SomeInterface&TestAccessProperties\WithFooProperty::$bar.', + 193, + ], + [ + 'Cannot access property $ipsum on TestAccessProperties\FooAccessProperties|null.', + 207, + ], + [ + 'Cannot access property $foo on null.', + 220, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$lorem.', + 247, + ], + [ + 'Access to an undefined property TestAccessProperties\FooAccessProperties::$dolor.', + 250, + ], + [ + 'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.', + 264, + ], + [ + 'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.', + 266, + ], + [ + 'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.', + 270, + ], + [ + 'Cannot access property $bar on TestAccessProperties\NullCoalesce|null.', + 272, + ], + [ + 'Cannot access property $foo on TestAccessProperties\NullCoalesce|null.', + 272, + ], + [ + 'Cannot access property $foo on TestAccessProperties\NullCoalesce|null.', + 272, + ], + [ + 'Access to an undefined property TestAccessProperties\IssetPropertyInWhile::$foo.', + 282, + ], + [ + 'Access to an undefined property class@anonymous/tests/PHPStan/Rules/Properties/data/access-properties.php:294::$barProperty.', + 299, + ], + [ + 'Cannot access property $foo on TestAccessProperties\PropertyIssetOnPossibleFalse|false.', + 315, + ], + [ + 'Access to an undefined property TestAccessProperties\AccessInIsset::$foo.', + 379, + ], + [ + 'Access to an undefined property TestAccessProperties\AccessInIsset::$foo.', + 386, + ], + [ + 'Cannot access property $selfOrNull on TestAccessProperties\RevertNonNullabilityForIsset|null.', + 402, + ], + [ + 'Cannot access property $array on stdClass|null.', + 412, + ], + ], + ); + } + public function testAccessPropertiesAfterIsNullInBooleanOr(): void { $this->checkThisOnly = false; @@ -576,6 +791,39 @@ public function testDynamicProperties(): void $this->analyse([__DIR__ . '/data/dynamic-properties.php'], []); } + public function testCheckDynamicProperties(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = true; + $this->checkDynamicProperties = true; + $this->analyse([__DIR__ . '/data/dynamic-properties.php'], [ + [ + 'Access to an undefined property DynamicProperties\Foo::$dynamicProperty.', + 9, + ], + [ + 'Access to an undefined property DynamicProperties\Foo::$dynamicProperty.', + 10, + ], + [ + 'Access to an undefined property DynamicProperties\Foo::$dynamicProperty.', + 11, + ], + [ + 'Access to an undefined property DynamicProperties\Bar::$dynamicProperty.', + 14, + ], + [ + 'Access to an undefined property DynamicProperties\Bar::$dynamicProperty.', + 15, + ], + [ + 'Access to an undefined property DynamicProperties\Bar::$dynamicProperty.', + 16, + ], + ]); + } + public function testBug4559(): void { $this->checkThisOnly = false; From a624060bffaa182b05cacfe9e67b22169f359f5c Mon Sep 17 00:00:00 2001 From: Yohta Kimura Date: Thu, 21 Apr 2022 10:37:40 +0900 Subject: [PATCH 10/10] `Access to an undefined property` should be appropriate for these cases. Should be fixed by specifying types in isset. Related issue https://github.com/phpstan/phpstan/issues/3171 --- src/Rules/Properties/AccessPropertiesRule.php | 2 +- tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Rules/Properties/AccessPropertiesRule.php b/src/Rules/Properties/AccessPropertiesRule.php index 7c389eff86..55d2e237f2 100644 --- a/src/Rules/Properties/AccessPropertiesRule.php +++ b/src/Rules/Properties/AccessPropertiesRule.php @@ -79,7 +79,7 @@ private function processSingleProperty(Scope $scope, PropertyFetch $node, string return []; } - if ($type->canAccessProperties()->no() || $type->canAccessProperties()->maybe() && !$this->canAccessUndefinedProperties($scope, $node)) { + if ($type->canAccessProperties()->no() || $type->canAccessProperties()->maybe() && !$scope->isUndefinedExpressionAllowed($node)) { return [ RuleErrorBuilder::message(sprintf( 'Cannot access property $%s on %s.', diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index 902127910b..1ee7b1aec8 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -495,7 +495,7 @@ public function testAccessPropertiesOnDynamicProperties(): void 299, ], [ - 'Cannot access property $foo on TestAccessProperties\PropertyIssetOnPossibleFalse|false.', + 'Access to an undefined property TestAccessProperties\PropertyIssetOnPossibleFalse|false::$foo.', 315, ], [ @@ -511,7 +511,7 @@ public function testAccessPropertiesOnDynamicProperties(): void 402, ], [ - 'Cannot access property $array on stdClass|null.', + 'Access to an undefined property stdClass|null::$array.', 412, ], ],