Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPStan incorrectly widens conditional return types? #10653

Open
lkrms opened this issue Feb 29, 2024 · 1 comment
Open

PHPStan incorrectly widens conditional return types? #10653

lkrms opened this issue Feb 29, 2024 · 1 comment
Labels
Milestone

Comments

@lkrms
Copy link

lkrms commented Feb 29, 2024

Bug report

This one has me stumped after trying many different variants of the PHPDoc tags on throwOnFailure(). As best I can tell, PHPStan is widening the incoming type from <something>|false to <something>|bool, which it then narrows to <something>|true per the conditional return type.

<?php declare(strict_types=1);

class A
{
    private int $Counter = 0;

    /**
     * @return stdClass
     */
    public function getMayFail()
    {
		// PHPStan reports "Method A::getMayFail() should return stdClass but returns stdClass|true."
        return $this->throwOnFailure($this->mayFail());
    }

    /**
     * @template T
     *
     * @param T $result
     * @return (T is false ? never : T)
     */
    public function throwOnFailure($result)
    {
        if ($result === false) {
            throw new Exception('Operation failed');
        }
        return $result;
    }

    /**
     * @return stdClass|false
     */
    public function mayFail()
    {
        $this->Counter++;
        return $this->Counter % 2 ? new stdClass() : false;
    }
}

If I change the DocBlock to this, it works:

    /**
     * @template T
     *
     * @param T|false $result
     * @return ($result is false ? never : T)
     */

...but other tools (Intelephense, at least) don't like it and it isn't amenable to adding a second template for the failure value (e.g. so the caller can specify whether false or -1 are regarded as failures); and the original syntax should work 😉

Code snippet that reproduces the problem

https://phpstan.org/r/06424f05-e1f9-4d72-903b-0f2cc1cc183b

Expected output

No errors; types should narrow to exclude false without adding true.

Did PHPStan help you today? Did it make you happy in any way?

In this moment it isn't making me happy 🤣 But I'm finally leveling up to 9 across my projects and am appreciating the quality improvements ❤️

@lkrms
Copy link
Author

lkrms commented Feb 29, 2024

Just flagging that after triggering Type false is not always the same as T. It breaks the contract for some argument types, typically subtypes. and chasing that down, it seems possible/likely this might be related to phpstan/phpstan-src#2652, as mentioned in #9961...

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants