Everything

php

[RFC][DISCUSSION] - Throwables error code's type generalization

php.internals

Net Mo

9 years ago

Good morning nice internals PHPeople,

I have just written a new RFC. I believe is mostly about cosmetic changes,
as what is being proposed is probably already informally supported by PHP.

I would like your opinion about it:
https://wiki.php.net/rfc/throwable-code-generalization

Thank you.

Marco Pivetta

9 years ago

Looks like a step backwards to me. In fact, I just had issues with code (in
zendframework https://github.com/zendframework/zend-servicemanager/pull/167)
that assumed integer error codes, but then failed because PDO failed to
adhere to the property specification (PDO needs fixing here, not the other
way around!).

If you allow anything in there, we'll just have more issues in consumer
libraries that have this assumption and consume exceptions (all of them, if
they are done correctly), since you just broke invariance.

If you really need to add contextual data to an exception, please add a new
property instead.



On 18 Dec 2016 10:38 a.m., "Wes" <netmo.php@gmail.com> wrote:

Net Mo

9 years ago

Yo, Marco :P I don't need to add any contextual data, I'm just trying
improve consistency by formalizing what PHP already does. I think it's the
right way to fix this because I don't think changing PDO is ever going to
happen. You either special case PDO in your code or formalize the special
case so that it becomes the norm. Again, there is no actual formal type
restriction right now, existing code that assumes `int` is wrong, even if
it's not their fault. If you are making that assumption your code is broken
already.

Marco Pivetta

9 years ago

Fix one library or fix all of them: good luck with the second one. 😉

Anyway, the type change (on the property, in this case) can be applied to
any interface change reducing return type strictness: it is a BC break.

On 18 Dec 2016 11:47 a.m., "Wes" <netmo.php@gmail.com> wrote:

Net Mo

9 years ago

Will be nice to have regardless if PHP gets stuff like enums. It's not just
a dirty fix.

Strictly speaking it is not a BC change because `int` is not actually
enforced at all. Also this would be a problem in other languages...

function bar(): int{
mixed $code = (new MyException)->getCode();
return $code;
}

...but in PHP, as long MyException->getCode() continues to return `int`,
there would be no actual BC break.

But if you think it'll be necessary I'll require 2/3 of the votes.

\o

Marco Pivetta

9 years ago

Hey Wes,

On Sun, Dec 18, 2016 at 12:03 PM, Wes <netmo.php@gmail.com> wrote:

> Will be nice to have regardless if PHP gets stuff like enums. It's not
> just a dirty fix.
>
> Strictly speaking it is not a BC change because `int` is not actually
> enforced at all. Also this would be a problem in other languages...
>

According to this reasoning, all documentation should be disregarded and
considered void, because type documentation is unenforced. That would also
mean that any pre-PHP7 code that defined return types via docblock are
basically unusable.

The fact that PHP's type system doesn't enforce it doesn't mean that we
should blindly allow everything everywhere, as that removes any sort of
guarantees on contracts, making it impossible to reason about what the code
will do.

But if you think it'll be necessary I'll require 2/3 of the votes.

>

It's a core change anyway, no?

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

Niklas Keller

9 years ago

2016-12-18 10:38 GMT+01:00 Wes <netmo.php@gmail.com>:

> Good morning nice internals PHPeople,
>
> I have just written a new RFC. I believe is mostly about cosmetic changes,
> as what is being proposed is probably already informally supported by PHP.
>
> I would like your opinion about it:
> https://wiki.php.net/rfc/throwable-code-generalization
>
> Thank you.
>

+1 from my side. Currently exception codes hardly see any use. Nobody wants
to invent some codes that have to be documented and are hard to remember.
Instead, most people just extend their exceptions to subclasses. But what
if the code comes from external software? It's fine for MySQL, because
MySQL provides numeric error codes. But a lot of software is now talking to
remove APIs. To my experience, most of them use non-numeric error codes,
simply because they're human-readable.

I don't think we should allow objects there, objects are hardly codes. But
allowing string|int would be great for usage with many APIs and make $code
actually useful again.