[phpstan] Add PHPStan rule to flag same class-type union in param and return

.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\\Set\\Contract\\SetListInterface" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\DuplicateRegistrationFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\SingleRegistrationFactory" --skip-type="Rector\\ChangesReporting\\Contract\\Output\\OutputFormatterInterface" --skip-type="Rector\\NodeTypeResolver\\Contract\\NodeTypeResolverInterface" --skip-type="Rector\\CodingStyle\\Contract\\ClassNameImport\\ClassNameImportSkipVoterInterface" --skip-type="Rector\\Php80\\Contract\\ConverterAttributeDecoratorInterface" --skip-type="Rector\\NodeNameResolver\\Contract\\NodeNameResolverInterface" --skip-type="Rector\\StaticTypeMapper\\Contract\\PhpDocParser\\PhpDocTypeMapperInterface" --skip-type="Rector\\PhpAttribute\\Contract\\AnnotationToAttributeMapperInterface" --skip-type="Rector\\PHPStanStaticTypeMapper\\Contract\\TypeMapperInterface" --skip-type="Rector\\StaticTypeMapper\\Contract\\PhpParser\\PhpParserNodeMapperInterface"
+                            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\\Set\\Contract\\SetListInterface" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\DuplicateRegistrationFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\SingleServiceRegistrationRule\\Source\\SingleRegistrationFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\PreferGenericTemplateOnSameUnionTypeRule\\Source\\SameUnionParamAndReturn" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\PreferGenericTemplateOnSameUnionTypeRule\\Source\\SkipGenericTemplate" --skip-type="Rector\\ChangesReporting\\Contract\\Output\\OutputFormatterInterface" --skip-type="Rector\\NodeTypeResolver\\Contract\\NodeTypeResolverInterface" --skip-type="Rector\\CodingStyle\\Contract\\ClassNameImport\\ClassNameImportSkipVoterInterface" --skip-type="Rector\\Php80\\Contract\\ConverterAttributeDecoratorInterface" --skip-type="Rector\\NodeNameResolver\\Contract\\NodeNameResolverInterface" --skip-type="Rector\\StaticTypeMapper\\Contract\\PhpDocParser\\PhpDocTypeMapperInterface" --skip-type="Rector\\PhpAttribute\\Contract\\AnnotationToAttributeMapperInterface" --skip-type="Rector\\PHPStanStaticTypeMapper\\Contract\\TypeMapperInterface" --skip-type="Rector\\StaticTypeMapper\\Contract\\PhpParser\\PhpParserNodeMapperInterface"
 
                     -
                         name: 'Compatible PHPStan versions'

phpstan.neon

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

rules/CodingStyle/Rector/ClassMethod/FuncGetArgsToVariadicParamRector.php

@@ -109,6 +109,11 @@ public function provideMinPhpVersion(): int
         return PhpVersionFeature::VARIADIC_PARAM;
     }
 
+    /**
+     * @template TFunctionLike of ClassMethod|Function_|Closure
+     * @param TFunctionLike $node
+     * @return TFunctionLike|null
+     */
     private function applyVariadicParams(
         ClassMethod|Function_|Closure $node,
         string $variableName

rules/Php81/Rector/MethodCall/MyCLabsMethodCallToEnumConstRector.php

@@ -244,6 +244,11 @@ private function getValidEnumExpr(Node $node): null|ClassConstFetch|Expr
         };
     }
 
+    /**
+     * @template TExpr of PropertyFetch|Variable
+     * @param TExpr $expr
+     * @return TExpr|null
+     */
     private function getPropertyFetchOrVariable(PropertyFetch|Variable $expr): null|PropertyFetch|Variable
     {
         if (! $this->isObjectType($expr, new ObjectType('MyCLabs\Enum\Enum'))) {

rules/Privatization/NodeManipulator/VisibilityManipulator.php

@@ -159,6 +159,11 @@ public function removeReadonly(Class_|Property|Param $node): void
         }
     }
 
+    /**
+     * @template TNode of ClassConst|ClassMethod|Property
+     * @param TNode $node
+     * @return TNode|null
+     */
     public function publicize(ClassConst|ClassMethod|Property $node): ClassConst|ClassMethod|Property|null
     {
         // already non-public

rules/Renaming/Rector/MethodCall/RenameMethodRector.php

@@ -157,6 +157,11 @@ private function shouldKeepForParentInterface(
         );
     }
 
+    /**
+     * @template TClassLike of Class_|Trait_|Interface_
+     * @param TClassLike $classOrInterface
+     * @return TClassLike|null
+     */
     private function refactorClass(
         Class_|Trait_|Interface_ $classOrInterface,
         Scope $scope

rules/Renaming/Rector/PropertyFetch/RenamePropertyRector.php

@@ -128,6 +128,11 @@ private function isUsingTrait(ClassLike $classLike, string $traitName): bool
         return $classReflection->hasTraitUse($traitName);
     }
 
+    /**
+     * @template TPropertyFetch of PropertyFetch|StaticPropertyFetch
+     * @param TPropertyFetch $propertyFetch
+     * @return TPropertyFetch|null
+     */
     private function refactorPropertyFetch(
         PropertyFetch|StaticPropertyFetch $propertyFetch
     ): null|PropertyFetch|StaticPropertyFetch {

rules/TypeDeclaration/NodeAnalyzer/StrictReturnNewArrayResolver.php

@@ -49,6 +49,11 @@ public function __construct(
     /**
      * @return ClassMethod|Function_|null the passed node when an array return type was added, null otherwise
      */
+    /**
+     * @template TFunctionLike of ClassMethod|Function_
+     * @param TFunctionLike $node
+     * @return TFunctionLike|null
+     */
     public function resolve(ClassMethod|Function_ $node): ClassMethod|Function_|null
     {
         // 1. is variable instantiated with array
@@ -90,6 +95,11 @@ public function resolve(ClassMethod|Function_ $node): ClassMethod|Function_|null
         return $this->processAddArrayReturnType($node, $returnType);
     }
 
+    /**
+     * @template TFunctionLike of ClassMethod|Function_
+     * @param TFunctionLike $node
+     * @return TFunctionLike|null
+     */
     private function processAddArrayReturnType(
         ClassMethod|Function_ $node,
         Type $returnType

rules/TypeDeclaration/NodeManipulator/AddNeverReturnType.php

@@ -33,6 +33,11 @@ public function __construct(
     ) {
     }
 
+    /**
+     * @template TFunctionLike of ClassMethod|Function_|Closure
+     * @param TFunctionLike $node
+     * @return TFunctionLike|null
+     */
     public function add(ClassMethod|Function_|Closure $node, Scope $scope): ClassMethod|Function_|Closure|null
     {
         if ($this->shouldSkip($node, $scope)) {

rules/TypeDeclaration/NodeManipulator/AddReturnTypeFromCast.php

@@ -27,6 +27,11 @@ public function __construct(
     ) {
     }
 
+    /**
+     * @template TFunctionLike of ClassMethod|Function_
+     * @param TFunctionLike $functionLike
+     * @return TFunctionLike|null
+     */
     public function add(ClassMethod|Function_ $functionLike, Scope $scope): ClassMethod|Function_|null
     {
         if ($functionLike->returnType instanceof Node) {

rules/TypeDeclaration/NodeManipulator/AddReturnTypeFromParam.php

@@ -36,6 +36,11 @@ public function __construct(
     ) {
     }
 
+    /**
+     * @template TFunctionLike of ClassMethod|Function_
+     * @param TFunctionLike $functionLike
+     * @return TFunctionLike|null
+     */
     public function add(ClassMethod|Function_ $functionLike, Scope $scope): ClassMethod|Function_|null
     {
         if ($functionLike->stmts === null) {

rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictTypedCallRector.php

@@ -170,6 +170,11 @@ private function isUnionPossibleReturnsVoid(ClassMethod|Function_|Closure $node)
         return false;
     }
 
+    /**
+     * @template TFunctionLike of ClassMethod|Function_|Closure
+     * @param TFunctionLike $node
+     * @return TFunctionLike
+     */
     private function processSingleUnionType(
         ClassMethod|Function_|Closure $node,
         UnionType $unionType,
@@ -200,6 +205,11 @@ private function shouldSkip(ClassMethod|Function_|Closure $node, Scope $scope):
         return $this->isUnionPossibleReturnsVoid($node);
     }
 
+    /**
+     * @template TFunctionLike of ClassMethod|Function_|Closure
+     * @param TFunctionLike $functionLike
+     * @return TFunctionLike
+     */
     private function refactorSingleReturnType(
         Return_ $return,
         Identifier|Name|NullableType|ComplexType $returnedStrictTypeNode,

rules/TypeDeclaration/Rector/While_/WhileNullableToInstanceofRector.php

@@ -99,6 +99,11 @@ private function createInstanceof(Expr $expr, ObjectType $objectType): Instanceo
         return new Instanceof_($expr, $fullyQualified);
     }
 
+    /**
+     * @template TLoop of While_|Do_
+     * @param TLoop $while
+     * @return TLoop|null
+     */
     private function refactorNotIdentical(While_|Do_ $while, NotIdentical $notIdentical): While_|Do_|null
     {
         if (! $this->valueResolver->isNull($notIdentical->right)) {

utils/phpstan/src/Rule/PreferGenericTemplateOnSameUnionTypeRule.php

@@ -0,0 +1,121 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Rule;
+
+use PhpParser\Comment\Doc;
+use PhpParser\Node;
+use PhpParser\Node\ComplexType;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\UnionType;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+/**
+ * Flags a method whose param and return share the same class-type union, e.g. (A|B $node): A|B|null.
+ * Such a method usually returns the passed node, so a @template generic narrows the return to the passed type.
+ *
+ * @implements Rule<ClassMethod>
+ * @see \Rector\Utils\PHPStan\Tests\Rule\PreferGenericTemplateOnSameUnionTypeRule\PreferGenericTemplateOnSameUnionTypeRuleTest
+ */
+final class PreferGenericTemplateOnSameUnionTypeRule implements Rule
+{
+    private const string ERROR_MESSAGE = 'Param and return share the same class-type union "%s". Add a @template generic to narrow the return type to the passed one.';
+
+    public function getNodeType(): string
+    {
+        return ClassMethod::class;
+    }
+
+    /**
+     * @param ClassMethod $node
+     * @return list<IdentifierRuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $docComment = $node->getDocComment();
+        if ($docComment instanceof Doc && str_contains($docComment->getText(), '@template')) {
+            return [];
+        }
+
+        $returnNames = $this->resolveClassUnionNames($node->returnType);
+        if ($returnNames === null) {
+            return [];
+        }
+
+        foreach ($node->params as $param) {
+            $paramNames = $this->resolveClassUnionNames($param->type);
+            if ($paramNames === null) {
+                continue;
+            }
+
+            if ($paramNames !== $returnNames) {
+                continue;
+            }
+
+            $shortNames = array_map(
+                static function (string $name): string {
+                    $parts = explode('\\', $name);
+                    return end($parts);
+                },
+                $returnNames
+            );
+
+            return [
+                RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, implode('|', $shortNames)))
+                    ->identifier('rector.genericTemplateOverSameUnionType')
+                    ->line($node->getStartLine())
+                    ->build(),
+            ];
+        }
+
+        return [];
+    }
+
+    /**
+     * 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;
+                }
+
+                // built-in scalar/array type, not a class
+                return null;
+            }
+
+            if ($memberType instanceof Name) {
+                $names[] = $memberType->toString();
+                continue;
+            }
+
+            // IntersectionType or anything else
+            return null;
+        }
+
+        if (count($names) < 2) {
+            return null;
+        }
+
+        $names = array_unique($names);
+        sort($names);
+
+        return $names;
+    }
+}

utils/phpstan/tests/Rule/PreferGenericTemplateOnSameUnionTypeRule/PreferGenericTemplateOnSameUnionTypeRuleTest.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\PreferGenericTemplateOnSameUnionTypeRule;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Rector\Utils\PHPStan\Rule\PreferGenericTemplateOnSameUnionTypeRule;
+
+/**
+ * @extends RuleTestCase<PreferGenericTemplateOnSameUnionTypeRule>
+ */
+final class PreferGenericTemplateOnSameUnionTypeRuleTest extends RuleTestCase
+{
+    public function testSameUnionParamAndReturn(): void
+    {
+        $expectedErrorMessage = 'Param and return share the same class-type union "ClassMethod|Function_". Add a @template generic to narrow the return type to the passed one.';
+
+        $this->analyse([__DIR__ . '/Source/SameUnionParamAndReturn.php'], [[$expectedErrorMessage, 12]]);
+    }
+
+    public function testSkip(): void
+    {
+        $this->analyse([__DIR__ . '/Source/SkipGenericTemplate.php'], []);
+    }
+
+    protected function getRule(): Rule
+    {
+        return new PreferGenericTemplateOnSameUnionTypeRule();
+    }
+}

utils/phpstan/tests/Rule/PreferGenericTemplateOnSameUnionTypeRule/Source/SameUnionParamAndReturn.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\PreferGenericTemplateOnSameUnionTypeRule\Source;
+
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Function_;
+
+final class SameUnionParamAndReturn
+{
+    public function process(ClassMethod|Function_ $node): ClassMethod|Function_|null
+    {
+        return $node;
+    }
+}

utils/phpstan/tests/Rule/PreferGenericTemplateOnSameUnionTypeRule/Source/SkipGenericTemplate.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\PreferGenericTemplateOnSameUnionTypeRule\Source;
+
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Function_;
+use PhpParser\Node\Stmt\Interface_;
+
+final class SkipGenericTemplate
+{
+    /**
+     * @template TNode of ClassMethod|Function_
+     * @param TNode $node
+     * @return TNode|null
+     */
+    public function alreadyGeneric(ClassMethod|Function_ $node): ClassMethod|Function_|null
+    {
+        return $node;
+    }
+
+    public function scalarUnion(int|float $value): int|float
+    {
+        return $value + 1;
+    }
+
+    public function mismatchedUnion(ClassMethod|Function_ $node): ClassMethod|Interface_|null
+    {
+        return null;
+    }
+}