From 998146ccef3414cf16dca4d17a2acd78e9fece75 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 18 Dec 2024 17:10:21 +0100 Subject: [PATCH] Remove NoSingleInterfaceImplementerRule as way complex, let devs handle it (#156) --- README.md | 41 -------- config/code-complexity-rules.neon | 17 ---- phpstan.neon | 3 - .../ImplementedInterfaceCollector.php | 42 -------- src/Collector/InterfaceCollector.php | 31 ------ .../InterfaceOfAbstractClassCollector.php | 41 -------- src/Enum/RuleIdentifier.php | 5 - .../NoSingleInterfaceImplementerRule.php | 98 ------------------- .../Fixture/AllowAbstract.php | 9 -- .../Fixture/ImplementsSimpleInterface.php | 9 -- .../Fixture/SimpleInterface.php | 9 -- .../NoSingleInterfaceImplementerRuleTest.php | 69 ------------- .../Source/SomeType.php | 7 -- .../config/configured_rule.neon | 3 - 14 files changed, 384 deletions(-) delete mode 100644 src/Collector/ImplementedInterfaceCollector.php delete mode 100644 src/Collector/InterfaceCollector.php delete mode 100644 src/Collector/InterfaceOfAbstractClassCollector.php delete mode 100644 src/Rules/NoSingleInterfaceImplementerRule.php delete mode 100644 tests/Rules/NoSingleInterfaceImplementerRule/Fixture/AllowAbstract.php delete mode 100644 tests/Rules/NoSingleInterfaceImplementerRule/Fixture/ImplementsSimpleInterface.php delete mode 100644 tests/Rules/NoSingleInterfaceImplementerRule/Fixture/SimpleInterface.php delete mode 100644 tests/Rules/NoSingleInterfaceImplementerRule/NoSingleInterfaceImplementerRuleTest.php delete mode 100644 tests/Rules/NoSingleInterfaceImplementerRule/Source/SomeType.php delete mode 100644 tests/Rules/NoSingleInterfaceImplementerRule/config/configured_rule.neon diff --git a/README.md b/README.md index 49e57996..2d061540 100644 --- a/README.md +++ b/README.md @@ -596,47 +596,6 @@ final class SomeClass
-### NoSingleInterfaceImplementerRule - -Interface "%s" has only single implementer. Consider using the class directly as there is no point in using the interface. - -```yaml -rules: - - Symplify\PHPStanRules\Rules\NoSingleInterfaceImplementerRule -``` - -```php -class SomeClass implements SomeInterface -{ -} - -interface SomeInterface -{ -} -``` - -:x: - -
- -```php -class SomeClass implements SomeInterface -{ -} - -class AnotherClass implements SomeInterface -{ -} - -interface SomeInterface -{ -} -``` - -:+1: - -
- ### NoTestMocksRule Mocking "%s" class is forbidden. Use direct/anonymous class instead for better static analysis diff --git a/config/code-complexity-rules.neon b/config/code-complexity-rules.neon index 7809585e..2ae1382d 100644 --- a/config/code-complexity-rules.neon +++ b/config/code-complexity-rules.neon @@ -1,19 +1,2 @@ rules: - Symplify\PHPStanRules\Rules\NoDynamicNameRule - - Symplify\PHPStanRules\Rules\NoSingleInterfaceImplementerRule - -services: - - - class: Symplify\PHPStanRules\Collector\InterfaceCollector - tags: - - phpstan.collector - - - - class: Symplify\PHPStanRules\Collector\ImplementedInterfaceCollector - tags: - - phpstan.collector - - - - class: Symplify\PHPStanRules\Collector\InterfaceOfAbstractClassCollector - tags: - - phpstan.collector diff --git a/phpstan.neon b/phpstan.neon index c04302c2..d5c790e2 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -33,9 +33,6 @@ parameters: # part of public contract - '#Method Symplify\\PHPStanRules\\Tests\\Rules\\(.*?)\\(.*?)Test\:\:testRule\(\) has parameter \$(expectedError(.*?)|expectedErrors) with no value type specified in iterable type array#' - # overly detailed - - '#Class Symplify\\PHPStanRules\\Collector\\(.*?) implements generic interface PHPStan\\Collectors\\Collector but does not specify its types\: TNodeType, TValue#' - # useful to have IDE know the types - identifier: phpstanApi.instanceofType diff --git a/src/Collector/ImplementedInterfaceCollector.php b/src/Collector/ImplementedInterfaceCollector.php deleted file mode 100644 index 727361c2..00000000 --- a/src/Collector/ImplementedInterfaceCollector.php +++ /dev/null @@ -1,42 +0,0 @@ -|null - */ - public function processNode(Node $node, Scope $scope): ?array - { - $implementedInterfaceNames = []; - - // skip abstract classes, as they can enforce child implementations - if ($node->isAbstract()) { - return null; - } - - foreach ($node->implements as $implement) { - $implementedInterfaceNames[] = $implement->toString(); - } - - if ($implementedInterfaceNames === []) { - return null; - } - - return $implementedInterfaceNames; - } -} diff --git a/src/Collector/InterfaceCollector.php b/src/Collector/InterfaceCollector.php deleted file mode 100644 index 04fc1aa8..00000000 --- a/src/Collector/InterfaceCollector.php +++ /dev/null @@ -1,31 +0,0 @@ -namespacedName instanceof Name) { - return null; - } - - return $node->namespacedName->toString(); - } -} diff --git a/src/Collector/InterfaceOfAbstractClassCollector.php b/src/Collector/InterfaceOfAbstractClassCollector.php deleted file mode 100644 index 17c1fa76..00000000 --- a/src/Collector/InterfaceOfAbstractClassCollector.php +++ /dev/null @@ -1,41 +0,0 @@ -|null - */ - public function processNode(Node $node, Scope $scope): ?array - { - if (! $node->isAbstract()) { - return null; - } - - $interfaceNames = []; - - foreach ($node->implements as $implement) { - $interfaceNames[] = $implement->toString(); - } - - if ($interfaceNames === []) { - return null; - } - - return $interfaceNames; - } -} diff --git a/src/Enum/RuleIdentifier.php b/src/Enum/RuleIdentifier.php index 6c3ce298..e3890d45 100644 --- a/src/Enum/RuleIdentifier.php +++ b/src/Enum/RuleIdentifier.php @@ -36,11 +36,6 @@ final class RuleIdentifier */ public const PHP_PARSER_NO_LEADING_BACKSLASH_IN_NAME = 'phpParser.noLeadingBackslashInName'; - /** - * @var string - */ - public const NO_SINGLE_INTERFACE_IMPLEMENTER = 'symplify.noSingleInterfaceImplementer'; - /** * @var string */ diff --git a/src/Rules/NoSingleInterfaceImplementerRule.php b/src/Rules/NoSingleInterfaceImplementerRule.php deleted file mode 100644 index 22c4f04a..00000000 --- a/src/Rules/NoSingleInterfaceImplementerRule.php +++ /dev/null @@ -1,98 +0,0 @@ - - */ -final readonly class NoSingleInterfaceImplementerRule implements Rule -{ - /** - * @api used in test - * @var string - */ - public const ERROR_MESSAGE = 'Interface "%s" has only single implementer. Consider using the class directly as there is no point in using the interface.'; - - public function __construct( - private ReflectionProvider $reflectionProvider - ) { - } - - public function getNodeType(): string - { - return CollectedDataNode::class; - } - - /** - * @param CollectedDataNode $node - */ - public function processNode(Node $node, Scope $scope): array - { - $implementedInterfaces = Arrays::flatten($node->get(ImplementedInterfaceCollector::class)); - $interfaces = Arrays::flatten($node->get(InterfaceCollector::class)); - $interfacesOfAbstractClass = Arrays::flatten($node->get(InterfaceOfAbstractClassCollector::class)); - - $onceUsedInterfaces = $this->resolveOnceUsedInterfaces($implementedInterfaces); - $onceImplementedInterfaces = array_intersect($onceUsedInterfaces, $interfaces); - - // remove the abstract class implemented, as required transitionally - $onceImplementedInterfaces = array_diff($onceImplementedInterfaces, $interfacesOfAbstractClass); - - if ($onceImplementedInterfaces === []) { - return []; - } - - $ruleErrors = []; - foreach ($onceImplementedInterfaces as $onceImplementedInterface) { - $interfaceReflection = $this->reflectionProvider->getClass($onceImplementedInterface); - - // most likely internal - if ($interfaceReflection->getFileName() === null) { - continue; - } - - $ruleErrors[] = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $onceImplementedInterface)) - ->file($interfaceReflection->getFileName()) - ->identifier(RuleIdentifier::NO_SINGLE_INTERFACE_IMPLEMENTER) - ->build(); - } - - return $ruleErrors; - } - - /** - * @param string[] $implementedInterfaces - * @return string[] - */ - private function resolveOnceUsedInterfaces(array $implementedInterfaces): array - { - $onceUsedInterfaces = []; - - $implementedInterfacesToCount = array_count_values($implementedInterfaces); - foreach ($implementedInterfacesToCount as $interfaceName => $countUsed) { - if ($countUsed !== 1) { - continue; - } - - $onceUsedInterfaces[] = $interfaceName; - } - - return $onceUsedInterfaces; - } -} diff --git a/tests/Rules/NoSingleInterfaceImplementerRule/Fixture/AllowAbstract.php b/tests/Rules/NoSingleInterfaceImplementerRule/Fixture/AllowAbstract.php deleted file mode 100644 index a67c3395..00000000 --- a/tests/Rules/NoSingleInterfaceImplementerRule/Fixture/AllowAbstract.php +++ /dev/null @@ -1,9 +0,0 @@ -analyse($filePaths, $expectedErrorMessagesWithLines); - } - - public static function provideData(): Iterator - { - yield [[__DIR__ . '/Fixture/SimpleInterface.php'], []]; - yield [[__DIR__ . '/Fixture/AllowAbstract.php', __DIR__ . '/Fixture/SimpleInterface.php'], []]; - - // already counted in abstract class - yield [[__DIR__ . '/Fixture/AllowAbstract.php', __DIR__ . '/Fixture/SimpleInterface.php', __DIR__ . '/Fixture/ImplementsSimpleInterface.php'], []]; - - yield [ - [ - __DIR__ . '/Fixture/SimpleInterface.php', - __DIR__ . '/Fixture/ImplementsSimpleInterface.php', - ], [ - [ - sprintf(NoSingleInterfaceImplementerRule::ERROR_MESSAGE, SimpleInterface::class), - -1, - ], - ]]; - } - - /** - * @return string[] - */ - public static function getAdditionalConfigFiles(): array - { - return [__DIR__ . '/config/configured_rule.neon']; - } - - protected function getCollectors(): array - { - return [ - self::getContainer()->getByType(ImplementedInterfaceCollector::class), - self::getContainer()->getByType(InterfaceCollector::class), - self::getContainer()->getByType(InterfaceOfAbstractClassCollector::class), - ]; - } - - protected function getRule(): Rule - { - return self::getContainer()->getByType(NoSingleInterfaceImplementerRule::class); - } -} diff --git a/tests/Rules/NoSingleInterfaceImplementerRule/Source/SomeType.php b/tests/Rules/NoSingleInterfaceImplementerRule/Source/SomeType.php deleted file mode 100644 index f0033281..00000000 --- a/tests/Rules/NoSingleInterfaceImplementerRule/Source/SomeType.php +++ /dev/null @@ -1,7 +0,0 @@ -