[rector] add AvoidFeatureSetAttributeInRectorRule (#252)

* [rector] add AvoidFeatureSetAttributeInRectorRule

* restore interface

Author: TomasVotruba

composer.json

@@ -16,7 +16,7 @@
         "symfony/framework-bundle": "6.1.*",
         "phpecs/phpecs": "^2.2",
         "tomasvotruba/class-leak": "^2.1",
-        "rector/rector": "^2.2",
+        "rector/rector": "^2.2.11",
         "phpstan/extension-installer": "^1.4",
         "symplify/phpstan-extensions": "^12.0",
         "tomasvotruba/unused-public": "^2.1",

config/rector-rules.neon

@@ -8,6 +8,7 @@ rules:
     - Symplify\PHPStanRules\Rules\Rector\PreferDirectIsNameRule
     - Symplify\PHPStanRules\Rules\Rector\NoOnlyNullReturnInRefactorRule
     - Symplify\PHPStanRules\Rules\Rector\NoIntegerRefactorReturnRule
+    - Symplify\PHPStanRules\Rules\Rector\AvoidFeatureSetAttributeInRectorRule
 
 services:
     # $node->getAttribute($1) => Type|null by $1

src/Enum/RuleIdentifier/RectorRuleIdentifier.php

@@ -21,4 +21,6 @@ final class RectorRuleIdentifier
     public const NO_ONLY_NULL_RETURN_IN_REFACTOR = 'rector.noOnlyNullReturnInRefactor';
 
     public const NO_INTEGER_REFACTOR_RETURN = 'rector.noIntegerRefactorReturn';
+
+    public const AVOID_FEATURE_SET_ATTRIBUTE_IN_RECTOR = 'rector.avoidFeatureSetAttributeInRector';
 }

src/Rules/Rector/AvoidFeatureSetAttributeInRectorRule.php

@@ -0,0 +1,100 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Rector;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\NodeFinder;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\InClassNode;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\Constant\ConstantStringType;
+use Rector\Rector\AbstractRector;
+use Symplify\PHPStanRules\Enum\RuleIdentifier\RectorRuleIdentifier;
+use Symplify\PHPStanRules\Helper\NamingHelper;
+
+/**
+ * @see \Symplify\PHPStanRules\Tests\Rules\Rector\AvoidFeatureSetAttributeInRectorRule\AvoidFeatureSetAttributeInRectorRuleTest
+ *
+ * @implements Rule<InClassNode>
+ */
+final class AvoidFeatureSetAttributeInRectorRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'Instead of using Rector rule to setAttribute("%s") to be used later, create a service extending "DecoratingNodeVisitorInterface". This ensures attribute decoration and node changes are in 2 separated steps.';
+
+    /**
+     * @var string[]
+     */
+    private const ALLOWED_ATTRIBUTES = ['kind', 'origNode', 'comments', 'startLine', 'endLine', 'startTokenPos', 'endTokenPos'];
+
+    public function getNodeType(): string
+    {
+        return InClassNode::class;
+    }
+
+    /**
+     * @param InClassNode $node
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return [];
+        }
+
+        if (! $classReflection->is(AbstractRector::class)) {
+            return [];
+        }
+
+        $classLike = $node->getOriginalNode();
+
+        $nodeFinder = new NodeFinder();
+
+        /** @var MethodCall[] $methodCalls */
+        $methodCalls = $nodeFinder->findInstanceOf($classLike, MethodCall::class);
+
+        $ruleErrors = [];
+
+        foreach ($methodCalls as $methodCall) {
+            if (! NamingHelper::isName($methodCall->name, 'setAttribute')) {
+                continue;
+            }
+
+            $attributeName = $this->resolveAttributeKeyValue($methodCall, $scope);
+            if (! is_string($attributeName)) {
+                continue;
+            }
+
+            if (in_array($attributeName, self::ALLOWED_ATTRIBUTES, true)) {
+                continue;
+            }
+
+            $ruleError = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $attributeName))
+                ->identifier(RectorRuleIdentifier::AVOID_FEATURE_SET_ATTRIBUTE_IN_RECTOR)
+                ->build();
+
+            $ruleErrors[] = $ruleError;
+        }
+
+        return $ruleErrors;
+    }
+
+    private function resolveAttributeKeyValue(MethodCall $methodCall, Scope $scope): ?string
+    {
+        $firstArg = $methodCall->getArgs()[0];
+        $attributeNameType = $scope->getType($firstArg->value);
+
+        if (! $attributeNameType instanceof ConstantStringType) {
+            return null;
+        }
+
+        return $attributeNameType->getValue();
+    }
+}

tests/Rules/Doctrine/NoDoctrineListenerWithoutContractRule/NoDoctrineListenerWithoutContractRuleTest.php

@@ -5,6 +5,7 @@
 namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\NoDoctrineListenerWithoutContractRule;
 
 use Iterator;
+use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Symplify\PHPStanRules\Rules\Doctrine\NoDoctrineListenerWithoutContractRule;
@@ -31,7 +32,7 @@ public static function provideData(): Iterator
         ]]];
     }
 
-    protected function getRule(): NoDoctrineListenerWithoutContractRule
+    protected function getRule(): Rule
     {
         return new NoDoctrineListenerWithoutContractRule();
     }

tests/Rules/Doctrine/NoGetRepositoryOnServiceRepositoryEntityRule/NoGetRepositoryOnServiceRepositoryEntityRuleTest.php

@@ -6,6 +6,7 @@
 
 use Iterator;
 use PHPStan\Reflection\ReflectionProvider;
+use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Symplify\PHPStanRules\Rules\Doctrine\NoGetRepositoryOnServiceRepositoryEntityRule;
@@ -39,7 +40,7 @@ public static function provideData(): Iterator
         yield [__DIR__ . '/Fixture/SkipGetRepositoryOnNormalRepository.php', []];
     }
 
-    protected function getRule(): NoGetRepositoryOnServiceRepositoryEntityRule
+    protected function getRule(): Rule
     {
         $reflectionProvider = self::getContainer()->getByType(ReflectionProvider::class);
 

tests/Rules/Doctrine/NoGetRepositoryOutsideServiceRule/NoGetRepositoryOutsideServiceRuleTest.php

@@ -5,6 +5,7 @@
 namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\NoGetRepositoryOutsideServiceRule;
 
 use Iterator;
+use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Symplify\PHPStanRules\Rules\Doctrine\NoGetRepositoryOutsideServiceRule;
@@ -37,7 +38,7 @@ public static function provideData(): Iterator
         yield [__DIR__ . '/Fixture/SkipDynamicClassConstFetch.php', []];
     }
 
-    protected function getRule(): NoGetRepositoryOutsideServiceRule
+    protected function getRule(): Rule
     {
         return new NoGetRepositoryOutsideServiceRule();
     }

tests/Rules/Doctrine/NoParentRepositoryRule/NoParentRepositoryRuleTest.php

@@ -5,6 +5,7 @@
 namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\NoParentRepositoryRule;
 
 use Iterator;
+use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Symplify\PHPStanRules\Rules\Doctrine\NoParentRepositoryRule;
@@ -22,7 +23,7 @@ public static function provideData(): Iterator
         yield [__DIR__ . '/Fixture/SomeRepository.php', [[NoParentRepositoryRule::ERROR_MESSAGE, 9]]];
     }
 
-    protected function getRule(): NoParentRepositoryRule
+    protected function getRule(): Rule
     {
         return new NoParentRepositoryRule();
     }

tests/Rules/Doctrine/NoRepositoryCallInDataFixtureRule/NoRepositoryCallInDataFixtureRuleTest.php

@@ -5,6 +5,7 @@
 namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\NoRepositoryCallInDataFixtureRule;
 
 use Iterator;
+use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Symplify\PHPStanRules\Rules\Doctrine\NoRepositoryCallInDataFixtureRule;
@@ -30,7 +31,7 @@ public static function provideData(): Iterator
         yield [__DIR__ . '/Fixture/SkipNonFixtureClass.php', []];
     }
 
-    protected function getRule(): NoRepositoryCallInDataFixtureRule
+    protected function getRule(): Rule
     {
         return new NoRepositoryCallInDataFixtureRule();
     }

tests/Rules/Doctrine/RequireQueryBuilderOnRepositoryRule/RequireQueryBuilderOnRepositoryRuleTest.php

@@ -5,6 +5,7 @@
 namespace Symplify\PHPStanRules\Tests\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule;
 
 use Iterator;
+use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Symplify\PHPStanRules\Rules\Doctrine\RequireQueryBuilderOnRepositoryRule;
@@ -32,7 +33,7 @@ public static function provideData(): Iterator
         ]];
     }
 
-    protected function getRule(): RequireQueryBuilderOnRepositoryRule
+    protected function getRule(): Rule
     {
         return new RequireQueryBuilderOnRepositoryRule();
     }