[symfony] add NoSetClassServiceDuplicationRule (#237)

Author: TomasVotruba

config/symfony-config-rules.neon

@@ -16,3 +16,6 @@ rules:
 
     # $services->set('X', 'X')
     - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoServiceSameNameSetClassRule
+
+    # $services->set('X')->class('X')
+    - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule

src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php

@@ -59,4 +59,6 @@ final class SymfonyRuleIdentifier
     public const PREFER_AUTOWIRE_ATTRIBUTE_OVER_CONFIG_PARAM = 'symfony.preferAutowireAttributeOverConfigParam';
 
     public const RULE_IDENTIFIER = 'symfony.noServiceAutowireDuplicate';
+
+    public const NO_SET_CLASS_SERVICE_DUPLICATE = 'symfony.noSetClassServiceDuplicate';
 }

src/Rules/Symfony/ConfigClosure/NoSetClassServiceDuplicationRule.php

@@ -0,0 +1,117 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Symfony\ConfigClosure;
+
+use Nette\Utils\Strings;
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Identifier;
+use PhpParser\PrettyPrinter\Standard;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
+use Symplify\PHPStanRules\Enum\RuleIdentifier\SymfonyRuleIdentifier;
+
+/**
+ * @implements Rule<MethodCall>
+ */
+final readonly class NoSetClassServiceDuplicationRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'Instead of "$services->set(%s)->class(%s)" that brings no value, use simple $services->set(%s)';
+
+    private Standard $standard;
+
+    public function __construct()
+    {
+        $this->standard = new Standard();
+    }
+
+    public function getNodeType(): string
+    {
+        return MethodCall::class;
+    }
+
+    /**
+     * @param MethodCall $node
+     * @return RuleError[]
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if ($node->isFirstClassCallable()) {
+            return [];
+        }
+
+        // parent method must be a method call too
+        if (! $node->var instanceof MethodCall) {
+            return [];
+        }
+
+        if (! $this->isMethodName($node->name, 'class')) {
+            return [];
+        }
+
+        $parentMethodCall = $node->var;
+        if (! $this->isMethodName($parentMethodCall->name, 'set')) {
+            return [];
+        }
+
+        $parentSoleArgContents = $this->resolveSoleArgContents($parentMethodCall);
+        if ($parentSoleArgContents === null) {
+            return [];
+        }
+
+        $currentSoleArgContents = $this->resolveSoleArgContents($node);
+        if ($currentSoleArgContents === null) {
+            return [];
+        }
+
+        if ($parentSoleArgContents !== $currentSoleArgContents) {
+            return [];
+        }
+
+        if (str_contains($parentSoleArgContents, '\\')) {
+            $shortClassName = Strings::after($parentSoleArgContents, '\\', -1);
+        } else {
+            $shortClassName = $parentSoleArgContents;
+        }
+
+        $errorMessage = sprintf(
+            self::ERROR_MESSAGE,
+            $shortClassName,
+            $shortClassName,
+            $shortClassName
+        );
+
+        $identifierRuleError = RuleErrorBuilder::message($errorMessage)
+            ->identifier(SymfonyRuleIdentifier::NO_SET_CLASS_SERVICE_DUPLICATE)
+            ->build();
+
+        return [$identifierRuleError];
+    }
+
+    private function isMethodName(Node $node, string $name): bool
+    {
+        if (! $node instanceof Identifier) {
+            return false;
+        }
+
+        return $node->toString() === $name;
+    }
+
+    private function resolveSoleArgContents(MethodCall $methodCall): ?string
+    {
+        if (count($methodCall->getArgs()) !== 1) {
+            return null;
+        }
+
+        $firstArgExpr = $methodCall->getArgs()[0]
+            ->value;
+        return $this->standard->prettyPrintExpr($firstArgExpr);
+    }
+}

tests/Rules/Symfony/ConfigClosure/NoSetClassServiceDuplicationRule/Fixture/SetAndClassConfig.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
+
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+use Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule\Source\SomeClassToBeSet;
+
+return function (ContainerConfigurator $container) {
+    $services = $container->services();
+
+    $services->set(SomeClassToBeSet::class)
+        ->class(SomeClassToBeSet::class);
+};

tests/Rules/Symfony/ConfigClosure/NoSetClassServiceDuplicationRule/Fixture/SkipDifferentSetAndClass.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
+
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+use Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule\Source\SomeClassToBeSet;
+
+return function (ContainerConfigurator $container) {
+    $services = $container->services();
+
+    $services->set(SomeClassToBeSet::class)
+        ->class('AnotherValue');
+};

tests/Rules/Symfony/ConfigClosure/NoSetClassServiceDuplicationRule/Fixture/SkipSoleSet.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
+
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+use Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule\Source\SomeClassToBeSet;
+
+return function (ContainerConfigurator $container) {
+    $services = $container->services();
+
+    $services->set(SomeClassToBeSet::class);
+};

tests/Rules/Symfony/ConfigClosure/NoSetClassServiceDuplicationRule/NoSetClassServiceDuplicationRuleTest.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule;
+
+use Iterator;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule;
+
+final class NoSetClassServiceDuplicationRuleTest extends RuleTestCase
+{
+    /**
+     * @param mixed[] $expectedErrorMessagesWithLines
+     */
+    #[DataProvider('provideData')]
+    public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
+    {
+        $this->analyse([$filePath], $expectedErrorMessagesWithLines);
+    }
+
+    public static function provideData(): Iterator
+    {
+        yield [__DIR__ . '/Fixture/SetAndClassConfig.php', [
+            [
+                sprintf(NoSetClassServiceDuplicationRule::ERROR_MESSAGE, 'SomeClassToBeSet::class', 'SomeClassToBeSet::class', 'SomeClassToBeSet::class'),
+                11,
+            ],
+        ]];
+
+        yield [__DIR__ . '/Fixture/SkipDifferentSetAndClass.php', []];
+        yield [__DIR__ . '/Fixture/SkipSoleSet.php', []];
+    }
+
+    protected function getRule(): Rule
+    {
+        return new NoSetClassServiceDuplicationRule();
+    }
+}

tests/Rules/Symfony/ConfigClosure/NoSetClassServiceDuplicationRule/Source/SomeClassToBeSet.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\NoSetClassServiceDuplicationRule\Source;
+
+final class SomeClassToBeSet
+{
+
+}