[11.x] feat: narrow types for throw_if and throw_unless

[calebdw]

Hello!

This PR correctly causes the types to be narrowed for the throw_{if_unless} functions.

Thanks!

[calebdw]

Thank you sir!

[timacdonald]

@calebdw, is there any way for this to check for truthy values rather than explicitly true?

$condition is mixed and can accept things like eloquent models in the condition. I would expect this to work with something like the following where the model can be an instance of null.

$model = Model::first();

throw_unless($model);

See: https://phpstan.org/r/ee1060c6-333b-4d06-9362-17b69c691d28

All good if not. The experience hasn't changed for that use-case and has improved for others. Just wondered if it was possible.

[calebdw]

@timacdonald,

I don't think so, not without creating a custom PHPStan extension (which seems overkill). The issue is that PHPStan doesn't really have a "truthy" type (as far as I am aware). I tried reversing the condition and using false/empty but that wasn't the best at narrowing the type.

While your code is valid, all you need to do is slightly adjust the expression for better type narrowing:

$model = Model::first();

throw_if($model === null);
throw_if(is_null($model));
// etc.
assertType('Model', $model);

[crishoj]

The issue is that PHPStan doesn't really have a "truthy" type (as far as I am aware)

@calebdw could explicitly using !0|0.0|''|'0'|array{}|false|null for truthiness check be an approach?

[crishoj]

Source: phpstan/phpstan#11335 (comment)

These annotations reflect how throw_if and throw_unless work with truthy values:

if (! function_exists('throw_if')) {
    /**
     * Throw the given exception if the given condition is true.
     *
     * @template TValue
     * @template TException of \Throwable
     *
     * @param  TValue  $condition
     * @param  TException|class-string<TException>|string  $exception
     * @param  mixed  ...$parameters
     * @return ($condition is true ? never : ($condition is false ? TValue : ($condition is null ? TValue : ($condition is array{} ? TValue : ($condition is "0" ? TValue : ($condition is "" ? TValue : ($condition is 0 ? TValue : ($condition is 0.0 ? TValue : never))))))))
     *
     * @throws TException
     */
    function throw_if($condition, $exception = 'RuntimeException', ...$parameters)
    {
        if ($condition) {
            if (is_string($exception) && class_exists($exception)) {
                $exception = new $exception(...$parameters);
            }

            throw is_string($exception) ? new RuntimeException($exception) : $exception;
        }

        return $condition;
    }
}

if (! function_exists('throw_unless')) {
    /**
     * Throw the given exception unless the given condition is true.
     *
     * @template TValue
     * @template TException of \Throwable
     *
     * @param  TValue  $condition
     * @param  TException|class-string<TException>|string  $exception
     * @param  mixed  ...$parameters
     * @return ($condition is true ? TValue : ($condition is false ? never : ($condition is null ? never : ($condition is array{} ? never : ($condition is "0" ? never : ($condition is "" ? never : ($condition is 0 ? never : ($condition is 0.0 ? never : TValue))))))))
     *
     * @throws TException
     */
    function throw_unless($condition, $exception = 'RuntimeException', ...$parameters)
    {
        throw_if(! $condition, $exception, ...$parameters);

        return $condition;
    }
}

Not particularly pretty through.

[calebdw]

@crishoj, that seems more complicated than it's worth---particularly given that you can likely update the code to have a boolean expression that will narrow the type

[gazben]

@calebdw With the new changes 46dc7a4 here phpstan now throws Unreachable statement - code above always terminates. errors when I pass an eloquent model to the function.

When I modify the throw_unless return to this it still doesn't work:

@return ($condition is non-empty-mixed ? TValue : never)

I think this will affect a lot of people. To add a type conversion just for this is overkill.

EDIT: sorry the non-empty-mixed works for me after I cleared the cache.

[crishoj]

@gazben thanks for drawing attention to the "advanced" phpstan type non-empty-mixed — I wasn't aware of this.

(Added in phpstan/phpstan-src@387ebd5)

Given this, I would suggest changing the return type signatures to the following:

/**
 * Throw the given exception if the given condition is true.
 *
 * @template TValue
 * @template TException of \Throwable
 *
 * @param TValue                                     $condition
 * @param TException|class-string<TException>|string $exception
 * @param mixed                                      ...$parameters
 * @return ($condition is non-empty-mixed ? never : TValue)
 *
 * @throws TException
 */
function throw_if($condition, $exception = 'RuntimeException', ...$parameters)
{
   // ...
}

/**
 * Throw the given exception unless the given condition is true.
 *
 * @template TValue
 * @template TException of \Throwable
 *
 * @param TValue                                     $condition
 * @param TException|class-string<TException>|string $exception
 * @param mixed                                      ...$parameters
 * @return ($condition is non-empty-mixed ? TValue : never)
 *
 * @throws TException
 */
function throw_unless($condition, $exception = 'RuntimeException', ...$parameters)
{
   // ...
}

It works well in my end.

[calebdw]

@crishoj, I see you submitted a PR---you had to change the tests in order to use non-empty-mixed and you lost all type narrowing capabilities.

This now fails:

    throw_unless(is_int($foo));
    assertType('int', $foo);

[crishoj]

@calebdw Apologies, the loss of type-narrowing was unintentional.

Do you think we could adjust the type annotation so that both type narrowing and actual behavior with "falsey" values is achieved?

[calebdw]

You might could use something like the following (where true has to come first):

@return ($condition is true ? never : ($condition is non-empty-mixed ? never : TValue))

However, I still think this is more trouble than it's worth, more strict levels of phpstan require that a boolean expression is passed to an if anyway:

https://phpstan.org/r/857faf77-8e2b-45d0-8e6e-23a64ef179d8