From b7fba7516405c97e6439945d079adf979d14ef4d Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Tue, 18 Mar 2025 22:29:58 +0900 Subject: [PATCH 1/7] Support dynamic name expressions in rules --- src/Rules/Classes/ClassConstantRule.php | 23 ++++++++++++-- src/Rules/Methods/CallMethodsRule.php | 23 ++++++++++++-- src/Rules/Methods/CallStaticMethodsRule.php | 24 +++++++++++++-- src/Rules/Variables/DefinedVariableRule.php | 34 ++++++++++++++++----- 4 files changed, 88 insertions(+), 16 deletions(-) diff --git a/src/Rules/Classes/ClassConstantRule.php b/src/Rules/Classes/ClassConstantRule.php index 968b3d75f3..8f92b5908c 100644 --- a/src/Rules/Classes/ClassConstantRule.php +++ b/src/Rules/Classes/ClassConstantRule.php @@ -11,6 +11,7 @@ use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\ClassNameCheck; use PHPStan\Rules\ClassNameNodePair; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; @@ -20,6 +21,7 @@ use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use PHPStan\Type\VerbosityLevel; +use function array_map; use function array_merge; use function in_array; use function sprintf; @@ -47,11 +49,26 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!$node->name instanceof Node\Identifier) { - return []; + $errors = []; + if ($node->name instanceof Node\Identifier) { + $constantNames = [$node->name->name]; + } else { + $fetchType = $scope->getType($node->name); + $constantNames = array_map(static fn ($type): string => $type->getValue(), $fetchType->getConstantStrings()); + } + + foreach ($constantNames as $constantName) { + $errors = array_merge($errors, $this->processSingleClassConstFetch($scope, $node, $constantName)); } - $constantName = $node->name->name; + return $errors; + } + + /** + * @return list + */ + private function processSingleClassConstFetch(Scope $scope, ClassConstFetch $node, string $constantName): array + { $class = $node->class; $messages = []; if ($class instanceof Node\Name) { diff --git a/src/Rules/Methods/CallMethodsRule.php b/src/Rules/Methods/CallMethodsRule.php index 19bc52fe80..e2a073a3de 100644 --- a/src/Rules/Methods/CallMethodsRule.php +++ b/src/Rules/Methods/CallMethodsRule.php @@ -8,7 +8,10 @@ use PHPStan\Internal\SprintfHelper; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Rules\FunctionCallParametersCheck; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; +use PHPStan\Type\Constant\ConstantStringType; +use function array_map; use function array_merge; /** @@ -31,12 +34,26 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!$node->name instanceof Node\Identifier) { - return []; + $errors = []; + if ($node->name instanceof Node\Identifier) { + $methodNames = [$node->name->name]; + } else { + $callType = $scope->getType($node->name); + $methodNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $callType->getConstantStrings()); } - $methodName = $node->name->name; + foreach ($methodNames as $methodName) { + $errors = array_merge($errors, $this->processSingleMethodCall($scope, $node, $methodName)); + } + + return $errors; + } + /** + * @return list + */ + private function processSingleMethodCall(Scope $scope, MethodCall $node, string $methodName): array + { [$errors, $methodReflection] = $this->methodCallCheck->check($scope, $methodName, $node->var); if ($methodReflection === null) { return $errors; diff --git a/src/Rules/Methods/CallStaticMethodsRule.php b/src/Rules/Methods/CallStaticMethodsRule.php index e6b2c9dbb5..882d645f61 100644 --- a/src/Rules/Methods/CallStaticMethodsRule.php +++ b/src/Rules/Methods/CallStaticMethodsRule.php @@ -8,7 +8,10 @@ use PHPStan\Internal\SprintfHelper; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Rules\FunctionCallParametersCheck; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; +use PHPStan\Type\Constant\ConstantStringType; +use function array_map; use function array_merge; use function sprintf; @@ -32,11 +35,26 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!$node->name instanceof Node\Identifier) { - return []; + $errors = []; + if ($node->name instanceof Node\Identifier) { + $methodNames = [$node->name->name]; + } else { + $callType = $scope->getType($node->name); + $methodNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $callType->getConstantStrings()); } - $methodName = $node->name->name; + foreach ($methodNames as $methodName) { + $errors = array_merge($errors, $this->processSingleMethodCall($scope, $node, $methodName)); + } + + return $errors; + } + + /** + * @return list + */ + private function processSingleMethodCall(Scope $scope, StaticCall $node, string $methodName): array + { [$errors, $method] = $this->methodCallCheck->check($scope, $methodName, $node->class); if ($method === null) { return $errors; diff --git a/src/Rules/Variables/DefinedVariableRule.php b/src/Rules/Variables/DefinedVariableRule.php index fc665ed901..4b0186ae6f 100644 --- a/src/Rules/Variables/DefinedVariableRule.php +++ b/src/Rules/Variables/DefinedVariableRule.php @@ -5,8 +5,12 @@ use PhpParser\Node; use PhpParser\Node\Expr\Variable; use PHPStan\Analyser\Scope; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\Constant\ConstantStringType; +use function array_map; +use function array_merge; use function in_array; use function is_string; use function sprintf; @@ -31,11 +35,27 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!is_string($node->name)) { - return []; + $errors = []; + if (is_string($node->name)) { + $variableNames = [$node->name]; + } else { + $fetchType = $scope->getType($node->name); + $variableNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $fetchType->getConstantStrings()); + } + + foreach ($variableNames as $name) { + $errors = array_merge($errors, $this->processSingleVariable($scope, $node, $name)); } - if ($this->cliArgumentsVariablesRegistered && in_array($node->name, [ + return $errors; + } + + /** + * @return list + */ + private function processSingleVariable(Scope $scope, Variable $node, string $variableName): array + { + if ($this->cliArgumentsVariablesRegistered && in_array($variableName, [ 'argc', 'argv', ], true)) { @@ -49,18 +69,18 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($scope->hasVariableType($node->name)->no()) { + if ($scope->hasVariableType($variableName)->no()) { return [ - RuleErrorBuilder::message(sprintf('Undefined variable: $%s', $node->name)) + RuleErrorBuilder::message(sprintf('Undefined variable: $%s', $variableName)) ->identifier('variable.undefined') ->build(), ]; } elseif ( $this->checkMaybeUndefinedVariables - && !$scope->hasVariableType($node->name)->yes() + && !$scope->hasVariableType($variableName)->yes() ) { return [ - RuleErrorBuilder::message(sprintf('Variable $%s might not be defined.', $node->name)) + RuleErrorBuilder::message(sprintf('Variable $%s might not be defined.', $variableName)) ->identifier('variable.undefined') ->build(), ]; From b1f4ec62fb05c8974500aac18e7faf7cadb92527 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Wed, 19 Mar 2025 01:03:18 +0900 Subject: [PATCH 2/7] Add tests --- .../Rules/Classes/ClassConstantRuleTest.php | 20 ++++++++++ .../Classes/data/dynamic-constant-access.php | 23 +++++++++++ .../Rules/Methods/CallMethodsRuleTest.php | 23 +++++++++++ .../Methods/CallStaticMethodsRuleTest.php | 22 +++++++++++ .../Rules/Methods/data/dynamic-call.php | 38 +++++++++++++++++++ .../Variables/DefinedVariableRuleTest.php | 22 +++++++++++ .../Rules/Variables/data/dynamic-access.php | 21 ++++++++++ 7 files changed, 169 insertions(+) create mode 100644 tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php create mode 100644 tests/PHPStan/Rules/Methods/data/dynamic-call.php create mode 100644 tests/PHPStan/Rules/Variables/data/dynamic-access.php diff --git a/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php b/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php index 32086476be..388b3eed77 100644 --- a/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php @@ -435,4 +435,24 @@ public function testClassConstantAccessedOnTrait(): void ]); } + public function testDynamicAccess(): void + { + if (PHP_VERSION_ID < 80300) { + $this->markTestSkipped('Test requires PHP 8.3.'); + } + + $this->phpVersion = PHP_VERSION_ID; + + $this->analyse([__DIR__ . '/data/dynamic-constant-access.php'], [ + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::FOO.', + 20, + ], + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::BUZ.', + 20, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php b/tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php new file mode 100644 index 0000000000..fe3e8e73ea --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php @@ -0,0 +1,23 @@ += 8.3 + +namespace ClassConstantDynamicAccess; + +final class Foo +{ + + private const BAR = 'FOO'; + + /** @var 'FOO'|'BAR'|'BUZ' */ + public $name; + + public function test(string $string, object $obj): void + { + $bar = 'FOO'; + + echo self::{$foo}; + echo self::{$string}; + echo self::{$obj}; + echo self::{$this->name}; + } + +} diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index acbbd53dd3..e160a917bd 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -3552,4 +3552,27 @@ public function testBug6828(): void $this->analyse([__DIR__ . '/data/bug-6828.php'], []); } + public function testDynamicCall(): void + { + $this->checkThisOnly = false; + $this->checkNullables = true; + $this->checkUnionTypes = true; + $this->checkExplicitMixed = true; + + $this->analyse([__DIR__ . '/data/dynamic-call.php'], [ + [ + 'Call to an undefined method MethodsDynamicCall\Foo::bar().', + 23, + ], + [ + 'Call to an undefined method MethodsDynamicCall\Foo::bar().', + 26, + ], + [ + 'Call to an undefined method MethodsDynamicCall\Foo::buz().', + 26, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php index 51210125cf..3840c6e945 100644 --- a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php @@ -862,4 +862,26 @@ public function testBug12015(): void $this->analyse([__DIR__ . '/data/bug-12015.php'], []); } + public function testDynamicCall(): void + { + $this->checkThisOnly = false; + $this->checkExplicitMixed = true; + $this->checkImplicitMixed = true; + + $this->analyse([__DIR__ . '/data/dynamic-call.php'], [ + [ + 'Call to an undefined static method MethodsDynamicCall\Foo::bar().', + 33, + ], + [ + 'Call to an undefined static method MethodsDynamicCall\Foo::bar().', + 36, + ], + [ + 'Call to an undefined static method MethodsDynamicCall\Foo::buz().', + 36, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/dynamic-call.php b/tests/PHPStan/Rules/Methods/data/dynamic-call.php new file mode 100644 index 0000000000..50ed28f073 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/dynamic-call.php @@ -0,0 +1,38 @@ +$foo(); + echo $this->$string(); + echo $this->$obj(); + echo $this->{self::$name}(); + } + + public function testStaticCall(string $string, object $obj): void + { + $foo = 'bar'; + + echo self::$foo(); + echo self::$string(); + echo self::$obj(); + echo self::{self::$name}(); + } +} diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 17aa83b245..21838fdf92 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1098,4 +1098,26 @@ public function testPropertyHooks(): void ]); } + public function testDynamicAccess(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = true; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/dynamic-access.php'], [ + [ + 'Undefined variable: $bar', + 15, + ], + [ + 'Undefined variable: $bar', + 18, + ], + [ + 'Undefined variable: $buz', + 18, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/dynamic-access.php b/tests/PHPStan/Rules/Variables/data/dynamic-access.php new file mode 100644 index 0000000000..3c7674a99b --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/dynamic-access.php @@ -0,0 +1,21 @@ +name}; + } + +} From 4e890a352710cdad6c7fadc0245ef8b61898c155 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Wed, 19 Mar 2025 01:50:01 +0900 Subject: [PATCH 3/7] Use scope by method name --- src/Rules/Methods/CallMethodsRule.php | 19 ++++++++---- src/Rules/Methods/CallStaticMethodsRule.php | 19 ++++++++---- .../Rules/Methods/CallMethodsRuleTest.php | 16 ++++++++-- .../Methods/CallStaticMethodsRuleTest.php | 16 ++++++++-- .../Rules/Methods/data/dynamic-call.php | 30 +++++++++++++++++-- 5 files changed, 83 insertions(+), 17 deletions(-) diff --git a/src/Rules/Methods/CallMethodsRule.php b/src/Rules/Methods/CallMethodsRule.php index e2a073a3de..748f28b24e 100644 --- a/src/Rules/Methods/CallMethodsRule.php +++ b/src/Rules/Methods/CallMethodsRule.php @@ -3,7 +3,9 @@ namespace PHPStan\Rules\Methods; use PhpParser\Node; +use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Internal\SprintfHelper; use PHPStan\Reflection\ParametersAcceptorSelector; @@ -11,6 +13,7 @@ use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Type\Constant\ConstantStringType; +use function array_column; use function array_map; use function array_merge; @@ -36,14 +39,20 @@ public function processNode(Node $node, Scope $scope): array { $errors = []; if ($node->name instanceof Node\Identifier) { - $methodNames = [$node->name->name]; + $methodNameScopes = [$node->name->name => $scope]; } else { - $callType = $scope->getType($node->name); - $methodNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $callType->getConstantStrings()); + $nameType = $scope->getType($node->name); + $methodNameScopes = array_column(array_map( + static fn (ConstantStringType $constantString) => [ + $name = $constantString->getValue(), + $scope->filterByTruthyValue(new Identical($node->name, new String_($name))), + ], + $nameType->getConstantStrings(), + ), 1, 0); } - foreach ($methodNames as $methodName) { - $errors = array_merge($errors, $this->processSingleMethodCall($scope, $node, $methodName)); + foreach ($methodNameScopes as $methodName => $methodScope) { + $errors = array_merge($errors, $this->processSingleMethodCall($methodScope, $node, $methodName)); } return $errors; diff --git a/src/Rules/Methods/CallStaticMethodsRule.php b/src/Rules/Methods/CallStaticMethodsRule.php index 882d645f61..adf32ac22c 100644 --- a/src/Rules/Methods/CallStaticMethodsRule.php +++ b/src/Rules/Methods/CallStaticMethodsRule.php @@ -3,7 +3,9 @@ namespace PHPStan\Rules\Methods; use PhpParser\Node; +use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Internal\SprintfHelper; use PHPStan\Reflection\ParametersAcceptorSelector; @@ -11,6 +13,7 @@ use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Type\Constant\ConstantStringType; +use function array_column; use function array_map; use function array_merge; use function sprintf; @@ -37,14 +40,20 @@ public function processNode(Node $node, Scope $scope): array { $errors = []; if ($node->name instanceof Node\Identifier) { - $methodNames = [$node->name->name]; + $methodNameScopes = [$node->name->name => $scope]; } else { - $callType = $scope->getType($node->name); - $methodNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $callType->getConstantStrings()); + $nameType = $scope->getType($node->name); + $methodNameScopes = array_column(array_map( + static fn (ConstantStringType $constantString) => [ + $name = $constantString->getValue(), + $scope->filterByTruthyValue(new Identical($node->name, new String_($name))), + ], + $nameType->getConstantStrings(), + ), 1, 0); } - foreach ($methodNames as $methodName) { - $errors = array_merge($errors, $this->processSingleMethodCall($scope, $node, $methodName)); + foreach ($methodNameScopes as $methodName => $methodScope) { + $errors = array_merge($errors, $this->processSingleMethodCall($methodScope, $node, $methodName)); } return $errors; diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index e160a917bd..e544eeeb5b 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -3565,13 +3565,25 @@ public function testDynamicCall(): void 23, ], [ - 'Call to an undefined method MethodsDynamicCall\Foo::bar().', + 'Call to an undefined method MethodsDynamicCall\Foo::doBar().', 26, ], [ - 'Call to an undefined method MethodsDynamicCall\Foo::buz().', + 'Call to an undefined method MethodsDynamicCall\Foo::doBuz().', 26, ], + [ + 'Parameter #1 $n of method MethodsDynamicCall\Foo::doFoo() expects int, int|string given.', + 53, + ], + [ + 'Parameter #1 $s of method MethodsDynamicCall\Foo::doQux() expects string, int given.', + 54, + ], + [ + 'Parameter #1 $n of method MethodsDynamicCall\Foo::doFoo() expects int, string given.', + 55, + ], ]); } diff --git a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php index 3840c6e945..2cfc7891b5 100644 --- a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php @@ -874,13 +874,25 @@ public function testDynamicCall(): void 33, ], [ - 'Call to an undefined static method MethodsDynamicCall\Foo::bar().', + 'Call to an undefined static method MethodsDynamicCall\Foo::doBar().', 36, ], [ - 'Call to an undefined static method MethodsDynamicCall\Foo::buz().', + 'Call to an undefined static method MethodsDynamicCall\Foo::doBuz().', 36, ], + [ + 'Parameter #1 $n of method MethodsDynamicCall\Foo::doFoo() expects int, int|string given.', + 58, + ], + [ + 'Parameter #1 $s of static method MethodsDynamicCall\Foo::doQux() expects string, int given.', + 59, + ], + [ + 'Parameter #1 $n of method MethodsDynamicCall\Foo::doFoo() expects int, string given.', + 60, + ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/dynamic-call.php b/tests/PHPStan/Rules/Methods/data/dynamic-call.php index 50ed28f073..3a917c0c81 100644 --- a/tests/PHPStan/Rules/Methods/data/dynamic-call.php +++ b/tests/PHPStan/Rules/Methods/data/dynamic-call.php @@ -5,14 +5,14 @@ final class Foo { - /** @var 'foo'|'bar'|'buz'|'qux' */ + /** @var 'doFoo'|'doBar'|'doBuz'|'doQux' */ public static $name; - public function foo(): void + public function doFoo(int $n = 0): void { } - public static function qux(): void + public static function doQux(string $s = 'default'): void { } @@ -35,4 +35,28 @@ public function testStaticCall(string $string, object $obj): void echo self::$obj(); echo self::{self::$name}(); } + + public function testScope(): void + { + $param1 = 1; + $param2 = 'str'; + $name1 = 'doFoo'; + if (rand(0, 1)) { + $name = $name1; + $param = $param1; + } else { + $name = 'doQux'; + $param = $param2; + } + + $this->$name($param); // ok + $this->$name1($param); + $this->$name($param1); + $this->$name($param2); + + self::$name($param); // ok + self::$name1($param); + self::$name($param1); + self::$name($param2); + } } From fc63761b61ce8eb0d05447dd58f6a396064071de Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Wed, 19 Mar 2025 02:29:31 +0900 Subject: [PATCH 4/7] Replaced with a simple foreach --- src/Rules/Methods/CallMethodsRule.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Rules/Methods/CallMethodsRule.php b/src/Rules/Methods/CallMethodsRule.php index 748f28b24e..68957aa2e5 100644 --- a/src/Rules/Methods/CallMethodsRule.php +++ b/src/Rules/Methods/CallMethodsRule.php @@ -12,9 +12,6 @@ use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; -use PHPStan\Type\Constant\ConstantStringType; -use function array_column; -use function array_map; use function array_merge; /** @@ -42,13 +39,11 @@ public function processNode(Node $node, Scope $scope): array $methodNameScopes = [$node->name->name => $scope]; } else { $nameType = $scope->getType($node->name); - $methodNameScopes = array_column(array_map( - static fn (ConstantStringType $constantString) => [ - $name = $constantString->getValue(), - $scope->filterByTruthyValue(new Identical($node->name, new String_($name))), - ], - $nameType->getConstantStrings(), - ), 1, 0); + $methodNameScopes = []; + foreach ($nameType->getConstantStrings() as $constantString) { + $name = $constantString->getValue(); + $methodNameScopes[$name] = $scope->filterByTruthyValue(new Identical($node->name, new String_($name))); + } } foreach ($methodNameScopes as $methodName => $methodScope) { From ebb3b6ffb656261fcde14c6a01113a83475700fb Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Wed, 19 Mar 2025 04:38:10 +0900 Subject: [PATCH 5/7] Fix variable scope --- src/Rules/Variables/DefinedVariableRule.php | 18 +++--- .../Variables/DefinedVariableRuleTest.php | 56 +++++++++++++++++++ .../Rules/Variables/data/dynamic-access.php | 33 +++++++++++ 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/src/Rules/Variables/DefinedVariableRule.php b/src/Rules/Variables/DefinedVariableRule.php index 4b0186ae6f..2c7163434c 100644 --- a/src/Rules/Variables/DefinedVariableRule.php +++ b/src/Rules/Variables/DefinedVariableRule.php @@ -3,13 +3,13 @@ namespace PHPStan\Rules\Variables; use PhpParser\Node; +use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use PHPStan\Type\Constant\ConstantStringType; -use function array_map; use function array_merge; use function in_array; use function is_string; @@ -37,14 +37,18 @@ public function processNode(Node $node, Scope $scope): array { $errors = []; if (is_string($node->name)) { - $variableNames = [$node->name]; + $variableNameScopes = [$node->name => $scope]; } else { - $fetchType = $scope->getType($node->name); - $variableNames = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $fetchType->getConstantStrings()); + $nameType = $scope->getType($node->name); + $variableNameScopes = []; + foreach ($nameType->getConstantStrings() as $constantString) { + $name = $constantString->getValue(); + $variableNameScopes[$name] = $scope->filterByTruthyValue(new Identical($node->name, new String_($name))); + } } - foreach ($variableNames as $name) { - $errors = array_merge($errors, $this->processSingleVariable($scope, $node, $name)); + foreach ($variableNameScopes as $name => $variableScope) { + $errors = array_merge($errors, $this->processSingleVariable($variableScope, $node, $name)); } return $errors; diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 21838fdf92..e6b29e2809 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1117,6 +1117,62 @@ public function testDynamicAccess(): void 'Undefined variable: $buz', 18, ], + [ + 'Variable $foo might not be defined.', + 36, + ], + [ + 'Variable $foo might not be defined.', + 37, + ], + [ + 'Variable $bar might not be defined.', + 38, + ], + [ + 'Variable $bar might not be defined.', + 40, + ], + [ + 'Variable $foo might not be defined.', + 41, + ], + [ + 'Variable $bar might not be defined.', + 42, + ], + [ + 'Undefined variable: $buz', + 44, + ], + [ + 'Undefined variable: $foo', + 45, + ], + [ + 'Undefined variable: $bar', + 46, + ], + [ + 'Undefined variable: $buz', + 49, + ], + [ + 'Variable $bar might not be defined.', + 49, + ], + [ + 'Variable $foo might not be defined.', + 49, + ], + [ + 'Variable $foo might not be defined.', + 50, + ], + [ + 'Variable $bar might not be defined.', + 51, + ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/dynamic-access.php b/tests/PHPStan/Rules/Variables/data/dynamic-access.php index 3c7674a99b..a1109c5858 100644 --- a/tests/PHPStan/Rules/Variables/data/dynamic-access.php +++ b/tests/PHPStan/Rules/Variables/data/dynamic-access.php @@ -18,4 +18,37 @@ public function test(string $string, object $obj): void echo ${$this->name}; } + public function testScope(): void + { + $name1 = 'foo'; + $rand = rand(); + if ($rand === 1) { + $foo = 1; + $name = $name1; + } elseif ($rand === 2) { + $name = 'bar'; + $bar = 'str'; + } else { + $name = 'buz'; + } + + if ($name === 'foo') { + echo $$name; // ok + echo $foo; // ok + echo $bar; + } elseif ($name === 'bar') { + echo $$name; // ok + echo $foo; + echo $bar; // ok + } else { + echo $$name; // ok + echo $foo; + echo $bar; + } + + echo $$name; // ok + echo $foo; + echo $bar; + } + } From 81efc7251a856e193034d5b749c047f25df554ee Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Wed, 19 Mar 2025 04:46:54 +0900 Subject: [PATCH 6/7] Fix class constant scope --- src/Rules/Classes/ClassConstantRule.php | 17 +++++++----- .../Rules/Classes/ClassConstantRuleTest.php | 24 +++++++++++++++++ .../Classes/data/dynamic-constant-access.php | 27 ++++++++++++++++++- 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/Rules/Classes/ClassConstantRule.php b/src/Rules/Classes/ClassConstantRule.php index 8f92b5908c..23c3aafdaa 100644 --- a/src/Rules/Classes/ClassConstantRule.php +++ b/src/Rules/Classes/ClassConstantRule.php @@ -3,7 +3,9 @@ namespace PHPStan\Rules\Classes; use PhpParser\Node; +use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\ClassConstFetch; +use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\NullsafeOperatorHelper; use PHPStan\Analyser\Scope; use PHPStan\Internal\SprintfHelper; @@ -21,7 +23,6 @@ use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use PHPStan\Type\VerbosityLevel; -use function array_map; use function array_merge; use function in_array; use function sprintf; @@ -51,14 +52,18 @@ public function processNode(Node $node, Scope $scope): array { $errors = []; if ($node->name instanceof Node\Identifier) { - $constantNames = [$node->name->name]; + $constantNameScopes = [$node->name->name => $scope]; } else { - $fetchType = $scope->getType($node->name); - $constantNames = array_map(static fn ($type): string => $type->getValue(), $fetchType->getConstantStrings()); + $nameType = $scope->getType($node->name); + $constantNameScopes = []; + foreach ($nameType->getConstantStrings() as $constantString) { + $name = $constantString->getValue(); + $constantNameScopes[$name] = $scope->filterByTruthyValue(new Identical($node->name, new String_($name))); + } } - foreach ($constantNames as $constantName) { - $errors = array_merge($errors, $this->processSingleClassConstFetch($scope, $node, $constantName)); + foreach ($constantNameScopes as $constantName => $constantScope) { + $errors = array_merge($errors, $this->processSingleClassConstFetch($constantScope, $node, $constantName)); } return $errors; diff --git a/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php b/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php index 388b3eed77..838201ffde 100644 --- a/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php @@ -452,6 +452,30 @@ public function testDynamicAccess(): void 'Access to undefined constant ClassConstantDynamicAccess\Foo::BUZ.', 20, ], + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::FOO.', + 37, + ], + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::BUZ.', + 39, + ], + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::QUX.', + 41, + ], + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::QUX.', + 44, + ], + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::BUZ.', + 44, + ], + [ + 'Access to undefined constant ClassConstantDynamicAccess\Foo::FOO.', + 44, + ], ]); } diff --git a/tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php b/tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php index fe3e8e73ea..10809e566a 100644 --- a/tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php +++ b/tests/PHPStan/Rules/Classes/data/dynamic-constant-access.php @@ -5,7 +5,7 @@ final class Foo { - private const BAR = 'FOO'; + private const BAR = 'BAR'; /** @var 'FOO'|'BAR'|'BUZ' */ public $name; @@ -20,4 +20,29 @@ public function test(string $string, object $obj): void echo self::{$this->name}; } + public function testScope(): void + { + $name1 = 'FOO'; + $rand = rand(); + if ($rand === 1) { + $foo = 1; + $name = $name1; + } elseif ($rand === 2) { + $name = 'BUZ'; + } else { + $name = 'QUX'; + } + + if ($name === 'FOO') { + echo self::{$name}; + } elseif ($name === 'BUZ') { + echo self::{$name}; + } else { + echo self::{$name}; + } + + echo self::{$name}; + } + + } From de7abf93df3895390106588a4e3b41fbed4af1ca Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Wed, 19 Mar 2025 10:11:54 +0900 Subject: [PATCH 7/7] fixup! Replaced with a simple foreach --- src/Rules/Methods/CallStaticMethodsRule.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Rules/Methods/CallStaticMethodsRule.php b/src/Rules/Methods/CallStaticMethodsRule.php index adf32ac22c..954c3a8669 100644 --- a/src/Rules/Methods/CallStaticMethodsRule.php +++ b/src/Rules/Methods/CallStaticMethodsRule.php @@ -12,9 +12,6 @@ use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; -use PHPStan\Type\Constant\ConstantStringType; -use function array_column; -use function array_map; use function array_merge; use function sprintf; @@ -43,13 +40,11 @@ public function processNode(Node $node, Scope $scope): array $methodNameScopes = [$node->name->name => $scope]; } else { $nameType = $scope->getType($node->name); - $methodNameScopes = array_column(array_map( - static fn (ConstantStringType $constantString) => [ - $name = $constantString->getValue(), - $scope->filterByTruthyValue(new Identical($node->name, new String_($name))), - ], - $nameType->getConstantStrings(), - ), 1, 0); + $methodNameScopes = []; + foreach ($nameType->getConstantStrings() as $constantString) { + $name = $constantString->getValue(); + $methodNameScopes[$name] = $scope->filterByTruthyValue(new Identical($node->name, new String_($name))); + } } foreach ($methodNameScopes as $methodName => $methodScope) {