Hi internals,
A small cleanup RFC for PHP 8: https://wiki.php.net/rfc/lsp_errors
This makes all incompatible method signature (LSP) errors fatal, rather
than only warning in some cases. Especially after
https://wiki.php.net/rfc/parameter-no-type-variance I don't think there's
any reason to keep this backdoor in place.
Regards,
Nikita
Hi internals,
A small cleanup RFC for PHP 8: https://wiki.php.net/rfc/lsp_errors
This makes all incompatible method signature (LSP) errors fatal, rather
than only warning in some cases. Especially after
https://wiki.php.net/rfc/parameter-no-type-variance I don't think there's
any reason to keep this backdoor in place.
Thank you, Nikita.
+1 from me. This inconsistency was a personal WTF moment, 3 years ago:
https://stackoverflow.com/q/37889466/2908724
Hi internals,
A small cleanup RFC for PHP 8: https://wiki.php.net/rfc/lsp_errors
This makes all incompatible method signature (LSP) errors fatal, rather
than only warning in some cases. Especially after
https://wiki.php.net/rfc/parameter-no-type-variance I don't think there's
any reason to keep this backdoor in place.Regards,
Nikita
I see you are trying to solve inconsistency, but i would do it opposite way. LSP errors should be warnings, they are not show stopper errors. It’s very much possible code violating LSP keeps working correctly. And in modern PHP development, everybody considers warnings as show stoppers anyway, so impact here is only in production. When some LSP breakage sneaks unnoticed into production and my INI settings just silence these and log them, I would prefer it to not crash there, because problem with these show only when expecting parent but receiving child, not when child breaks LSP but its nowhere used in place of parent.
And this RFC conveniently shows only big LSP violation examples like array -> int, but not widely used narrowing like mixed/object -> specific instance.
Hi internals,
A small cleanup RFC for PHP 8: https://wiki.php.net/rfc/lsp_errors
This makes all incompatible method signature (LSP) errors fatal, rather
than only warning in some cases. Especially after
https://wiki.php.net/rfc/parameter-no-type-variance I don't think there's
any reason to keep this backdoor in place.Regards,
Nikita
And this RFC conveniently shows only big LSP violation examples like array -> int, but not widely used narrowing like mixed/object -> specific instance.
Type narrowing or contravariant parameters is properly supported for
PHP 7.4: https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters
, which is why this RFC doesn't need to cover them.
cheers
Dan
Ack
I believe rfc deals with reverse situation
On 9 April 2019 4:47:50 PM Dan Ackroyd Danack@basereality.com wrote:
And this RFC conveniently shows only big LSP violation examples like array
-> int, but not widely used narrowing like mixed/object -> specific instance.Type narrowing or contravariant parameters is properly supported for
PHP 7.4:
https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters
, which is why this RFC doesn't need to cover them.cheers
Dan
Ack
I believe rfc deals with reverse situation
On 9 April 2019 4:47:50 PM Dan Ackroyd Danack@basereality.com wrote:
And this RFC conveniently shows only big LSP violation examples like array
-> int, but not widely used narrowing like mixed/object -> specific instance.Type narrowing or contravariant parameters is properly supported for
PHP 7.4:
https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters
, which is why this RFC doesn't need to cover them.cheers
Dan
Ack
If you want the reverse to be true, then your code has bugs waiting to
show themselves. The earlier we can catch these bugs, the better.
One thing to note is that return type information does permit
optimizations in some cases. For instance, I believe if a private
method has a return type constraint of int, then the optimizer can
rule out certain edge cases, and in some cases use type specialized
handlers. We can do these sorts of optimizations in more places if the
LSP issues are always errors, instead of warnings.
So for both correctness and performance, I support this change.
If you want the reverse to be true, then your code has bugs waiting to
show themselves
I don’t, I just don’t want fatal errors in production when I upgrade library I extend. Let me keep logging such issues and fix them later, instead of producing hard crash for customers.
The earlier we can catch these bugs, the better.
What prevents somebody to catch warnings? This RFC is about hard crash that I disagree with. This type of issue is problematic only when expecting parent but injecting child.
I believe rfc deals with reverse situation
On 9 April 2019 4:47:50 PM Dan Ackroyd Danack@basereality.com wrote:
And this RFC conveniently shows only big LSP violation examples like array
-> int, but not widely used narrowing like mixed/object -> specific instance.Type narrowing or contravariant parameters is properly supported for
PHP 7.4:
https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters
, which is why this RFC doesn't need to cover them.cheers
Dan
AckIf you want the reverse to be true, then your code has bugs waiting to
show themselves. The earlier we can catch these bugs, the better.One thing to note is that return type information does permit
optimizations in some cases. For instance, I believe if a private
method has a return type constraint of int, then the optimizer can
rule out certain edge cases, and in some cases use type specialized
handlers. We can do these sorts of optimizations in more places if the
LSP issues are always errors, instead of warnings.So for both correctness and performance, I support this change.
If you want the reverse to be true, then your code has bugs waiting to
show themselves. The earlier we can catch these bugs, the better.One thing to note is that return type information does permit
optimizations in some cases. For instance, I believe if a private
method has a return type constraint of int, then the optimizer can
rule out certain edge cases, and in some cases use type specialized
handlers. We can do these sorts of optimizations in more places if the
LSP issues are always errors, instead of warnings.So for both correctness and performance, I support this change.
If you want the reverse to be true, then your code has bugs waiting to
show themselvesI don’t, I just don’t want fatal errors in production when I upgrade
library I extend. Let me keep logging such issues and fix them later,
instead of producing hard crash for customers.The earlier we can catch these bugs, the better.
What prevents somebody to catch warnings? This RFC is about hard crash
that I disagree with. This type of issue is problematic only when
expecting parent but injecting child.
Have you considered testing your code in a dev environment after
upgrading a library you extend, prior to pushing it to production?
If you blindly push library updates to production without testing,
there are many other types of errors you could run into besides this.
I for one would welcome consistent fatal errors for LSP violations,
to catch these kind of bugs as early as possible during development.
On 10 April 2019 4:48:38 PM Theodore Brown theodorejb@outlook.com wrote:
I for one would welcome consistent fatal errors for LSP violations,
to catch these kind of bugs as early as possible during development.Once again, nowadays warnings are converted to errors during development.
This change affects production only, where this isn't being done.Have you considered testing
I have yet to see codebase with 100% test coverage. Better idea is to use
static analyzer. My objection is for remaining cases.I don't understand what's the benefit of raising fatal error here. Code is
not lsp compliant? Cool, you will get hard error when calling such subclass
anyway. Until that call happens, there is no bug so warning is fine.
I have yet to see codebase with 100% test coverage.
True, but I would think that after upgrading a library, one would at
least do a basic test of loading the code that uses the library to
see if it still works.
Until that call happens, there is no bug so warning is fine.
I disagree. There is a bug, but without a fatal error at compile time
you are less likely to find out about it until the relevant call happens.
I don't understand what's the benefit of raising fatal error here.
Besides the benefit of finding bugs earlier, there is also the benefit
of more performance optimization opportunities that Levi mentioned,
as well as simplifying the PHP engine. 1
Am 09.04.2019 um 10:25 schrieb Nikita Popov:
A small cleanup RFC for PHP 8: https://wik
+1
Hi internals,
A small cleanup RFC for PHP 8: https://wiki.php.net/rfc/lsp_errors
This makes all incompatible method signature (LSP) errors fatal, rather
than only warning in some cases. Especially after
https://wiki.php.net/rfc/parameter-no-type-variance I don't think there's
any reason to keep this backdoor in place.
Heads up: Going to vote tomorrow.
Nikita