[rector] add NoOnlyNullReturnInRefactorRule (#247)

Author: TomasVotruba

config/rector-rules.neon

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

src/Enum/RuleIdentifier/RectorRuleIdentifier.php

@@ -17,4 +17,6 @@ final class RectorRuleIdentifier
     public const NO_PROPERTY_NODE_ASSIGN = 'rector.noPropertyNodeAssign';
 
     public const PREFER_DIRECT_IS_NAME = 'rector.preferDirectIsName';
+
+    public const NO_ONLY_NULL_RETURN_IN_REFACTOR = 'rector.noOnlyNullReturnInRefactor';
 }

src/Rules/Rector/NoOnlyNullReturnInRefactorRule.php

@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Rector;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\ConstFetch;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Return_;
+use PhpParser\NodeFinder;
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Rector\Rector\AbstractRector;
+use Symplify\PHPStanRules\Enum\RuleIdentifier\RectorRuleIdentifier;
+use Symplify\PHPStanRules\Helper\NamingHelper;
+
+/**
+ * @see \Symplify\PHPStanRules\Tests\Rules\Rector\NoOnlyNullReturnInRefactorRule\NoOnlyNullReturnInRefactorRuleTest
+ *
+ * @implements Rule<ClassMethod>
+ */
+final class NoOnlyNullReturnInRefactorRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'The refactor() method returns always null, but it should return at least one modified node';
+
+    public function getNodeType(): string
+    {
+        return ClassMethod::class;
+    }
+
+    /**
+     * @param ClassMethod $node
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! NamingHelper::isName($node->name, 'refactor')) {
+            return [];
+        }
+
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return [];
+        }
+
+        if (! $classReflection->is(AbstractRector::class)) {
+            return [];
+        }
+
+        $nodeFinder = new NodeFinder();
+        $returns = $nodeFinder->findInstanceOf((array) $node->stmts, Return_::class);
+
+        // should not happen, but out of scope of this PR
+        if ($returns === []) {
+            return [];
+        }
+
+        foreach ($returns as $return) {
+            if (! $return->expr instanceof ConstFetch) {
+                return [];
+            }
+
+            if ($return->expr->name->toLowerString() !== 'null') {
+                return [];
+            }
+        }
+
+        return [RuleErrorBuilder::message(self::ERROR_MESSAGE)
+            ->identifier(RectorRuleIdentifier::NO_ONLY_NULL_RETURN_IN_REFACTOR)
+            ->build()];
+    }
+}

tests/Rules/Rector/NoOnlyNullReturnInRefactorRule/Fixture/RefactorOnlyReturnNull.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\NoOnlyNullReturnInRefactorRule\Fixture;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\If_;
+use Rector\Rector\AbstractRector;
+
+final class RefactorOnlyReturnNull extends AbstractRector
+{
+    public function getNodeTypes(): array
+    {
+        return [];
+    }
+
+    public function refactor(Node $node)
+    {
+        if ($node instanceof If_) {
+            return null;
+        }
+
+        return null;
+    }
+}

tests/Rules/Rector/NoOnlyNullReturnInRefactorRule/Fixture/SkipOtherReturnThanNull.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\NoOnlyNullReturnInRefactorRule\Fixture;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\If_;
+use Rector\Rector\AbstractRector;
+
+final class SkipOtherReturnThanNull extends AbstractRector
+{
+    public function getNodeTypes(): array
+    {
+        return [];
+    }
+
+    public function refactor(Node $node)
+    {
+        if ($node instanceof If_) {
+            return null;
+        }
+
+        return $node;
+    }
+}

tests/Rules/Rector/NoOnlyNullReturnInRefactorRule/NoOnlyNullReturnInRefactorRuleTest.php

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

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

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