On Thu, Apr 1, 2021 at 3:56 PM Andreas Leathley <a.leath...@gmx.net> wrote:
> On 01.04.21 10:56, Benjamin Eberlei wrote:
> > This RFC is using the declaration of the return type, to specify that it
> > never returns. As such noreturn or never is a keyword a long the lines of
> > public or final, but it is not a return type.
> >
> > I don't think the argument for potential optimizations in Opcache to
> > eliminate dead code or allow IDEs to hint at dead code are valuable
> enough
> > to justify this change.
>
> I already use @return no-return (supported by Psalm/PHPStan) in my code
> now, to clarify the code flow, and for me it fits perfectly for return
> types, as not returning is also a result of the function/method. Having
> a return type of "int" or "string" (or even "void") seems misleading
> when the method will never return anyway.
>
> noreturn/never might not be useful in all code, but for some parts like
> in a controller of a web application it makes handling redirects,
> authorization or 404 errors much easier, clearer and less error-prone,
> for example:
>
> ```php
> if (!isset($user)) {
> $this->notFound(); // this is a noreturn/never method
> }
>
> if (!$this->isGranted('adminPrivileges')) {
> $this->notAuthorized(); // this is a noreturn/never method
> }
> ```
>
> Adding noreturn/never on a language level would make sure calling
> "$this->notAuthorized();" never continues code flow. This is often also
> a security issue, as seen by an alternative way of handling the above
> use cases:
>
> ```php
> if (!isset($user)) {
> $this->notFound();
> return;
> }
>
> if (!$this->isGranted('adminPrivileges')) {
> $this->notAuthorized();
> return;
> }
> ```
>
> If you forget a return, suddenly you have issues. So even though
> noreturn/never is a niche language feature not everyone will use, it
> does address a very real use case.
I don't know, you are arguing that you forgot a return, but you also did
not forget to add a never return type. You
could easily fix this by inlining the exception instead.
```php
if (!isset($user)) {
throw new NotFoundException();
}
```
Even when you insist on the function, it is a one liner that throws
immediately. Adding the never return type there isn't going to catch a
great many bugs.
> Using attributes has the disadvantage
> of not making the intent clear:
>
> ```php
> #[\NoReturn]
> function notAuthorized(): string
> {
> // Some code
> }
> ```
>
> Now you have a return type of string, but also the information that the
> function will never return anyway. That is confusing, but will be
> inevitable in some cases when implementing interfaces or an abstract
> class. In my opinion, the string return type is wrong in this case and
> can only mislead a developer.
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
