[rector] add rule to encourage node passing over assignig in rector rules (#244)

Author: TomasVotruba

config/rector-rules.neon

@@ -4,6 +4,7 @@ rules:
     - Symplify\PHPStanRules\Rules\Rector\NoInstanceOfStaticReflectionRule
     - Symplify\PHPStanRules\Rules\Rector\NoLeadingBackslashInNameRule
     - Symplify\PHPStanRules\Rules\Rector\NoClassReflectionStaticReflectionRule
+    - Symplify\PHPStanRules\Rules\Rector\NoPropertyNodeAssignRule
 
 services:
     # $node->getAttribute($1) => Type|null by $1

src/Enum/RuleIdentifier/RectorRuleIdentifier.php

@@ -13,4 +13,6 @@ final class RectorRuleIdentifier
     public const PHP_RULE_IMPLEMENTS_MIN_VERSION = 'rector.phpRuleImplementsMinVersion';
 
     public const NO_CLASS_REFLECTION_STATIC_REFLECTION = 'rector.noClassReflectionStaticReflection';
+
+    public const NO_PROPERTY_NODE_ASSIGN = 'rector.noPropertyNodeAssign';
 }

src/Rules/Rector/NoPropertyNodeAssignRule.php

@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Rector;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\ObjectType;
+use Rector\Contract\Rector\RectorInterface;
+use Symplify\PHPStanRules\Enum\RuleIdentifier\RectorRuleIdentifier;
+
+/**
+ * @see \Symplify\PHPStanRules\Tests\Rules\Rector\NoPropertyNodeAssignRule\NoPropertyNodeAssignRuleTest
+ *
+ * @implements Rule<Assign>
+ */
+final class NoPropertyNodeAssignRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'Avoid assigning a node to property to avoid object juggling, pass it as argument instead';
+
+    public function getNodeType(): string
+    {
+        return Assign::class;
+    }
+
+    /**
+     * @param Assign $node
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! $scope->isInClass()) {
+            return [];
+        }
+
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection->isSubclassOf(RectorInterface::class)) {
+            return [];
+        }
+
+        if (! $node->var instanceof PropertyFetch) {
+            return [];
+        }
+
+        $propertyFetch = $node->var;
+        if (! $propertyFetch->var instanceof Variable) {
+            return [];
+        }
+
+        if (! is_string($propertyFetch->var->name)) {
+            return [];
+        }
+
+        if ($propertyFetch->var->name !== 'this') {
+            return [];
+        }
+
+        $assignedExprType = $scope->getType($node->expr);
+        if (! $assignedExprType instanceof ObjectType) {
+            return [];
+        }
+
+        if (! $assignedExprType->isInstanceOf(Node::class)->yes()) {
+            return [];
+        }
+
+        $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
+            ->identifier(RectorRuleIdentifier::NO_PROPERTY_NODE_ASSIGN)
+            ->build();
+
+        return [$identifierRuleError];
+    }
+}

tests/Rules/Rector/NoPropertyNodeAssignRule/Fixture/SkipNoRectorAssign.php

@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\NoPropertyNodeAssignRule\Fixture;
+
+use PhpParser\Node;
+
+final class SkipNoRectorAssign
+{
+    private ?\PhpParser\Node $localNode = null;
+
+    public function refactor(Node $node)
+    {
+        $this->localNode = $node;
+    }
+}

tests/Rules/Rector/NoPropertyNodeAssignRule/Fixture/SomeNodePropertyAssign.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\NoPropertyNodeAssignRule\Fixture;
+
+use PhpParser\Node;
+use Rector\Rector\AbstractRector;
+
+final class SomeNodePropertyAssign extends AbstractRector
+{
+    private ?\PhpParser\Node $localNode = null;
+
+    public function getNodeTypes(): array
+    {
+    }
+
+    public function refactor(Node $node)
+    {
+        $this->localNode = $node;
+    }
+}

tests/Rules/Rector/NoPropertyNodeAssignRule/NoPropertyNodeAssignRuleTest.php

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

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

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