feat: add NarrowTooWideReturnTypeRector

[calebdw]

Hello!

Supercedes #7147
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!

[calebdw]

This skips all native mixed returns as can't be sure

[samsonasik]

Please run on existing rector-src codebase:

bin/rector

this change seems invalid and should be skipped:

9) src/Console/Command/SetupCICommand.php:50

    ---------- begin diff ----------
@@ @@
         return self::SUCCESS;
     }

-    /**
-     * @return CiDetector::CI_*|null
-     */
-    private function resolveCurrentCI(): ?string
+    private function resolveCurrentCI(): null
     {
         if (file_exists(getcwd() . '/.github')) {
             return CiDetector::CI_GITHUB_ACTIONS;
    ----------- end diff -----------

Applied rules:
 * RemoveUselessReturnTagRector
 * NarrowTooWideReturnTypeRector

[calebdw]

@samsonasik, there's only 1 change now:

[samsonasik]

The issue on @return CiDetector::CI_*|null seems because of CiDetector::CI_* which type incorrectly detection target as autocompleting type.

so probably better loop the docblock type, if found something that can't be converted to native type, just skip it, eg these pattern:

CiDetector::CI_*
@template TType of object|null
@return TType

[calebdw]

is CiDetector::CI_* even a valid phpdoc?

The docs would seem to want to use key-of<Type::ARRAY_CONST> or value-of<Type::ARRAY_CONST>

https://phpstan.org/writing-php-code/phpdoc-types#key-and-value-types-of-arrays-and-iterables

[samsonasik]

Yes, that's valid docblock, see https://phpstan.org/r/77bb2280-9ef5-48a3-846a-02009e413399

[samsonasik]

You can fixture to skip this:

use OndraM\CiDetector\CiDetector;

/**
 * @return int|CiDetector::CI_*|null
 */
function skipSomeComplexDocSubType(): int|string|null
{
    if (file_exists(getcwd() . '/.github')) {
        return CiDetector::CI_GITHUB_ACTIONS;
    }

    if (file_exists(getcwd() . '/.gitlab-ci.yml')) {
        return CiDetector::CI_GITLAB;
    }

    if (rand(0, 1)) {
        return 1;
    }

    return null;
}

[calebdw]

The issue on @return CiDetector::CI_|null seems because of CiDetector::CI_ which type incorrectly detection target as autocompleting type.

I'm digging into this, and it's a bug in the TypeFactory::uniquateTypes() method---when the code falls back to the phpstan resolution it works perfectly

[samsonasik]

Please add fixture for this example to be skipped:

use OndraM\CiDetector\CiDetector;

/**
 * @return int|CiDetector::CI_*|null
 */
function skipSomeComplexDocSubType(): int|string|null
{
    if (file_exists(getcwd() . '/.github')) {
        return CiDetector::CI_GITHUB_ACTIONS;
    }

    if (file_exists(getcwd() . '/.gitlab-ci.yml')) {
        return CiDetector::CI_GITLAB;
    }

    if (rand(0, 1)) {
        return 1;
    }

    return null;
}

[calebdw]

Yes, I'm working on that. However, commenting this out causes the code to pass:

[samsonasik]

That seems not, as its docblock value become extracted, and not what we want:

1) src/Console/Command/SetupCICommand.php:51

    ---------- begin diff ----------
@@ @@
     }

     /**
-     * @return int|CiDetector::CI_*|null
+     * @return 'GitHub Actions'|'GitLab'|int|null
      */
     private function resolveCurrentCI(): int|string|null
     {
@@ @@
             return CiDetector::CI_GITLAB;
         }

-        if (rand(0, 1)) {
+        if (random_int(0, 1) !== 0) {
             return 1;
         }
    ----------- end diff -----------

Applied rules:
 * ExplicitBoolCompareRector
 * NarrowTooWideReturnTypeRector
 * RandomFunctionRector

[calebdw]

Sorry, I should have clarified, I have another test fixture with a union of exact values and it's removing all but the last one

[calebdw]

I'll work on this more tomorrow, I'm going to restructure the rector so that the native and phpdoc return types are handled separately

[calebdw]

@samsonasik, I've refactored the code to handle the native return types and the phpdoc return types separately and this helps with flexibility.

However, the test is failing because calling $phpDocInfo->getReturnType() removes extra constant string types:

Commenting out this line causes the test to pass because the extra constant types are no longer being removed:

[samsonasik]

I will check your branch and take care the rest, I prefer to just skip these for now:

• final result is true, false, null
• if there is docblock that can't be convertible to native type: constfetch magic * , template type, etc
• both parent and child exists in union

which improvement can be in separate PRs

[samsonasik]

@calebdw this should be good to merge now 👍 , I skipped various complex sub type and mostly placeholder: null, false, true usage.

Improvement if needed, can be in separate PR.

thank you

@TomasVotruba let's merge it and test in the wild ;)

[TomasVotruba]

Can't wait to test this on clients' projects 👌😎

Thank you @calebdw

[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.