diff --git a/README.md b/README.md index b9446499..7c557926 100644 --- a/README.md +++ b/README.md @@ -2885,6 +2885,41 @@ public function __construct(
+### PreferClassServiceReferenceRule + +In a Symfony PHP config closure, when a `$services->alias('some.helper', SomeHelper::class)` points a string id at a class, a `service('some.helper')` reference should name the service by its class instead - `service(SomeHelper::class)`. Then the string alias nothing else asks for can be dropped. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\PreferClassServiceReferenceRule +``` + +```php +$services->alias('some.helper', SomeHelper::class); + +$services->set(SomeConsumer::class) + ->args([service('some.helper')]); +``` + +:x: + +
+ +```php +$services->alias('some.helper', SomeHelper::class); + +$services->set(SomeConsumer::class) + ->args([service(SomeHelper::class)]); +``` + +:+1: + +
+ +--- + +
+ ## 4. PHPUnit-specific Rules ### NoAssertFuncCallInTestsRule diff --git a/config/symfony-config-rules.neon b/config/symfony-config-rules.neon index e79a0ee3..2a9af87a 100644 --- a/config/symfony-config-rules.neon +++ b/config/symfony-config-rules.neon @@ -22,3 +22,14 @@ rules: # $services->set('X')->class('X') - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule + + # service('id') where an alias points 'id' at a class + - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\PreferClassServiceReferenceRule + +services: + - + class: Symplify\PHPStanRules\Collector\ClassTargetServiceAliasCollector + tags: [phpstan.collector] + - + class: Symplify\PHPStanRules\Collector\ServiceStringReferenceCollector + tags: [phpstan.collector] diff --git a/src/Collector/ClassTargetServiceAliasCollector.php b/src/Collector/ClassTargetServiceAliasCollector.php new file mode 100644 index 00000000..9392c585 --- /dev/null +++ b/src/Collector/ClassTargetServiceAliasCollector.php @@ -0,0 +1,72 @@ +alias('some.helper', SomeHelper::class). + * + * Such an id names the very same service its class name does, so a reference by the class name says the same + * without the loose string. The alias the other way around, $services->alias(SomeHelper::class, 'some.helper'), + * is left out. + * + * @implements Collector + */ +final class ClassTargetServiceAliasCollector implements Collector +{ + public function getNodeType(): string + { + return MethodCall::class; + } + + /** + * @return array{string, string, int}|null the string service id, the class name it points at and the + * line of the alias() call + */ + public function processNode(Node $node, Scope $scope): ?array + { + if (! $node->name instanceof Identifier || $node->name->toString() !== 'alias') { + return null; + } + + $args = $node->getArgs(); + if (count($args) !== 2) { + return null; + } + + if (! $args[0]->value instanceof String_) { + return null; + } + + $className = $this->matchClassName($args[1]->value); + if ($className === null) { + return null; + } + + return [$args[0]->value->value, $className, $node->getStartLine()]; + } + + private function matchClassName(Node $aliasValue): ?string + { + if (! $aliasValue instanceof ClassConstFetch || ! $aliasValue->class instanceof Name) { + return null; + } + + if (! $aliasValue->name instanceof Identifier || $aliasValue->name->toLowerString() !== 'class') { + return null; + } + + return $aliasValue->class->toString(); + } +} diff --git a/src/Collector/ServiceStringReferenceCollector.php b/src/Collector/ServiceStringReferenceCollector.php new file mode 100644 index 00000000..b9361ce6 --- /dev/null +++ b/src/Collector/ServiceStringReferenceCollector.php @@ -0,0 +1,53 @@ + + */ +final class ServiceStringReferenceCollector implements Collector +{ + private const string SERVICE_FUNCTION = 'Symfony\Component\DependencyInjection\Loader\Configurator\service'; + + public function getNodeType(): string + { + return FuncCall::class; + } + + /** + * @return array{string, int}|null the string service id with the line the reference is on + */ + public function processNode(Node $node, Scope $scope): ?array + { + if (! $node->name instanceof Name) { + return null; + } + + $functionName = $node->name->toString(); + if ($functionName !== 'service' && $functionName !== self::SERVICE_FUNCTION) { + return null; + } + + $firstArg = $node->getArgs()[0] ?? null; + if (! $firstArg instanceof Arg || ! $firstArg->value instanceof String_) { + return null; + } + + return [$firstArg->value->value, $node->getStartLine()]; + } +} diff --git a/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php b/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php index c1b8f8b1..f24f685b 100644 --- a/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php +++ b/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php @@ -65,4 +65,6 @@ final class SymfonyRuleIdentifier public const string NO_CONTROLLER_METHOD_INJECTION = 'symfony.noControllerMethodInjection'; public const string FILE_NAME_MATCHES_EXTENSION = 'symfony.fileNameMatchesExtension'; + + public const string PREFER_CLASS_SERVICE_REFERENCE = 'symfony.preferClassServiceReference'; } diff --git a/src/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule.php b/src/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule.php new file mode 100644 index 00000000..89a42301 --- /dev/null +++ b/src/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule.php @@ -0,0 +1,93 @@ +alias('some.helper', SomeHelper::class) is + * registered. + * + * The class name names the very same service, so the reference should say the same by the type instead: + * + * $services->alias('some.helper', SomeHelper::class); + * ... + * ->args([service('some.helper')]); + * + * ->args([service(SomeHelper::class)]); + * + * @see \Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferClassServiceReferenceRule\PreferClassServiceReferenceRuleTest + * + * @implements Rule + */ +final class PreferClassServiceReferenceRule implements Rule +{ + public const string ERROR_MESSAGE = 'Reference the service by its class, service(%s::class), rather than by the string id "%s" a class name alias already covers'; + + public function getNodeType(): string + { + return CollectedDataNode::class; + } + + /** + * @param CollectedDataNode $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + $classNamesByServiceId = $this->resolveClassNamesByServiceId($node); + + /** @var array> $referencesByFilePath */ + $referencesByFilePath = $node->get(ServiceStringReferenceCollector::class); + + $ruleErrors = []; + + foreach ($referencesByFilePath as $filePath => $references) { + foreach ($references as [$serviceId, $line]) { + $className = $classNamesByServiceId[$serviceId] ?? null; + if ($className === null) { + continue; + } + + $ruleErrors[] = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $className, $serviceId)) + ->identifier(SymfonyRuleIdentifier::PREFER_CLASS_SERVICE_REFERENCE) + ->file($filePath) + ->line($line) + ->build(); + } + } + + return $ruleErrors; + } + + /** + * @return array the class name every string service id is aliased to + */ + private function resolveClassNamesByServiceId(CollectedDataNode $collectedDataNode): array + { + /** @var array> $aliasesByFilePath */ + $aliasesByFilePath = $collectedDataNode->get(ClassTargetServiceAliasCollector::class); + + $classNamesByServiceId = []; + + foreach ($aliasesByFilePath as $aliases) { + foreach ($aliases as [$serviceId, $className]) { + $classNamesByServiceId[$serviceId] = $className; + } + } + + return $classNamesByServiceId; + } +} diff --git a/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/SkipClassServiceReferenceConfig.php b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/SkipClassServiceReferenceConfig.php new file mode 100644 index 00000000..2d2be44b --- /dev/null +++ b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/SkipClassServiceReferenceConfig.php @@ -0,0 +1,16 @@ +services(); + + $services->alias('some.helper', SomeHelper::class); + + $services->set(SomeHelper::class) + ->args([service(SomeHelper::class)]); +}; diff --git a/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/SkipUnaliasedStringReferenceConfig.php b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/SkipUnaliasedStringReferenceConfig.php new file mode 100644 index 00000000..ae06e144 --- /dev/null +++ b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/SkipUnaliasedStringReferenceConfig.php @@ -0,0 +1,14 @@ +services(); + + $services->set(SomeHelper::class) + ->args([service('unaliased.service')]); +}; diff --git a/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/StringServiceReferenceConfig.php b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/StringServiceReferenceConfig.php new file mode 100644 index 00000000..55fdaeb8 --- /dev/null +++ b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Fixture/StringServiceReferenceConfig.php @@ -0,0 +1,16 @@ +services(); + + $services->alias('some.helper', SomeHelper::class); + + $services->set(SomeHelper::class) + ->args([service('some.helper')]); +}; diff --git a/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/PreferClassServiceReferenceRuleTest.php b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/PreferClassServiceReferenceRuleTest.php new file mode 100644 index 00000000..d6619cd8 --- /dev/null +++ b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/PreferClassServiceReferenceRuleTest.php @@ -0,0 +1,67 @@ + $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + $errorMessage = sprintf(PreferClassServiceReferenceRule::ERROR_MESSAGE, SomeHelper::class, 'some.helper'); + yield [__DIR__ . '/Fixture/StringServiceReferenceConfig.php', [[$errorMessage, 15]]]; + + yield [__DIR__ . '/Fixture/SkipClassServiceReferenceConfig.php', []]; + yield [__DIR__ . '/Fixture/SkipUnaliasedStringReferenceConfig.php', []]; + } + + /** + * @return string[] + */ + #[Override] + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/config/configured_rule.neon']; + } + + protected function getRule(): Rule + { + return self::getContainer()->getByType(PreferClassServiceReferenceRule::class); + } + + /** + * @return list> + */ + #[Override] + protected function getCollectors(): array + { + return [ + self::getContainer()->getByType(ClassTargetServiceAliasCollector::class), + self::getContainer()->getByType(ServiceStringReferenceCollector::class), + ]; + } +} diff --git a/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Source/SomeHelper.php b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Source/SomeHelper.php new file mode 100644 index 00000000..20cfe98c --- /dev/null +++ b/tests/Rules/Symfony/ConfigClosure/PreferClassServiceReferenceRule/Source/SomeHelper.php @@ -0,0 +1,9 @@ +