[rector] add AvoidFeatureSetAttributeInRectorRule

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,101 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Rector;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Stmt\Class_;
+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<Class_>
+ */
+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/Rector/AvoidFeatureSetAttributeInRectorRule/AvoidFeatureSetAttributeInRectorRuleTest.php

@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\AvoidFeatureSetAttributeInRectorRule;
+
+use Iterator;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symplify\PHPStanRules\Rules\Rector\AvoidFeatureSetAttributeInRectorRule;
+
+final class AvoidFeatureSetAttributeInRectorRuleTest extends RuleTestCase
+{
+    #[DataProvider('provideData')]
+    public function testRule(string $filePath, array $expectedErrorsWithLines): void
+    {
+        $this->analyse([$filePath], $expectedErrorsWithLines);
+    }
+
+    public static function provideData(): Iterator
+    {
+        yield [__DIR__ . '/Fixture/SetLocalAttribute.php', [[
+            sprintf(AvoidFeatureSetAttributeInRectorRule::ERROR_MESSAGE, 'some_attribute'), 11,
+        ]]];
+
+        yield [__DIR__ . '/Fixture/SkipAllowedSetAttributesNode.php', []];
+    }
+
+    protected function getRule(): Rule
+    {
+        return new AvoidFeatureSetAttributeInRectorRule();
+    }
+}

tests/Rules/Rector/AvoidFeatureSetAttributeInRectorRule/Fixture/SetLocalAttribute.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\AvoidFeatureSetAttributeInRectorRule\Fixture;
+
+use PhpParser\Node;
+use Rector\Rector\AbstractRector;
+use ReflectionClass;
+
+final class SetLocalAttribute extends AbstractRector
+{
+    public function getNodeTypes(): array
+    {
+        return [Node\Stmt\Class_::class];
+    }
+
+    public function refactor(Node $node)
+    {
+        $node->setAttribute('some_attribute', 'some_value');
+
+        return null;
+    }
+}

tests/Rules/Rector/AvoidFeatureSetAttributeInRectorRule/Fixture/SkipAllowedSetAttributesNode.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\AvoidFeatureSetAttributeInRectorRule\Fixture;
+
+use PhpParser\Node;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\Rector\AbstractRector;
+use ReflectionClass;
+
+final class SkipAllowedSetAttributesNode extends AbstractRector
+{
+    public function getNodeTypes(): array
+    {
+        return [Node\Stmt\Class_::class];
+    }
+
+    public function refactor(Node $node)
+    {
+        $node->setAttribute(AttributeKey::ORIGINAL_NODE, null);
+        $node->setAttribute(AttributeKey::KIND, 1);
+
+        return null;
+    }
+}

tests/Rules/Rector/AvoidFeatureSetAttributeInRectorRule/config/configured_rule.neon

@@ -0,0 +1,2 @@
+rules:
+    - Symplify\PHPStanRules\Rules\Rector\AvoidFeatureSetAttributeInRectorRule