[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