[PHPStan] Add PreferGenericTemplateOnReturnedUnionParamRule to spot hidden generics

.github/workflows/code_analysis.yaml

@@ -53,7 +53,7 @@ jobs:
                     -
                         name: 'Active Classes'
                         run: |
-                            vendor/bin/class-leak check bin config src rules utils --skip-suffix "Rector" --skip-type="Rector\\Utils\\Compiler\\Unprefixer" --skip-type="Rector\\NodeCollector\\BinaryOpConditionsCollector" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\PreferGenericTemplateOnSameUnionTypeRule\\Source\\SameUnionParamAndReturn" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\PreferGenericTemplateOnSameUnionTypeRule\\Source\\SkipGenericTemplate"
+                            vendor/bin/class-leak check bin config src rules utils --skip-suffix "Rector" --skip-type="Rector\\Utils\\Compiler\\Unprefixer" --skip-type="Rector\\NodeCollector\\BinaryOpConditionsCollector" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-path="Source"
 
                     -
                         name: 'Compatible PHPStan versions'

phpstan.neon

@@ -5,6 +5,7 @@ rules:
     - Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule
     - Rector\Utils\PHPStan\Rule\NoScopeFetchBeforeCheapGuardRule
     - Rector\Utils\PHPStan\Rule\PreferGenericTemplateOnSameUnionTypeRule
+    - Rector\Utils\PHPStan\Rule\NarrowReturnToUnionParamRule
 
 parameters:
     level: 8

rules/Arguments/ArgumentDefaultValueReplacer.php

@@ -105,7 +105,7 @@ private function processParams(
     private function processArgs(
         MethodCall|StaticCall|FuncCall|New_ $expr,
         ReplaceArgumentDefaultValueInterface $replaceArgumentDefaultValue
-    ): ?Expr {
+    ): MethodCall|StaticCall|FuncCall|New_|null {
         if ($expr->isFirstClassCallable()) {
             return null;
         }

rules/DeadCode/Rector/ClassMethod/RemoveNullTagValueNodeRector.php

@@ -159,7 +159,12 @@ static function (AstNode $astNode) use ($paramNames): ?int {
         );
     }
 
-    private function processVarTagNull(Expression|Property $node): ?Node
+    /**
+     * @template TNode of Expression|Property
+     * @param TNode $node
+     * @return TNode|null
+     */
+    private function processVarTagNull(Expression|Property $node): Expression|Property|null
     {
         $phpdocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
         $varTagValueNode = $phpdocInfo->getVarTagValueNode();

rules/Php54/Rector/Break_/RemoveZeroBreakContinueRector.php

@@ -110,7 +110,12 @@ public function refactor(Node $node): ?Node
         return null;
     }
 
-    private function processVariableNum(Break_|Continue_ $stmt, Variable $numVariable): ?Node
+    /**
+     * @template TNode of Break_|Continue_
+     * @param TNode $stmt
+     * @return TNode|null
+     */
+    private function processVariableNum(Break_|Continue_ $stmt, Variable $numVariable): Break_|Continue_|null
     {
         $staticType = $this->getType($numVariable);
 

utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php

@@ -0,0 +1,199 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Rule;
+
+use PhpParser\Node;
+use PhpParser\Node\ComplexType;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\IntersectionType;
+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 \Rector\Utils\PHPStan\Tests\Rule\NarrowReturnToUnionParamRule\NarrowReturnToUnionParamRuleTest
+ */
+final class NarrowReturnToUnionParamRule implements Rule
+{
+    private const string 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;
+        }
+
+        $foundReturn = new 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(null|Identifier|Name|ComplexType $type): bool
+    {
+        if ($type instanceof NullableType) {
+            return true;
+        }
+
+        if (! $type instanceof UnionType) {
+            return false;
+        }
+
+        return array_any($type->types, fn (Identifier|Name|IntersectionType $memberType): bool => $memberType instanceof Identifier && $memberType->toLowerString() === 'null');
+    }
+
+    /**
+     * 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(null|Identifier|Name|ComplexType $type): ?string
+    {
+        if ($type instanceof NullableType) {
+            $type = $type->type;
+        }
+
+        if ($type instanceof Name) {
+            return $type->toString();
+        }
+
+        if (! $type instanceof UnionType) {
+            return null;
+        }
+
+        $names = [];
+        foreach ($type->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(null|Identifier|Name|ComplexType $type): ?array
+    {
+        if (! $type instanceof UnionType) {
+            return null;
+        }
+
+        $names = [];
+        foreach ($type->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;
+    }
+}

utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NarrowReturnToUnionParamRule;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Rector\Utils\PHPStan\Rule\NarrowReturnToUnionParamRule;
+
+/**
+ * @extends RuleTestCase<NarrowReturnToUnionParamRule>
+ */
+final class NarrowReturnToUnionParamRuleTest extends RuleTestCase
+{
+    public function testBroadReturnOfUnionParam(): void
+    {
+        $expectedErrorMessage = 'Return type "PhpParser\Node" is too broad for a returned union param. Declare the explicit union return type "MethodCall|New_|null" instead.';
+
+        $this->analyse([__DIR__ . '/Source/BroadReturnOfUnionParam.php'], [[$expectedErrorMessage, 13]]);
+    }
+
+    public function testSkip(): void
+    {
+        $this->analyse([__DIR__ . '/Source/SkipBroadReturn.php'], []);
+    }
+
+    protected function getRule(): Rule
+    {
+        return new NarrowReturnToUnionParamRule();
+    }
+}

utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/BroadReturnOfUnionParam.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NarrowReturnToUnionParamRule\Source;
+
+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;
+    }
+}

utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/SkipBroadReturn.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NarrowReturnToUnionParamRule\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\ArrayDimFetch;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\New_;
+use PhpParser\Node\Expr\Variable;
+
+final class SkipBroadReturn
+{
+    public function doesNotReturnParam(New_|MethodCall $node): ?Node
+    {
+        if ($node instanceof New_) {
+            return null;
+        }
+
+        return null;
+    }
+
+    public function scalarUnion(int|float $value): int|float
+    {
+        return $value;
+    }
+
+    public function singleClassParam(New_ $node): ?Node
+    {
+        return $node;
+    }
+
+    public function alreadyExplicitUnionReturn(New_|MethodCall $node): New_|MethodCall|null
+    {
+        return $node;
+    }
+
+    public function returnTypeIsUnionMember(ArrayDimFetch|Variable $expr): ?Variable
+    {
+        while ($expr instanceof ArrayDimFetch) {
+            $expr = $expr->var;
+        }
+
+        if (! $expr instanceof Variable) {
+            return null;
+        }
+
+        return $expr;
+    }
+}