From 47825565dcab35e94943091ca1b270c8c0d17c65 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Wed, 6 Aug 2025 17:42:20 +0200 Subject: [PATCH 1/6] fix: dynamic variable error on isset --- src/Rules/IssetCheck.php | 8 ++++++++ tests/PHPStan/Rules/Variables/IssetRuleTest.php | 6 ++++++ tests/PHPStan/Rules/Variables/data/dynamic-variable.php | 7 +++++++ 3 files changed, 21 insertions(+) create mode 100644 tests/PHPStan/Rules/Variables/data/dynamic-variable.php diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 28b7780543..6c99040816 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -41,6 +41,14 @@ public function __construct( */ public function check(Expr $expr, Scope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?IdentifierRuleError $error = null): ?IdentifierRuleError { + // [FIX] Ignore isset($$foo): dynamic variable + if ( + $expr instanceof Node\Expr\Variable && + $expr->name instanceof Node\Expr\Variable + ) { + return null; + } + // mirrored in PHPStan\Analyser\MutatingScope::issetCheck() if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) { $hasVariable = $scope->hasVariableType($expr->name); diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index 0acd26902a..3492426382 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -485,4 +485,10 @@ public function testIssetAfterRememberedConstructor(): void ]); } + public function testDynamicVariable(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/dynamic-variable.php'], []); + } } diff --git a/tests/PHPStan/Rules/Variables/data/dynamic-variable.php b/tests/PHPStan/Rules/Variables/data/dynamic-variable.php new file mode 100644 index 0000000000..34815b84ef --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/dynamic-variable.php @@ -0,0 +1,7 @@ + Date: Thu, 7 Aug 2025 08:42:01 +0200 Subject: [PATCH 2/6] fix: issue in defined variable rule instead of isset --- src/Rules/IssetCheck.php | 8 -------- src/Rules/Variables/DefinedVariableRule.php | 7 ++++++- .../PHPStan/Rules/Variables/DefinedVariableRuleTest.php | 9 +++++++++ tests/PHPStan/Rules/Variables/IssetRuleTest.php | 7 ------- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 6c99040816..28b7780543 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -41,14 +41,6 @@ public function __construct( */ public function check(Expr $expr, Scope $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, ?IdentifierRuleError $error = null): ?IdentifierRuleError { - // [FIX] Ignore isset($$foo): dynamic variable - if ( - $expr instanceof Node\Expr\Variable && - $expr->name instanceof Node\Expr\Variable - ) { - return null; - } - // mirrored in PHPStan\Analyser\MutatingScope::issetCheck() if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) { $hasVariable = $scope->hasVariableType($expr->name); diff --git a/src/Rules/Variables/DefinedVariableRule.php b/src/Rules/Variables/DefinedVariableRule.php index 8fbb1e5d0b..f33e371fbe 100644 --- a/src/Rules/Variables/DefinedVariableRule.php +++ b/src/Rules/Variables/DefinedVariableRule.php @@ -41,6 +41,11 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { $errors = []; + + if ($scope->isUndefinedExpressionAllowed($node)) { + return []; + } + if (is_string($node->name)) { $variableNameScopes = [$node->name => $scope]; } else { @@ -78,7 +83,7 @@ private function processSingleVariable(Scope $scope, Variable $node, string $var } } - if ($scope->isInExpressionAssign($node) || $scope->isUndefinedExpressionAllowed($node)) { + if ($scope->isInExpressionAssign($node)) { return []; } diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 96b89cf070..6460b2fff8 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1169,4 +1169,13 @@ public function testBug8719(): void $this->analyse([__DIR__ . '/data/bug-8719.php'], []); } + public function testDynamicVariable(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = true; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + + $this->analyse([__DIR__ . '/data/dynamic-variable.php'], []); + } } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index 3492426382..cbcd119ffd 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -484,11 +484,4 @@ public function testIssetAfterRememberedConstructor(): void ], ]); } - - public function testDynamicVariable(): void - { - $this->treatPhpDocTypesAsCertain = true; - - $this->analyse([__DIR__ . '/data/dynamic-variable.php'], []); - } } From cfe14f4e843a8354157638de0920e12e7087df44 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Thu, 7 Aug 2025 08:50:35 +0200 Subject: [PATCH 3/6] fix: cs fixer --- tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php | 1 + tests/PHPStan/Rules/Variables/IssetRuleTest.php | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 6460b2fff8..c430114012 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1178,4 +1178,5 @@ public function testDynamicVariable(): void $this->analyse([__DIR__ . '/data/dynamic-variable.php'], []); } + } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index cbcd119ffd..0acd26902a 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -484,4 +484,5 @@ public function testIssetAfterRememberedConstructor(): void ], ]); } + } From 3182d3ed5f3b37c580dea290f30806ec2d961bf3 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Thu, 7 Aug 2025 09:04:59 +0200 Subject: [PATCH 4/6] feat: add more tests --- .../PHPStan/Rules/Variables/DefinedVariableRuleTest.php | 9 +++++++-- ...ic-variable.php => dynamic-variable-inside-isset.php} | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) rename tests/PHPStan/Rules/Variables/data/{dynamic-variable.php => dynamic-variable-inside-isset.php} (87%) diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index c430114012..259d6c5563 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1169,14 +1169,19 @@ public function testBug8719(): void $this->analyse([__DIR__ . '/data/bug-8719.php'], []); } - public function testDynamicVariable(): void + public function testDynamicVariableInsideIsset(): void { $this->cliArgumentsVariablesRegistered = true; $this->polluteScopeWithLoopInitialAssignments = true; $this->checkMaybeUndefinedVariables = true; $this->polluteScopeWithAlwaysIterableForeach = true; - $this->analyse([__DIR__ . '/data/dynamic-variable.php'], []); + $this->analyse([__DIR__ . '/data/dynamic-variable-inside-isset.php'], [ + [ + 'Variable $bar might not be defined.', + 9, + ], + ]); } } diff --git a/tests/PHPStan/Rules/Variables/data/dynamic-variable.php b/tests/PHPStan/Rules/Variables/data/dynamic-variable-inside-isset.php similarity index 87% rename from tests/PHPStan/Rules/Variables/data/dynamic-variable.php rename to tests/PHPStan/Rules/Variables/data/dynamic-variable-inside-isset.php index 34815b84ef..cf66bfe5a6 100644 --- a/tests/PHPStan/Rules/Variables/data/dynamic-variable.php +++ b/tests/PHPStan/Rules/Variables/data/dynamic-variable-inside-isset.php @@ -5,3 +5,5 @@ if (!isset($$foo)) { echo 'Wololo'; } + +echo $$foo; From aba6f4953657604318fb7038dec8b3b7bd9c4bcd Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Thu, 7 Aug 2025 09:06:57 +0200 Subject: [PATCH 5/6] fix: rename tests files and add namespace according to review --- tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php | 4 ++-- .../data/{dynamic-variable-inside-isset.php => bug-13353.php} | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) rename tests/PHPStan/Rules/Variables/data/{dynamic-variable-inside-isset.php => bug-13353.php} (82%) diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 259d6c5563..9d564c10fd 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1169,14 +1169,14 @@ public function testBug8719(): void $this->analyse([__DIR__ . '/data/bug-8719.php'], []); } - public function testDynamicVariableInsideIsset(): void + public function testBug13353(): void { $this->cliArgumentsVariablesRegistered = true; $this->polluteScopeWithLoopInitialAssignments = true; $this->checkMaybeUndefinedVariables = true; $this->polluteScopeWithAlwaysIterableForeach = true; - $this->analyse([__DIR__ . '/data/dynamic-variable-inside-isset.php'], [ + $this->analyse([__DIR__ . '/data/bug-13353.php'], [ [ 'Variable $bar might not be defined.', 9, diff --git a/tests/PHPStan/Rules/Variables/data/dynamic-variable-inside-isset.php b/tests/PHPStan/Rules/Variables/data/bug-13353.php similarity index 82% rename from tests/PHPStan/Rules/Variables/data/dynamic-variable-inside-isset.php rename to tests/PHPStan/Rules/Variables/data/bug-13353.php index cf66bfe5a6..ef13d0635f 100644 --- a/tests/PHPStan/Rules/Variables/data/dynamic-variable-inside-isset.php +++ b/tests/PHPStan/Rules/Variables/data/bug-13353.php @@ -1,5 +1,7 @@ Date: Thu, 7 Aug 2025 09:13:56 +0200 Subject: [PATCH 6/6] fix: tests from review --- tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 9d564c10fd..c1b910789a 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1179,7 +1179,7 @@ public function testBug13353(): void $this->analyse([__DIR__ . '/data/bug-13353.php'], [ [ 'Variable $bar might not be defined.', - 9, + 11, ], ]); }