feat: add NarrowTooWideReturnTypeRector

[calebdw]

Hello!

Closes rectorphp/rector#9309

This adds a rule that automatically narrows too wide return types (helps solve the return.unusedType phpstan errors):

final class SomeClass
{
-   public function getData(): string|int|\DateTime
+   public function getData(): string|int
    {
        if (rand(0, 1)) {
            return 'text';
        }
        return 1000;
    }
}

Thanks!

[TomasVotruba]

Thanks for great rule 👌 Will save a lot of manual work.

The rule name seems negative, like PHPStan rule.
Rector fixes something for better, e.g. NarrowTooWideUnionReturnTypeRector

[samsonasik]

Please:

• don't use force push except needed (need rebase due to main branch updated), it is hard to review the updated code. Just add more commits
• don't close conversation except actually resolved after new commit.

[samsonasik]

I think @return docblock should be updated as well if exists:

/**
- * @return \Iterator<int>|string
+ * @return \Iterator<int>
 */
-function foo(): \Iterator|string
+function foo(): \Iterator
{
   return new \ArrayIterator([1]);
}

You can use :

$this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $newType);

[calebdw]

I think @return docblock should be updated as well if exists:

I imagine that I can longer just use getNativeType() then

[samsonasik]

getType() still needed as you extract UnionType, that's can be identifier, eg: string

[calebdw]

I'm not too clear on the difference, but I imagine that getNativeType does not include generic information?

[samsonasik]

getType() still needed as you extract UnionType, that's can be identifier, eg: string, here:

        $types = $returnType instanceof UnionType
            ? $returnType->types
            : [$returnType->type, new Identifier('null')];
        $usedTypes = [];

        foreach ($types as $type) {
            $declaredType = $type instanceof Expr
                ? $this->nodeTypeResolver->getNativeType($type)
                : $this->getType($type);

The $returnType->types can be collected of Name or Identifier, not Expr.

[samsonasik]

Please add fixture to skip docblock base type:

/**
 * @param int $a
 */
function foo($a): int|string
{
    return $a;
}

[samsonasik]

Please add test for this:

    /**
     * @param int $a
     * @return int|string 
     */
    function mixedReturn($a): int|string
    {
        return $a;
    }

[samsonasik]

please add fixture to skip non-typed based then, to skip this:

    /**
     * @param class-string<SomeInterface> $class
     * @return class-string<SomeInterface>|int
     */
    public function bar($class): string|int
    {
        return $class;
    }

Above is marked as MixedType on its type, see

https://phpstan.org/r/3e952cb5-c876-4b6e-bfef-89aa5d5b942b

[calebdw]

I see no reason to do that, this signature is enforced by PHPStan and trying to loop over all the parameter types is only going to unnecessarily complicate the code

[samsonasik]

That's needed, upgrading to native type is only when we sure about this, when unsure, just skip :)

We had docblock based changing rules in the past, but mostly cause issues in projects, that's why using native type as posible should be used

[calebdw]

done

[samsonasik]

Expr returned is not always based on param, see my latest comments :)

[samsonasik]

Please add fixture to skip this, which actually skip MixedType:

    /**
     * @return class-string<\stdClass>|int
     */
    public function skipMixedByDoc(): string|int
    {
        /**
         * @var class-string<\stdClass> $class
         */
        $class = get();
        return $class;
    }

see https://phpstan.org/r/47037a56-f5da-4e99-bb3d-7adc3b609612 that it still mixed.

[samsonasik]

@calebdw sorry, I am going to close this, as you don't follow my guideline and suggestion. I am here for the barricade to avoid the new rule cause issues in our project.

Take your time to understand our philosophy to take upgrade as safe as possible.

Thank you for understanding

[github-actions]

This pull request has been automatically locked because it has been closed for 150 days. Please open a new PR if you want to continue the work.