[Rector] Add NoScopeFetchBeforeCheapGuardRule and NarrowReturnToUnionParamRule to rector set

config/rector-rules.neon

@@ -11,6 +11,8 @@ rules:
     - Symplify\PHPStanRules\Rules\Rector\AvoidFeatureSetAttributeInRectorRule
     - Symplify\PHPStanRules\Rules\Rector\RectorCheaperGuardsFirstRule
     - Symplify\PHPStanRules\Rules\Rector\PreferGenericTemplateOnSameUnionTypeRule
+    - Symplify\PHPStanRules\Rules\Rector\NoScopeFetchBeforeCheapGuardRule
+    - Symplify\PHPStanRules\Rules\Rector\NarrowReturnToUnionParamRule
     - Symplify\PHPStanRules\Rules\ForbiddenTraitRule
     - Symplify\PHPStanRules\Rules\ForbiddenSwitchRule
 

src/Rules/Rector/NarrowReturnToUnionParamRule.php

@@ -0,0 +1,204 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Rector;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PhpParser\Node\NullableType;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Return_;
+use PhpParser\Node\UnionType;
+use PhpParser\NodeFinder;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+/**
+ * Flags a method that takes a class-type union param (A|B) but declares a broader single return type
+ * (e.g. their shared parent Node), then returns that very param.
+ * The return type should be the explicit union the param carries, not the broad parent.
+ *
+ * @implements Rule<ClassMethod>
+ * @see \Symplify\PHPStanRules\Tests\Rules\Rector\NarrowReturnToUnionParamRule\NarrowReturnToUnionParamRuleTest
+ */
+final class NarrowReturnToUnionParamRule implements Rule
+{
+    public const ERROR_MESSAGE = 'Return type "%s" is too broad for a returned union param. Declare the explicit union return type "%s" instead.';
+
+    public function getNodeType(): string
+    {
+        return ClassMethod::class;
+    }
+
+    /**
+     * @param ClassMethod $node
+     * @return list<IdentifierRuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $returnName = $this->resolveSingleClassReturnName($node->returnType);
+        if ($returnName === null) {
+            return [];
+        }
+
+        foreach ($node->params as $param) {
+            $paramNames = $this->resolveClassUnionNames($param->type);
+            if ($paramNames === null) {
+                continue;
+            }
+
+            // return type is one of the union members, not a broad shared parent - already narrow enough
+            if (in_array($returnName, $paramNames, true)) {
+                continue;
+            }
+
+            if (! $param->var instanceof Variable || ! is_string($param->var->name)) {
+                continue;
+            }
+
+            if (! $this->methodReturnsVariable($node, $param->var->name)) {
+                continue;
+            }
+
+            $shortNames = array_map(static function (string $name): string {
+                $parts = explode('\\', $name);
+                return end($parts);
+            }, $paramNames);
+
+            if ($this->isNullableReturnType($node->returnType)) {
+                $shortNames[] = 'null';
+            }
+
+            return [
+                RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $returnName, implode('|', $shortNames)))
+                    ->identifier('rector.explicitUnionReturnType')
+                    ->line($node->getStartLine())
+                    ->build(),
+            ];
+        }
+
+        return [];
+    }
+
+    private function methodReturnsVariable(ClassMethod $classMethod, string $variableName): bool
+    {
+        if ($classMethod->stmts === null) {
+            return false;
+        }
+
+        $nodeFinder = new NodeFinder();
+        $foundReturn = $nodeFinder->findFirst(
+            $classMethod->stmts,
+            static fn (Node $subNode): bool => $subNode instanceof Return_
+                && $subNode->expr instanceof Variable
+                && $subNode->expr->name === $variableName
+        );
+
+        return $foundReturn instanceof Return_;
+    }
+
+    private function isNullableReturnType(?Node $node): bool
+    {
+        if ($node instanceof NullableType) {
+            return true;
+        }
+
+        if (! $node instanceof UnionType) {
+            return false;
+        }
+
+        foreach ($node->types as $memberType) {
+            if ($memberType instanceof Identifier && $memberType->toLowerString() === 'null') {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns the single class return type name (nullable allowed), or null when the return type is
+     * missing, scalar, or a union of 2+ class types - the latter is already an explicit union.
+     */
+    private function resolveSingleClassReturnName(?Node $node): ?string
+    {
+        if ($node instanceof NullableType) {
+            $node = $node->type;
+        }
+
+        if ($node instanceof Name) {
+            return $node->toString();
+        }
+
+        if (! $node instanceof UnionType) {
+            return null;
+        }
+
+        $names = [];
+        foreach ($node->types as $memberType) {
+            if ($memberType instanceof Identifier) {
+                if ($memberType->toLowerString() === 'null') {
+                    continue;
+                }
+
+                return null;
+            }
+
+            if (! $memberType instanceof Name) {
+                return null;
+            }
+
+            $names[] = $memberType->toString();
+        }
+
+        if (count($names) !== 1) {
+            return null;
+        }
+
+        return $names[0];
+    }
+
+    /**
+     * Returns sorted class-type member names (null stripped), or null when the type is not a
+     * union of 2+ class types - scalars, array and intersections disqualify it.
+     *
+     * @return list<string>|null
+     */
+    private function resolveClassUnionNames(?Node $node): ?array
+    {
+        if (! $node instanceof UnionType) {
+            return null;
+        }
+
+        $names = [];
+        foreach ($node->types as $memberType) {
+            if ($memberType instanceof Identifier) {
+                if ($memberType->toLowerString() === 'null') {
+                    continue;
+                }
+
+                return null;
+            }
+
+            if (! $memberType instanceof Name) {
+                return null;
+            }
+
+            $names[] = $memberType->toString();
+        }
+
+        if (count($names) < 2) {
+            return null;
+        }
+
+        $names = array_unique($names);
+        sort($names);
+
+        return $names;
+    }
+}

src/Rules/Rector/NoScopeFetchBeforeCheapGuardRule.php

@@ -0,0 +1,152 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Rector;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\ConstFetch;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use PhpParser\Node\Stmt\If_;
+use PhpParser\Node\Stmt\Return_;
+use PhpParser\NodeFinder;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Rector\PHPStan\ScopeFetcher;
+
+/**
+ * Flags ScopeFetcher::fetch() called right before a cheap guard that does not use the scope.
+ * The guard should run first, so most nodes bail out before scope resolution.
+ *
+ * @implements Rule<ClassMethod>
+ * @see \Symplify\PHPStanRules\Tests\Rules\Rector\NoScopeFetchBeforeCheapGuardRule\NoScopeFetchBeforeCheapGuardRuleTest
+ */
+final class NoScopeFetchBeforeCheapGuardRule implements Rule
+{
+    public const ERROR_MESSAGE = 'Move the "return null" guard above ScopeFetcher::fetch(), as it does not use the scope - most nodes then bail out before scope resolution.';
+
+    public function getNodeType(): string
+    {
+        return ClassMethod::class;
+    }
+
+    /**
+     * @param ClassMethod $node
+     * @return list<IdentifierRuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if ($node->name->toString() !== 'refactor') {
+            return [];
+        }
+
+        $stmts = $node->stmts;
+        if ($stmts === null) {
+            return [];
+        }
+
+        foreach ($stmts as $key => $stmt) {
+            $scopeVariableName = $this->matchScopeFetchVariableName($stmt);
+            if ($scopeVariableName === null) {
+                continue;
+            }
+
+            $nextStmt = $stmts[$key + 1] ?? null;
+            if (! $nextStmt instanceof If_) {
+                continue;
+            }
+
+            if (! $this->isReturnNullGuard($nextStmt)) {
+                continue;
+            }
+
+            if ($this->usesVariable($nextStmt->cond, $scopeVariableName)) {
+                continue;
+            }
+
+            return [
+                RuleErrorBuilder::message(self::ERROR_MESSAGE)
+                    ->identifier('rector.scopeFetchBeforeCheapGuard')
+                    ->line($stmt->getStartLine())
+                    ->build(),
+            ];
+        }
+
+        return [];
+    }
+
+    private function matchScopeFetchVariableName(Stmt $stmt): ?string
+    {
+        if (! $stmt instanceof Expression) {
+            return null;
+        }
+
+        if (! $stmt->expr instanceof Assign) {
+            return null;
+        }
+
+        $assign = $stmt->expr;
+        if (! $assign->var instanceof Variable || ! is_string($assign->var->name)) {
+            return null;
+        }
+
+        if (! $assign->expr instanceof StaticCall) {
+            return null;
+        }
+
+        $staticCall = $assign->expr;
+        if (! $staticCall->class instanceof Name) {
+            return null;
+        }
+
+        if ($staticCall->class->toString() !== ScopeFetcher::class) {
+            return null;
+        }
+
+        if (! $staticCall->name instanceof Identifier || $staticCall->name->toString() !== 'fetch') {
+            return null;
+        }
+
+        return $assign->var->name;
+    }
+
+    private function isReturnNullGuard(If_ $if): bool
+    {
+        if (count($if->stmts) !== 1) {
+            return false;
+        }
+
+        $onlyStmt = $if->stmts[0];
+        if (! $onlyStmt instanceof Return_) {
+            return false;
+        }
+
+        return ! $onlyStmt->expr instanceof Expr || $this->isNullConstant($onlyStmt->expr);
+    }
+
+    private function isNullConstant(Expr $expr): bool
+    {
+        return $expr instanceof ConstFetch && $expr->name->toLowerString() === 'null';
+    }
+
+    private function usesVariable(Expr $expr, string $variableName): bool
+    {
+        $nodeFinder = new NodeFinder();
+        $foundVariable = $nodeFinder->findFirst(
+            [$expr],
+            static fn (Node $subNode): bool => $subNode instanceof Variable && $subNode->name === $variableName
+        );
+
+        return $foundVariable instanceof Variable;
+    }
+}

tests/Rules/Rector/NarrowReturnToUnionParamRule/Fixture/BroadReturnOfUnionParam.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\NarrowReturnToUnionParamRule\Fixture;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\New_;
+
+final class BroadReturnOfUnionParam
+{
+    public function process(New_|MethodCall $node): ?Node
+    {
+        if ($node instanceof MethodCall) {
+            return null;
+        }
+
+        return $node;
+    }
+}