[symfony] Add NoConstructorAndRequiredTogetherRule (#168)

* [symfony] Add NoConstructorAndRequiredTogetherRule

* docs

Author: TomasVotruba

README.md

@@ -1177,6 +1177,17 @@ final class SomeFixture extends AbstractFixture
 
 ## 3. Symfony-specific Rules
 
+### NoConstructorAndRequiredTogetherRule
+
+Constructor injection and `#[Required]` method should not be used together in single class. Pick one, to keep architecture clean.
+
+```yaml
+rules:
+    - Symplify\PHPStanRules\Rules\Symfony\NoConstructorAndRequiredTogetherRule
+```
+
+<br>
+
 ### NoGetDoctrineInControllerRule
 
 Prevents using `$this->getDoctrine()` in controllers, to promote dependency injection.

src/Enum/MethodName.php

@@ -10,4 +10,6 @@ final class MethodName
      * @var string
      */
     public const INVOKE = '__invoke';
+
+    public const CONSTRUCTOR = '__construct';
 }

src/Enum/SymfonyRuleIdentifier.php

@@ -25,4 +25,6 @@ final class SymfonyRuleIdentifier
     public const SINGLE_REQUIRED_METHOD = 'symfony.singleRequiredMethod';
 
     public const SYMFONY_REQUIRED_ONLY_IN_ABSTRACT = 'symfony.requiredOnlyInAbstract';
+
+    public const NO_CONSTRUCT_AND_REQUIRED = 'symfony.noConstructAndRequired';
 }

src/Rules/Symfony/NoConstructorAndRequiredTogetherRule.php

@@ -0,0 +1,87 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Symfony;
+
+use PhpParser\Comment\Doc;
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Symplify\PHPStanRules\Enum\MethodName;
+use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier;
+
+/**
+ * @implements Rule<Class_>
+ *
+ * @see \Symplify\PHPStanRules\Tests\Rules\Symfony\NoConstructorAndRequiredTogetherRule\NoConstructorAndRequiredTogetherRuleTest
+ */
+final class NoConstructorAndRequiredTogetherRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'Avoid using __construct() and @required in the same class. Pick one to keep architecture clean';
+
+    public function getNodeType(): string
+    {
+        return Class_::class;
+    }
+
+    /**
+     * @param Class_ $node
+     * @return IdentifierRuleError[]
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if ($node->isAnonymous()) {
+            return [];
+        }
+
+        if (! $node->getMethod(MethodName::CONSTRUCTOR)) {
+            return [];
+        }
+
+        if (! $this->hasAutowiredMethod($node)) {
+            return [];
+        }
+
+        $ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
+            ->identifier(SymfonyRuleIdentifier::NO_CONSTRUCT_AND_REQUIRED)
+            ->build();
+
+        return [
+            $ruleError,
+        ];
+    }
+
+    private function hasAutowiredMethod(Class_ $class): bool
+    {
+        foreach ($class->getMethods() as $classMethod) {
+            if (! $classMethod->isPublic()) {
+                continue;
+            }
+
+            $docComment = $classMethod->getDocComment();
+            if (! $docComment instanceof Doc) {
+                continue;
+            }
+
+            if (! str_contains($docComment->getText(), '@required')) {
+                continue;
+            }
+
+            // special case when its allowed, to avoid circular references
+            if (str_contains($docComment->getText(), 'circular')) {
+                continue;
+            }
+
+            return true;
+        }
+
+        return false;
+    }
+}

tests/Rules/Symfony/NoConstructorAndRequiredTogetherRule/Fixture/ConstructorAndRequiredInSingleClass.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoConstructorAndRequiredTogetherRule\Fixture;
+
+final class ConstructorAndRequiredInSingleClass
+{
+    public function __construct()
+    {
+    }
+
+    /**
+     * @required
+     */
+    public function someRequired()
+    {
+
+    }
+}

tests/Rules/Symfony/NoConstructorAndRequiredTogetherRule/Fixture/SkipCircularDependencyPrevention.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoConstructorAndRequiredTogetherRule\Fixture;
+
+final class SkipCircularDependencyPrevention
+{
+    public function __construct()
+    {
+    }
+
+    /**
+     * Avoid circular dependency
+     * @required
+     */
+    public function someRequired()
+    {
+
+    }
+}

tests/Rules/Symfony/NoConstructorAndRequiredTogetherRule/Fixture/SkipOtherSoleMethod.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoConstructorAndRequiredTogetherRule\Fixture;
+
+final class SkipOtherSoleMethod
+{
+    /**
+     * @required
+     */
+    public function autowire()
+    {
+    }
+}

tests/Rules/Symfony/NoConstructorAndRequiredTogetherRule/Fixture/SkipSoleMethod.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoConstructorAndRequiredTogetherRule\Fixture;
+
+final class SkipSoleMethod
+{
+    public function __construct()
+    {
+    }
+}

tests/Rules/Symfony/NoConstructorAndRequiredTogetherRule/NoConstructorAndRequiredTogetherRuleTest.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoConstructorAndRequiredTogetherRule;
+
+use Iterator;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symplify\PHPStanRules\Rules\Symfony\NoConstructorAndRequiredTogetherRule;
+
+final class NoConstructorAndRequiredTogetherRuleTest 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/ConstructorAndRequiredInSingleClass.php', [[
+            NoConstructorAndRequiredTogetherRule::ERROR_MESSAGE,
+            7,
+        ]]];
+
+        yield [__DIR__ . '/Fixture/SkipSoleMethod.php', []];
+        yield [__DIR__ . '/Fixture/SkipOtherSoleMethod.php', []];
+
+        yield [__DIR__ . '/Fixture/SkipCircularDependencyPrevention.php', []];
+    }
+
+    protected function getRule(): Rule
+    {
+        return new NoConstructorAndRequiredTogetherRule();
+    }
+}