From 606ac75a5f3b9dc93ddeb248adaa1de3348061f2 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Fri, 10 Oct 2025 13:52:44 +0200 Subject: [PATCH 1/7] Added rule to enforce interfaces usage --- .gitignore | 6 ++ rules/RequireInterfaceInDependenciesRule.php | 79 +++++++++++++++++++ .../Dependencies/ClassWithoutInterface.php | 17 ++++ .../Fixtures/Dependencies/ConcreteClass.php | 17 ++++ .../Fixtures/Dependencies/TestInterface.php | 14 ++++ .../RequireInterfaceInDependenciesFixture.php | 51 ++++++++++++ ...RequireInterfaceInDependenciesRuleTest.php | 43 ++++++++++ 7 files changed, 227 insertions(+) create mode 100644 .gitignore create mode 100644 rules/RequireInterfaceInDependenciesRule.php create mode 100644 tests/rules/Fixtures/Dependencies/ClassWithoutInterface.php create mode 100644 tests/rules/Fixtures/Dependencies/ConcreteClass.php create mode 100644 tests/rules/Fixtures/Dependencies/TestInterface.php create mode 100644 tests/rules/Fixtures/RequireInterfaceInDependenciesFixture.php create mode 100644 tests/rules/RequireInterfaceInDependenciesRuleTest.php diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..b04fa3f --- /dev/null +++ b/.gitignore @@ -0,0 +1,6 @@ +/vendor +/phpunit.xml +/.php_cs.cache +composer.lock +.php-cs-fixer.cache +.phpunit.result.cache diff --git a/rules/RequireInterfaceInDependenciesRule.php b/rules/RequireInterfaceInDependenciesRule.php new file mode 100644 index 0000000..4973ae1 --- /dev/null +++ b/rules/RequireInterfaceInDependenciesRule.php @@ -0,0 +1,79 @@ + + */ +final class RequireInterfaceInDependenciesRule implements Rule +{ + public function getNodeType(): string + { + return Node\Stmt\ClassMethod::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $errors = []; + + if (!$node->params) { + return []; + } + + foreach ($node->params as $param) { + + if (!$param->type instanceof Node\Name) { + continue; + } + if ($param->var instanceof Error) { + continue; + } + $typeName = $param->type->toString(); + + // Skip built-in types and primitives + if ($this->isBuiltInType($typeName)) { + continue; + } + + if (!interface_exists($typeName) && class_exists($typeName)) { + // Check if this class implements any interface + $interfaces = class_implements($typeName); + + if (!empty($interfaces)) { + $errors[] = RuleErrorBuilder::message( + sprintf( + 'Parameter $%s uses concrete class %s instead of an interface. Available interfaces: %s', + is_string($param->var->name) ? $param->var->name : $param->var->name->getType(), + $typeName, + implode(', ', $interfaces) + ) + )->build(); + } + } + } + + return $errors; + } + + private function isBuiltInType(string $type): bool + { + $builtInTypes = [ + 'string', 'int', 'float', 'bool', 'array', 'object', + 'callable', 'iterable', 'mixed', 'void', 'never', + ]; + + return in_array(strtolower($type), $builtInTypes); + } +} diff --git a/tests/rules/Fixtures/Dependencies/ClassWithoutInterface.php b/tests/rules/Fixtures/Dependencies/ClassWithoutInterface.php new file mode 100644 index 0000000..8f66616 --- /dev/null +++ b/tests/rules/Fixtures/Dependencies/ClassWithoutInterface.php @@ -0,0 +1,17 @@ +concreteClass = $concreteClass; + $this->testInterface = $testInterface; + $this->classWithoutInterface = $classWithoutInterface; + } + + public function methodWithConcreteClass(ConcreteClass $class): void + { + } + + public function methodWithInterface(TestInterface $interface): void + { + } + + public function methodWithoutInterface(ClassWithoutInterface $class): void + { + } + + public function methodWithBuiltInTypes(string $str, int $num): void + { + } +} diff --git a/tests/rules/RequireInterfaceInDependenciesRuleTest.php b/tests/rules/RequireInterfaceInDependenciesRuleTest.php new file mode 100644 index 0000000..3e6497f --- /dev/null +++ b/tests/rules/RequireInterfaceInDependenciesRuleTest.php @@ -0,0 +1,43 @@ + + */ +final class RequireInterfaceInDependenciesRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new RequireInterfaceInDependenciesRule(); + } + + public function testRule(): void + { + $this->analyse( + [ + __DIR__ . '/Fixtures/RequireInterfaceInDependenciesFixture.php', + ], + [ + [ + 'Parameter $concreteClass uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\Dependencies\ConcreteClass instead of an interface. Available interfaces: Ibexa\Tests\PHPStan\Rules\Fixtures\Dependencies\TestInterface', + 21, + ], + [ + 'Parameter $class uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\Dependencies\ConcreteClass instead of an interface. Available interfaces: Ibexa\Tests\PHPStan\Rules\Fixtures\Dependencies\TestInterface', + 31, + ], + ] + ); + } +} From 6ff261120cfd1b54db147393582944d7d9f0f9b0 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Fri, 10 Oct 2025 15:50:18 +0200 Subject: [PATCH 2/7] cleanup some names --- .../AbstractCorrectClass.php} | 8 ++------ .../NamingConvention/CorrectNameInterface.php | 13 +++++++++++++ .../Fixtures/NamingConvention/CorrectNameTrait.php | 13 +++++++++++++ .../rules/Fixtures/NamingConvention/SimpleClass.php | 13 +++++++++++++ .../rules/Fixtures/NamingConvention/SimpleThing.php | 13 +++++++++++++ tests/rules/Fixtures/NamingConvention/WrongName.php | 13 +++++++++++++ .../ConcreteClass.php | 2 +- .../TestInterface.php | 2 +- .../RequireInterfaceInDependenciesFixture.php | 6 +++--- .../RequireInterfaceInDependenciesRuleTest.php | 8 ++++---- 10 files changed, 76 insertions(+), 15 deletions(-) rename tests/rules/Fixtures/{Dependencies/ClassWithoutInterface.php => NamingConvention/AbstractCorrectClass.php} (56%) create mode 100644 tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php create mode 100644 tests/rules/Fixtures/NamingConvention/CorrectNameTrait.php create mode 100644 tests/rules/Fixtures/NamingConvention/SimpleClass.php create mode 100644 tests/rules/Fixtures/NamingConvention/SimpleThing.php create mode 100644 tests/rules/Fixtures/NamingConvention/WrongName.php rename tests/rules/Fixtures/{Dependencies => RequireInterfaceInDependencies}/ConcreteClass.php (81%) rename tests/rules/Fixtures/{Dependencies => RequireInterfaceInDependencies}/TestInterface.php (78%) diff --git a/tests/rules/Fixtures/Dependencies/ClassWithoutInterface.php b/tests/rules/Fixtures/NamingConvention/AbstractCorrectClass.php similarity index 56% rename from tests/rules/Fixtures/Dependencies/ClassWithoutInterface.php rename to tests/rules/Fixtures/NamingConvention/AbstractCorrectClass.php index 8f66616..37c4dd5 100644 --- a/tests/rules/Fixtures/Dependencies/ClassWithoutInterface.php +++ b/tests/rules/Fixtures/NamingConvention/AbstractCorrectClass.php @@ -6,12 +6,8 @@ */ declare(strict_types=1); -namespace Ibexa\Tests\PHPStan\Rules\Fixtures\Dependencies; +namespace Ibexa\Tests\PHPStan\Rules\Fixtures\NamingConvention; -class ClassWithoutInterface +abstract class AbstractCorrectClass { - public function doSomething(): void - { - // Implementation - } } diff --git a/tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php b/tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php new file mode 100644 index 0000000..408ec76 --- /dev/null +++ b/tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php @@ -0,0 +1,13 @@ + Date: Tue, 21 Oct 2025 11:29:48 +0200 Subject: [PATCH 3/7] Added rule for enforcing either interface or abstract class as dependency / method argument --- phpstan-baseline.neon | 26 ++++++++ ... RequireAbstractionInDependenciesRule.php} | 38 +++++++++-- .../NamingConvention/CorrectNameInterface.php | 13 ---- .../NamingConvention/CorrectNameTrait.php | 13 ---- .../Fixtures/NamingConvention/SimpleThing.php | 13 ---- .../Fixtures/NamingConvention/WrongName.php | 13 ---- .../AbstractClass.php} | 5 +- .../ClassWithoutInterface.php} | 8 ++- .../ConcreteClass.php | 2 +- .../ConcreteClassExtendingAbstract.php | 17 +++++ .../TestInterface.php | 2 +- ...equireAbstractionInDependenciesFixture.php | 66 +++++++++++++++++++ .../RequireInterfaceInDependenciesFixture.php | 51 -------------- ...quireAbstractionInDependenciesRuleTest.php | 51 ++++++++++++++ ...RequireInterfaceInDependenciesRuleTest.php | 43 ------------ 15 files changed, 202 insertions(+), 159 deletions(-) create mode 100644 phpstan-baseline.neon rename rules/{RequireInterfaceInDependenciesRule.php => RequireAbstractionInDependenciesRule.php} (59%) delete mode 100644 tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php delete mode 100644 tests/rules/Fixtures/NamingConvention/CorrectNameTrait.php delete mode 100644 tests/rules/Fixtures/NamingConvention/SimpleThing.php delete mode 100644 tests/rules/Fixtures/NamingConvention/WrongName.php rename tests/rules/Fixtures/{NamingConvention/SimpleClass.php => RequireAbstractionInDependencies/AbstractClass.php} (56%) rename tests/rules/Fixtures/{NamingConvention/AbstractCorrectClass.php => RequireAbstractionInDependencies/ClassWithoutInterface.php} (53%) rename tests/rules/Fixtures/{RequireInterfaceInDependencies => RequireAbstractionInDependencies}/ConcreteClass.php (80%) create mode 100644 tests/rules/Fixtures/RequireAbstractionInDependencies/ConcreteClassExtendingAbstract.php rename tests/rules/Fixtures/{RequireInterfaceInDependencies => RequireAbstractionInDependencies}/TestInterface.php (77%) create mode 100644 tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php delete mode 100644 tests/rules/Fixtures/RequireInterfaceInDependenciesFixture.php create mode 100644 tests/rules/RequireAbstractionInDependenciesRuleTest.php delete mode 100644 tests/rules/RequireInterfaceInDependenciesRuleTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon new file mode 100644 index 0000000..6caad11 --- /dev/null +++ b/phpstan-baseline.neon @@ -0,0 +1,26 @@ +parameters: + ignoreErrors: + - + message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$abstractClass is never read, only written\\.$#" + count: 1 + path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php + + - + message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$classWithoutInterface is never read, only written\\.$#" + count: 1 + path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php + + - + message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$concreteClass is never read, only written\\.$#" + count: 1 + path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php + + - + message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$concreteExtendingAbstract is never read, only written\\.$#" + count: 1 + path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php + + - + message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$testInterface is never read, only written\\.$#" + count: 1 + path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php diff --git a/rules/RequireInterfaceInDependenciesRule.php b/rules/RequireAbstractionInDependenciesRule.php similarity index 59% rename from rules/RequireInterfaceInDependenciesRule.php rename to rules/RequireAbstractionInDependenciesRule.php index 4973ae1..4a42ace 100644 --- a/rules/RequireInterfaceInDependenciesRule.php +++ b/rules/RequireAbstractionInDependenciesRule.php @@ -17,7 +17,7 @@ /** * @implements \PHPStan\Rules\Rule<\PhpParser\Node\Stmt\ClassMethod> */ -final class RequireInterfaceInDependenciesRule implements Rule +final class RequireAbstractionInDependenciesRule implements Rule { public function getNodeType(): string { @@ -33,7 +33,6 @@ public function processNode(Node $node, Scope $scope): array } foreach ($node->params as $param) { - if (!$param->type instanceof Node\Name) { continue; } @@ -47,17 +46,42 @@ public function processNode(Node $node, Scope $scope): array continue; } - if (!interface_exists($typeName) && class_exists($typeName)) { - // Check if this class implements any interface + // Skip interfaces - they are always acceptable + if (interface_exists($typeName)) { + continue; + } + + // Check if it's a class + if (class_exists($typeName)) { + $reflection = new \ReflectionClass($typeName); + + // Skip abstract classes - they are acceptable + if ($reflection->isAbstract()) { + continue; + } + + // This is a concrete class - check if it has interfaces or extends an abstract class $interfaces = class_implements($typeName); + $parentClass = $reflection->getParentClass(); + $hasAbstractParent = $parentClass && $parentClass->isAbstract(); + + if (!empty($interfaces) || $hasAbstractParent) { + $suggestions = []; + + if (!empty($interfaces)) { + $suggestions[] = 'Available interfaces: ' . implode(', ', $interfaces); + } + + if ($hasAbstractParent) { + $suggestions[] = 'Abstract parent: ' . $parentClass->getName(); + } - if (!empty($interfaces)) { $errors[] = RuleErrorBuilder::message( sprintf( - 'Parameter $%s uses concrete class %s instead of an interface. Available interfaces: %s', + 'Parameter $%s uses concrete class %s instead of an interface or abstract class. %s', is_string($param->var->name) ? $param->var->name : $param->var->name->getType(), $typeName, - implode(', ', $interfaces) + implode('. ', $suggestions) ) )->build(); } diff --git a/tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php b/tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php deleted file mode 100644 index 408ec76..0000000 --- a/tests/rules/Fixtures/NamingConvention/CorrectNameInterface.php +++ /dev/null @@ -1,13 +0,0 @@ -concreteClass = $concreteClass; + $this->testInterface = $testInterface; + $this->classWithoutInterface = $classWithoutInterface; + $this->abstractClass = $abstractClass; + $this->concreteExtendingAbstract = $concreteExtendingAbstract; + } + + public function methodWithConcreteClass(ConcreteClass $class): void + { + } + + public function methodWithInterface(TestInterface $interface): void + { + } + + public function methodWithAbstractClass(AbstractClass $abstract): void + { + } + + public function methodWithConcreteExtendingAbstract(ConcreteClassExtendingAbstract $concreteExtendingAbstract): void + { + } + + public function methodWithoutInterface(ClassWithoutInterface $class): void + { + } + + public function methodWithBuiltInTypes(string $str, int $num): void + { + } +} diff --git a/tests/rules/Fixtures/RequireInterfaceInDependenciesFixture.php b/tests/rules/Fixtures/RequireInterfaceInDependenciesFixture.php deleted file mode 100644 index 2e067f9..0000000 --- a/tests/rules/Fixtures/RequireInterfaceInDependenciesFixture.php +++ /dev/null @@ -1,51 +0,0 @@ -concreteClass = $concreteClass; - $this->testInterface = $testInterface; - $this->classWithoutInterface = $classWithoutInterface; - } - - public function methodWithConcreteClass(ConcreteClass $class): void - { - } - - public function methodWithInterface(TestInterface $interface): void - { - } - - public function methodWithoutInterface(ClassWithoutInterface $class): void - { - } - - public function methodWithBuiltInTypes(string $str, int $num): void - { - } -} diff --git a/tests/rules/RequireAbstractionInDependenciesRuleTest.php b/tests/rules/RequireAbstractionInDependenciesRuleTest.php new file mode 100644 index 0000000..7519621 --- /dev/null +++ b/tests/rules/RequireAbstractionInDependenciesRuleTest.php @@ -0,0 +1,51 @@ + + */ +final class RequireAbstractionInDependenciesRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new RequireAbstractionInDependenciesRule(); + } + + public function testRule(): void + { + $this->analyse( + [ + __DIR__ . '/Fixtures/RequireAbstractionInDependenciesFixture.php', + ], + [ + [ + 'Parameter $concreteClass uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\ConcreteClass instead of an interface or abstract class. Available interfaces: Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\TestInterface', + 29, + ], + [ + 'Parameter $concreteExtendingAbstract uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\ConcreteClassExtendingAbstract instead of an interface or abstract class. Abstract parent: Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\AbstractClass', + 29, + ], + [ + 'Parameter $class uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\ConcreteClass instead of an interface or abstract class. Available interfaces: Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\TestInterface', + 43, + ], + [ + 'Parameter $concreteExtendingAbstract uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\ConcreteClassExtendingAbstract instead of an interface or abstract class. Abstract parent: Ibexa\Tests\PHPStan\Rules\Fixtures\RequireAbstractionInDependencies\AbstractClass', + 55, + ], + ] + ); + } +} diff --git a/tests/rules/RequireInterfaceInDependenciesRuleTest.php b/tests/rules/RequireInterfaceInDependenciesRuleTest.php deleted file mode 100644 index 13bdff2..0000000 --- a/tests/rules/RequireInterfaceInDependenciesRuleTest.php +++ /dev/null @@ -1,43 +0,0 @@ - - */ -final class RequireInterfaceInDependenciesRuleTest extends RuleTestCase -{ - protected function getRule(): Rule - { - return new RequireInterfaceInDependenciesRule(); - } - - public function testRule(): void - { - $this->analyse( - [ - __DIR__ . '/Fixtures/RequireInterfaceInDependenciesFixture.php', - ], - [ - [ - 'Parameter $concreteClass uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\RequireInterfaceInDependencies\ConcreteClass instead of an interface. Available interfaces: Ibexa\Tests\PHPStan\Rules\Fixtures\RequireInterfaceInDependencies\TestInterface', - 23, - ], - [ - 'Parameter $class uses concrete class Ibexa\Tests\PHPStan\Rules\Fixtures\RequireInterfaceInDependencies\ConcreteClass instead of an interface. Available interfaces: Ibexa\Tests\PHPStan\Rules\Fixtures\RequireInterfaceInDependencies\TestInterface', - 33, - ], - ] - ); - } -} From 43ea95a0c784ed1cef3b5e8a06091b9b686804ec Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Tue, 21 Oct 2025 12:02:04 +0200 Subject: [PATCH 4/7] include baseline --- phpstan.neon | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpstan.neon b/phpstan.neon index ab3d432..c4d75b1 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,3 +1,6 @@ +includes: + - phpstan-baseline.neon + parameters: level: 8 paths: From 161b24ee45879dde11d1298aafe45759fb89fab8 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Tue, 21 Oct 2025 13:32:40 +0200 Subject: [PATCH 5/7] Added rule to config --- extension.neon | 1 + 1 file changed, 1 insertion(+) diff --git a/extension.neon b/extension.neon index 80cf7ed..075005a 100644 --- a/extension.neon +++ b/extension.neon @@ -7,3 +7,4 @@ parameters: - stubs/Money/MoneyParser.stub rules: - Ibexa\PHPStan\Rules\NoConfigResolverParametersInConstructorRule + - Ibexa\PHPStan\Rules\RequireAbstractionInDependenciesRule From 25e1155849f2a4eb0c8e81cd1aa431ad3826616a Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Wed, 22 Oct 2025 11:17:47 +0200 Subject: [PATCH 6/7] move errors from baseline to phpstan.neon --- phpstan-baseline.neon | 26 -------------------------- phpstan.neon | 5 +++++ 2 files changed, 5 insertions(+), 26 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6caad11..e69de29 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,26 +0,0 @@ -parameters: - ignoreErrors: - - - message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$abstractClass is never read, only written\\.$#" - count: 1 - path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php - - - - message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$classWithoutInterface is never read, only written\\.$#" - count: 1 - path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php - - - - message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$concreteClass is never read, only written\\.$#" - count: 1 - path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php - - - - message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$concreteExtendingAbstract is never read, only written\\.$#" - count: 1 - path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php - - - - message: "#^Property Ibexa\\\\Tests\\\\PHPStan\\\\Rules\\\\Fixtures\\\\RequireAbstractionInDependenciesFixture\\:\\:\\$testInterface is never read, only written\\.$#" - count: 1 - path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php diff --git a/phpstan.neon b/phpstan.neon index c4d75b1..0e28375 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,3 +7,8 @@ parameters: - rules - tests checkMissingCallableSignature: true + ignoreErrors: + # Test fixture properties are intentionally write-only for testing purposes + - + message: "#^Property .* is never read, only written\\.$#" + path: tests/rules/Fixtures/RequireAbstractionInDependenciesFixture.php From 9058c273b85d277806f95b5caace9af26528f6b9 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Wed, 22 Oct 2025 11:30:12 +0200 Subject: [PATCH 7/7] refactored rule to use reflection provider to clear up whitelist of built in types --- .../RequireAbstractionInDependenciesRule.php | 133 ++++++++++-------- ...quireAbstractionInDependenciesRuleTest.php | 4 +- 2 files changed, 76 insertions(+), 61 deletions(-) diff --git a/rules/RequireAbstractionInDependenciesRule.php b/rules/RequireAbstractionInDependenciesRule.php index 4a42ace..255b882 100644 --- a/rules/RequireAbstractionInDependenciesRule.php +++ b/rules/RequireAbstractionInDependenciesRule.php @@ -11,7 +11,9 @@ use PhpParser\Node; use PhpParser\Node\Expr\Error; use PHPStan\Analyser\Scope; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; /** @@ -19,6 +21,14 @@ */ final class RequireAbstractionInDependenciesRule implements Rule { + private ReflectionProvider $reflectionProvider; + + public function __construct( + ReflectionProvider $reflectionProvider + ) { + $this->reflectionProvider = $reflectionProvider; + } + public function getNodeType(): string { return Node\Stmt\ClassMethod::class; @@ -26,78 +36,81 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - $errors = []; - if (!$node->params) { return []; } - foreach ($node->params as $param) { - if (!$param->type instanceof Node\Name) { - continue; - } - if ($param->var instanceof Error) { - continue; - } - $typeName = $param->type->toString(); - - // Skip built-in types and primitives - if ($this->isBuiltInType($typeName)) { - continue; - } - - // Skip interfaces - they are always acceptable - if (interface_exists($typeName)) { - continue; - } + $errors = []; - // Check if it's a class - if (class_exists($typeName)) { - $reflection = new \ReflectionClass($typeName); - - // Skip abstract classes - they are acceptable - if ($reflection->isAbstract()) { - continue; - } - - // This is a concrete class - check if it has interfaces or extends an abstract class - $interfaces = class_implements($typeName); - $parentClass = $reflection->getParentClass(); - $hasAbstractParent = $parentClass && $parentClass->isAbstract(); - - if (!empty($interfaces) || $hasAbstractParent) { - $suggestions = []; - - if (!empty($interfaces)) { - $suggestions[] = 'Available interfaces: ' . implode(', ', $interfaces); - } - - if ($hasAbstractParent) { - $suggestions[] = 'Abstract parent: ' . $parentClass->getName(); - } - - $errors[] = RuleErrorBuilder::message( - sprintf( - 'Parameter $%s uses concrete class %s instead of an interface or abstract class. %s', - is_string($param->var->name) ? $param->var->name : $param->var->name->getType(), - $typeName, - implode('. ', $suggestions) - ) - )->build(); - } + foreach ($node->params as $param) { + $error = $this->validateParameter($param); + if ($error !== null) { + $errors[] = $error; } } return $errors; } - private function isBuiltInType(string $type): bool + private function validateParameter(Node\Param $param): ?RuleError { - $builtInTypes = [ - 'string', 'int', 'float', 'bool', 'array', 'object', - 'callable', 'iterable', 'mixed', 'void', 'never', - ]; + if (!$param->type instanceof Node\Name) { + return null; + } + + if ($param->var instanceof Error) { + return null; + } + + $typeName = $param->type->toString(); + + // Skip if the type doesn't exist in reflection + if (!$this->reflectionProvider->hasClass($typeName)) { + return null; + } + + $classReflection = $this->reflectionProvider->getClass($typeName); + + // Skip interfaces - they are always acceptable + if ($classReflection->isInterface()) { + return null; + } + + // Skip abstract classes - they are acceptable + if ($classReflection->isAbstract()) { + return null; + } + + $reflection = $classReflection->getNativeReflection(); + + // This is a concrete class - check if it has interfaces or extends an abstract class + $interfaces = class_implements($typeName); + $parentClass = $reflection->getParentClass(); + $hasAbstractParent = $parentClass && $parentClass->isAbstract(); + + // If there are no interfaces and no abstract parent, it's acceptable (no violation) + if (empty($interfaces) && !$hasAbstractParent) { + return null; + } + + // Build error with suggestions + $suggestions = []; + + if (!empty($interfaces)) { + $suggestions[] = 'Available interfaces: ' . implode(', ', $interfaces); + } + + if ($hasAbstractParent) { + $suggestions[] = 'Abstract parent: ' . $parentClass->getName(); + } - return in_array(strtolower($type), $builtInTypes); + return RuleErrorBuilder::message( + sprintf( + 'Parameter $%s uses concrete class %s instead of an interface or abstract class. %s', + is_string($param->var->name) ? $param->var->name : $param->var->name->getType(), + $typeName, + implode('. ', $suggestions) + ) + )->build(); } } diff --git a/tests/rules/RequireAbstractionInDependenciesRuleTest.php b/tests/rules/RequireAbstractionInDependenciesRuleTest.php index 7519621..3365496 100644 --- a/tests/rules/RequireAbstractionInDependenciesRuleTest.php +++ b/tests/rules/RequireAbstractionInDependenciesRuleTest.php @@ -19,7 +19,9 @@ final class RequireAbstractionInDependenciesRuleTest extends RuleTestCase { protected function getRule(): Rule { - return new RequireAbstractionInDependenciesRule(); + return new RequireAbstractionInDependenciesRule( + $this->createReflectionProvider() + ); } public function testRule(): void