From 0fe2f51b3557c85a404dc8be82c3baccff054615 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Wed, 6 Aug 2025 17:42:20 +0200 Subject: [PATCH 01/10] fix: dynamic variable error on isset --- src/Rules/IssetCheck.php | 8 ++++++++ tests/PHPStan/Rules/Variables/IssetRuleTest.php | 8 +++++++- tests/PHPStan/Rules/Variables/data/dynamic-variable.php | 7 +++++++ 3 files changed, 22 insertions(+), 1 deletion(-) 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 40b837c134..50cc6a9272 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -492,6 +492,13 @@ public function testIssetAfterRememberedConstructor(): void ]); } + public function testDynamicVariable(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/dynamic-variable.php'], []); + } + public function testPr4374(): void { $this->treatPhpDocTypesAsCertain = true; @@ -510,5 +517,4 @@ public function testBug10640(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-10640.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 02/10] 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 50cc6a9272..7c1eca91f0 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -492,13 +492,6 @@ public function testIssetAfterRememberedConstructor(): void ]); } - public function testDynamicVariable(): void - { - $this->treatPhpDocTypesAsCertain = true; - - $this->analyse([__DIR__ . '/data/dynamic-variable.php'], []); - } - public function testPr4374(): void { $this->treatPhpDocTypesAsCertain = true; From 444958a6510c1a54e3a8ca38548ed539182c71f7 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Thu, 7 Aug 2025 08:50:35 +0200 Subject: [PATCH 03/10] fix: cs fixer --- tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php | 1 + 1 file changed, 1 insertion(+) 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'], []); } + } From fc3b2bef0f028c81d41e15b00c1aecba23dd973f Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Thu, 7 Aug 2025 09:04:59 +0200 Subject: [PATCH 04/10] 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 198b9a388e2988f827786497f30d96bc0786e4f6 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Thu, 7 Aug 2025 09:06:57 +0200 Subject: [PATCH 05/10] 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 06/10] 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, ], ]); } From 8737a20c97722e4cce2176c170067cbe0d3b1bf5 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Mon, 1 Sep 2025 17:45:30 +0200 Subject: [PATCH 07/10] fix: add tests from review --- .../Rules/Variables/DefinedVariableRuleTest.php | 4 ++++ tests/PHPStan/Rules/Variables/data/bug-13353.php | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index c1b910789a..9a39ce7433 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1181,6 +1181,10 @@ public function testBug13353(): void 'Variable $bar might not be defined.', 11, ], + [ + 'Undefined variable: $bar', + 20 + ] ]); } diff --git a/tests/PHPStan/Rules/Variables/data/bug-13353.php b/tests/PHPStan/Rules/Variables/data/bug-13353.php index ef13d0635f..36d644925f 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-13353.php +++ b/tests/PHPStan/Rules/Variables/data/bug-13353.php @@ -9,3 +9,13 @@ } echo $$foo; + +function () { + $foo = 'bar'; + + if (!isset($$foo)) { + echo 'Wololo'; + } + + echo $$foo; +}; From cb08ebeaa427ef23a64c1bb779b4cb4d642e7cfd Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Mon, 1 Sep 2025 17:59:49 +0200 Subject: [PATCH 08/10] fix: cs fixer --- tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 9a39ce7433..a30bb0cb09 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1183,8 +1183,8 @@ public function testBug13353(): void ], [ 'Undefined variable: $bar', - 20 - ] + 20, + ], ]); } From bc536d8609c794f884f25c375a001190079a11e5 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Tue, 2 Sep 2025 09:15:57 +0200 Subject: [PATCH 09/10] fix: add tests on isset rules --- tests/PHPStan/Rules/Variables/IssetRuleTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index 7c1eca91f0..c4e4aa181e 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -510,4 +510,11 @@ public function testBug10640(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-10640.php'], []); } + + public function testBug13353(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/bug-13353.php'], []); + } } From 8270c51273f7b2d4e64e5368208220b1b503351c Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Tue, 2 Sep 2025 09:25:44 +0200 Subject: [PATCH 10/10] fix: cs fixer --- tests/PHPStan/Rules/Variables/IssetRuleTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index c4e4aa181e..8c0045e86f 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -517,4 +517,5 @@ public function testBug13353(): void $this->analyse([__DIR__ . '/data/bug-13353.php'], []); } + }