From e5d182f751529c1bbbb4c17831bacd9efe3aecb7 Mon Sep 17 00:00:00 2001 From: sayuprc Date: Sun, 24 Nov 2024 01:26:13 +0900 Subject: [PATCH 1/6] Fix PHPStan issue by enabling constructor check for class-string variables --- src/Rules/Classes/InstantiationRule.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Rules/Classes/InstantiationRule.php b/src/Rules/Classes/InstantiationRule.php index 7dd65488a0..89a97fbe23 100644 --- a/src/Rules/Classes/InstantiationRule.php +++ b/src/Rules/Classes/InstantiationRule.php @@ -16,11 +16,16 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; +use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantStringType; +use PHPStan\Type\Generic\TemplateGenericObjectType; +use PHPStan\Type\StaticType; +use PHPStan\Type\VerbosityLevel; use function array_map; use function array_merge; use function count; use function sprintf; +use function str_starts_with; use function strtolower; /** @@ -245,6 +250,21 @@ private function getClassNames(Node $node, Scope $scope): array $type = $scope->getType($node->class); + if (str_starts_with($type->describe(VerbosityLevel::precise()), 'class-string')) { + $classStringObjectType = $type->getClassStringObjectType(); + + if ( + !$classStringObjectType instanceof StaticType + && !$classStringObjectType instanceof CompoundType + && !$classStringObjectType instanceof TemplateGenericObjectType + ) { + return array_map( + static fn (string $name): array => [$name, true], + $classStringObjectType->getObjectClassNames(), + ); + } + } + return array_merge( array_map( static fn (ConstantStringType $type): array => [$type->getValue(), true], From a59fc5dd317a6ddf54f8f4ecfeacf01b4359ca1d Mon Sep 17 00:00:00 2001 From: sayuprc Date: Sun, 24 Nov 2024 13:23:34 +0900 Subject: [PATCH 2/6] Add tests --- .../Rules/Classes/InstantiationRuleTest.php | 30 ++++++++++++++++ .../Rules/Classes/data/class-string.php | 34 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/PHPStan/Rules/Classes/data/class-string.php diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index 43b9091daf..c44e16784d 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -504,4 +504,34 @@ public function testBug11815(): void $this->analyse([__DIR__ . '/data/bug-11815.php'], []); } + public function testClassString(): void + { + $this->analyse([__DIR__ . '/data/class-string.php'], [ + [ + 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', + 26, + ], + [ + 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', + 27, + ], + [ + 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', + 28, + ], + [ + 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', + 31, + ], + [ + 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', + 32, + ], + [ + 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', + 34, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Classes/data/class-string.php b/tests/PHPStan/Rules/Classes/data/class-string.php new file mode 100644 index 0000000000..01570a0e1c --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/class-string.php @@ -0,0 +1,34 @@ += 8.0 + +declare(strict_types = 1); + +namespace ClassString; + +class A +{ + public function __construct(public int $i) + { + } +} + +class HelloWorld +{ + /** + * @return class-string + */ + public static function sayHelloBug(): string + { + return A::class; + } +} + +$classString = HelloWorld::sayHelloBug(); +$bug = new (HelloWorld::sayHelloBug())('O_O'); +$bug = new ($classString)('O_O'); +$bug = new $classString('O_O'); + +$className = A::class; +$ok = new ($className)('O_O'); +$ok = new $className('O_O'); + +$ok = new A('O_O'); From f4cfd0005e3f6d3e645923f9bd3c126b1584603f Mon Sep 17 00:00:00 2001 From: sayuprc Date: Sun, 24 Nov 2024 21:53:30 +0900 Subject: [PATCH 3/6] Fixed behavior --- src/Rules/Classes/InstantiationRule.php | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/Rules/Classes/InstantiationRule.php b/src/Rules/Classes/InstantiationRule.php index 89a97fbe23..2de3bd9a11 100644 --- a/src/Rules/Classes/InstantiationRule.php +++ b/src/Rules/Classes/InstantiationRule.php @@ -16,11 +16,9 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; -use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantStringType; -use PHPStan\Type\Generic\TemplateGenericObjectType; -use PHPStan\Type\StaticType; use PHPStan\Type\VerbosityLevel; +use function array_filter; use function array_map; use function array_merge; use function count; @@ -253,16 +251,13 @@ private function getClassNames(Node $node, Scope $scope): array if (str_starts_with($type->describe(VerbosityLevel::precise()), 'class-string')) { $classStringObjectType = $type->getClassStringObjectType(); - if ( - !$classStringObjectType instanceof StaticType - && !$classStringObjectType instanceof CompoundType - && !$classStringObjectType instanceof TemplateGenericObjectType - ) { - return array_map( - static fn (string $name): array => [$name, true], - $classStringObjectType->getObjectClassNames(), - ); - } + return array_map( + static fn (string $name): array => [$name, true], + array_filter($classStringObjectType->getObjectClassNames(), function (string $name): bool { + $reflectionClass = $this->reflectionProvider->getClass($name); + return !$reflectionClass->isAbstract() && !$reflectionClass->isInterface(); + }), + ); } return array_merge( From 615d5e614f3820009fec65d334e9aad21db261f2 Mon Sep 17 00:00:00 2001 From: sayuprc Date: Sun, 24 Nov 2024 21:56:35 +0900 Subject: [PATCH 4/6] Add tests --- .../Rules/Classes/InstantiationRuleTest.php | 24 ++++-- .../Rules/Classes/data/class-string.php | 73 ++++++++++++++++--- 2 files changed, 81 insertions(+), 16 deletions(-) diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index c44e16784d..31d241bee6 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -509,27 +509,39 @@ public function testClassString(): void $this->analyse([__DIR__ . '/data/class-string.php'], [ [ 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', - 26, + 65, ], [ 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', - 27, + 66, ], [ 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', - 28, + 67, + ], + [ + 'Parameter #1 $i of class ClassString\C constructor expects int, string given.', + 75, + ], + [ + 'Parameter #1 $i of class ClassString\C constructor expects int, string given.', + 76, + ], + [ + 'Parameter #1 $i of class ClassString\C constructor expects int, string given.', + 77, ], [ 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', - 31, + 85, ], [ 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', - 32, + 86, ], [ 'Parameter #1 $i of class ClassString\A constructor expects int, string given.', - 34, + 87, ], ]); } diff --git a/tests/PHPStan/Rules/Classes/data/class-string.php b/tests/PHPStan/Rules/Classes/data/class-string.php index 01570a0e1c..bb07d5954a 100644 --- a/tests/PHPStan/Rules/Classes/data/class-string.php +++ b/tests/PHPStan/Rules/Classes/data/class-string.php @@ -11,24 +11,77 @@ public function __construct(public int $i) } } -class HelloWorld +abstract class B +{ + public function __construct(public int $i) + { + } +} + +class C extends B +{ +} + +interface D +{ +} + +class Foo { /** * @return class-string */ - public static function sayHelloBug(): string + public static function returnClassStringA(): string { return A::class; } + + /** + * @return class-string + */ + public static function returnClassStringB(): string + { + return B::class; + } + + /** + * @return class-string + */ + public static function returnClassStringC(): string + { + return C::class; + } + + /** + * @return class-string + */ + public static function returnClassStringD(): string + { + return D::class; + } } -$classString = HelloWorld::sayHelloBug(); -$bug = new (HelloWorld::sayHelloBug())('O_O'); -$bug = new ($classString)('O_O'); -$bug = new $classString('O_O'); +$classString = Foo::returnClassStringA(); +$error = new (Foo::returnClassStringA())('O_O'); +$error = new ($classString)('O_O'); +$error = new $classString('O_O'); -$className = A::class; -$ok = new ($className)('O_O'); -$ok = new $className('O_O'); +$classString = Foo::returnClassStringB(); +$ok = new (Foo::returnClassStringB())('O_O'); +$ok = new ($classString)('O_O'); +$ok = new $classString('O_O'); -$ok = new A('O_O'); +$classString = Foo::returnClassStringC(); +$error = new (Foo::returnClassStringC())('O_O'); +$error = new ($classString)('O_O'); +$error = new $classString('O_O'); + +$classString = Foo::returnClassStringD(); +$ok = new (Foo::returnClassStringD())('O_O'); +$ok = new ($classString)('O_O'); +$ok = new $classString('O_O'); + +$className = A::class; +$error = new ($className)('O_O'); +$error = new $className('O_O'); +$error = new A('O_O'); From 298e55e79123c44068a009c11c8b89a042b89007 Mon Sep 17 00:00:00 2001 From: sayuprc Date: Mon, 2 Dec 2024 22:12:43 +0900 Subject: [PATCH 5/6] Simplify --- src/Rules/Classes/InstantiationRule.php | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/Rules/Classes/InstantiationRule.php b/src/Rules/Classes/InstantiationRule.php index 2de3bd9a11..2b8bcc3490 100644 --- a/src/Rules/Classes/InstantiationRule.php +++ b/src/Rules/Classes/InstantiationRule.php @@ -6,6 +6,7 @@ use PhpParser\Node\Expr\New_; use PHPStan\Analyser\Scope; use PHPStan\Internal\SprintfHelper; +use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\Php\PhpMethodReflection; use PHPStan\Reflection\ReflectionProvider; @@ -17,13 +18,11 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantStringType; -use PHPStan\Type\VerbosityLevel; use function array_filter; use function array_map; use function array_merge; use function count; use function sprintf; -use function str_starts_with; use function strtolower; /** @@ -248,16 +247,18 @@ private function getClassNames(Node $node, Scope $scope): array $type = $scope->getType($node->class); - if (str_starts_with($type->describe(VerbosityLevel::precise()), 'class-string')) { - $classStringObjectType = $type->getClassStringObjectType(); - - return array_map( - static fn (string $name): array => [$name, true], - array_filter($classStringObjectType->getObjectClassNames(), function (string $name): bool { - $reflectionClass = $this->reflectionProvider->getClass($name); - return !$reflectionClass->isAbstract() && !$reflectionClass->isInterface(); - }), + if ($type->isClassString()->yes()) { + $concretes = array_filter( + $type->getClassStringObjectType()->getObjectClassReflections(), + static fn (ClassReflection $classReflection): bool => !$classReflection->isAbstract() && !$classReflection->isInterface(), ); + + if (0 < count($concretes)) { + return array_map( + static fn (ClassReflection $classReflection): array => [$classReflection->getName(), true], + $concretes, + ); + } } return array_merge( From 1072835ee16abd8768fd8fc197897063c072e6f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Mirtes?= Date: Thu, 16 Jan 2025 15:40:53 +0100 Subject: [PATCH 6/6] Update src/Rules/Classes/InstantiationRule.php --- src/Rules/Classes/InstantiationRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rules/Classes/InstantiationRule.php b/src/Rules/Classes/InstantiationRule.php index 2b8bcc3490..4c0a424c1b 100644 --- a/src/Rules/Classes/InstantiationRule.php +++ b/src/Rules/Classes/InstantiationRule.php @@ -253,7 +253,7 @@ private function getClassNames(Node $node, Scope $scope): array static fn (ClassReflection $classReflection): bool => !$classReflection->isAbstract() && !$classReflection->isInterface(), ); - if (0 < count($concretes)) { + if (count($concretes) > 0) { return array_map( static fn (ClassReflection $classReflection): array => [$classReflection->getName(), true], $concretes,