[RFC] Consistent type errors for internal functions View as Markdown

Hi internals,

I'd like to bring forward the following proposal for PHP 8, which will make
(zpp) parameter parsing failures always result in a TypeError (rather than
generating a warning+null, depending on circumstances):

https://wiki.php.net/rfc/consistent_type_errors

The goal here is to remove one of the inconsistencies between user-defined
and internal functions, and to put us in a position where we can actually
start specifying type information in arginfo without fear of breaking
things.

Regards,
Nikita

Hi Nikita,

Nikita Popov wrote:

I'd like to bring forward the following proposal for PHP 8, which will
make
(zpp) parameter parsing failures always result in a TypeError (rather
than
generating a warning+null, depending on circumstances):

https://wiki.php.net/rfc/consistent_type_errors

I like this proposal. IMO PHP's E_WARNING + NULL is the worst of its
“Keep Calm and Carry On” (sorry) behaviours, it would be nice to get rid
of it for good, rather than just in the comfy world of strict_types=1.

The goal here is to remove one of the inconsistencies between
user-defined
and internal functions, and to put us in a position where we can actually
start specifying type information in arginfo without fear of breaking
things.

Regrettably, as I pointed out to you via another channel, that idea also
faces the problem of the other deliberate inconsistency I am responsible
for in userland scalar type declarations, namely that the non-nullable
variety of those reject null as a valid value, unlike internal functions
which will happily coerce it. It's funny to mention this here, as the
E_WARNING + NULL behaviour your RFC would drop was a primary
justification of mine for making NULL special here. Unfortunately it's
not the only case, I'm sure uncountably much PHP code relies on things
like strlen($_GET['nonexistent']) working… but I digress.

Right, thanks for pointing that one out. We could either resolve this by
making the behavior of internal functions consistent with user-defined ones
here (which would be another BC break for another RFC), or alternatively we
could make the arginfo handling for internal functions consistent with zpp.
I already mentioned previously that we'll have to drop arginfo type checks
for internal functions if we want to get serious about adding those type
annotations, otherwise we'd be paying for duplicate type checks. That would
leave us with just the zpp behavior, which allows null for scalars. It's
not great, but probably much better than functions either accepting null or
not depending on whether they use arginfo.

Nikita

Hi internals,

I'd like to bring forward the following proposal for PHP 8, which will
make (zpp) parameter parsing failures always result in a TypeError (rather
than generating a warning+null, depending on circumstances):

https://wiki.php.net/rfc/consistent_type_errors

The goal here is to remove one of the inconsistencies between user-defined
and internal functions, and to put us in a position where we can actually
start specifying type information in arginfo without fear of breaking
things.

Regards,
Nikita

Heads up: This is headed for voting on Tuesday, assuming no significant
issues are raised in the meantime.

Nikita

Hi internals,

I'd like to bring forward the following proposal for PHP 8, which will make
(zpp) parameter parsing failures always result in a TypeError (rather than
generating a warning+null, depending on circumstances):

https://wiki.php.net/rfc/consistent_type_errors

The goal here is to remove one of the inconsistencies between user-defined
and internal functions, and to put us in a position where we can actually
start specifying type information in arginfo without fear of breaking
things.

I like this RFC, and from a user perspective, this consistency is much-needed.

While warnings don’t affect continued processing, uncaught TypeErrors do. What is the recommended upgrade path for users? How will this affect the silence @-operator, since I’m sure many users use that to squelch these warnings, and they accept the null or false return value as acceptable for processing the rest of the script.

Cheers,
Ben

Ben Ramsey ben@benramsey.com schrieb am Sa. 16. Feb. 2019 um 18:35:

Hi internals,

I'd like to bring forward the following proposal for PHP 8, which will
make
(zpp) parameter parsing failures always result in a TypeError (rather
than
generating a warning+null, depending on circumstances):

https://wiki.php.net/rfc/consistent_type_errors

The goal here is to remove one of the inconsistencies between
user-defined
and internal functions, and to put us in a position where we can actually
start specifying type information in arginfo without fear of breaking
things.

I like this RFC, and from a user perspective, this consistency is
much-needed.

While warnings don’t affect continued processing, uncaught TypeErrors do.
What is the recommended upgrade path for users? How will this affect the
silence @-operator, since I’m sure many users use that to squelch these
warnings, and they accept the null or false return value as acceptable for
processing the rest of the script.

Cheers,
Ben

This exception is only for argument errors. Fopen would continue to warn +
false when file doesnt exist, which is what @ is nostly used for (using
fopen as example)

This exception is only for argument errors. Fopen would continue to warn + false when file doesnt exist, which is what @ is nostly used for (using fopen as example)

I’ve seen many cases where it’s understood that the function might receive the wrong type, and the developer decides this is okay, so they use the @-operator to ignore the warning and allow the script to continue processing.

Hi internals,

I'd like to bring forward the following proposal for PHP 8, which will
make
(zpp) parameter parsing failures always result in a TypeError (rather
than
generating a warning+null, depending on circumstances):

https://wiki.php.net/rfc/consistent_type_errors

The goal here is to remove one of the inconsistencies between
user-defined
and internal functions, and to put us in a position where we can actually
start specifying type information in arginfo without fear of breaking
things.

I like this RFC, and from a user perspective, this consistency is
much-needed.

While warnings don’t affect continued processing, uncaught TypeErrors do.
What is the recommended upgrade path for users? How will this affect the
silence @-operator, since I’m sure many users use that to squelch these
warnings, and they accept the null or false return value as acceptable for
processing the rest of the script.

Cheers,
Ben

The recommended upgrade path here is basically "deal with it". If a
codebase does this (intentionally calling functions with invalid params and
suppressing errors) a lot, then it might make sense to register an error
handler that collects all "expected parameter" style errors in a production
environment, to easily identify all code that needs to be fixed. The error
messages have a consistent format, so it should be simple to identify and
eliminate all warnings of this type.

Nikita

Am 17.02.2019 um 17:24 schrieb Nikita Popov nikita.ppv@gmail.com:

The recommended upgrade path here is basically "deal with it". If a
codebase does this (intentionally calling functions with invalid params and
suppressing errors) a lot, then it might make sense to register an error
handler that collects all "expected parameter" style errors in a production
environment, to easily identify all code that needs to be fixed. The error
messages have a consistent format, so it should be simple to identify and
eliminate all warnings of this type.

I have an example where this might be harder than necessary:
I'm importing data from an external source. Now in the real-world the import data files can once in a blue moon contain bogus data, e.g. there could be an array instead of a string in one single entry and I'm doing a strlen() on it.

Previously this triggered a Warning but the import job completed for all the other entries while still informing me that something was amiss.

The new way leaves me with three options:

1. My import jobs stops at the single broken entry
2. I can add type-checks for every single field
3. I can start wrapping code with try/catch

Neither of the three options are very appealing to me.
Option 1) might be appropriate when you are importing data which life and death depends on but for lots of stuff you're more interested in having the other 99% of your data updated instead of failing completely.
Option 2) sounds like the most defensive way of programming but it adds lots of boiler-plate code and doesn't feel phpish to me.
Option 3) is Java's RuntimeExceptions madness all over again which is surprisingly had to get right (where to catch what).

I'm a bit surprised the PHP community had such a change of mind concerning Exception in the core and BC breaks over the last years but as the vote seems very much to be in favour of this change I guess I'll have to bite the bullet once PHP 8 arrives.

Regards,

• Chris

I have an example where this might be harder than necessary:
I'm importing data from an external source. Now in the real-world the
import data files can once in a blue moon contain bogus data, e.g.
there could be an array instead of a string in one single entry and I'm
doing a strlen() on it.

Previously this triggered a Warning but the import job completed for
all the other entries while still informing me that something was
amiss.

The problem with Warnings is that processing doesn't just continue for all the other entries, it continues with this entry that turned out to be bogus. So now rather than a missing record, you have a record containing a 0, or the word "array", etc; and the Warning probably won't tell you which record it was that failed.

The new way leaves me with three options:

1. My import jobs stops at the single broken entry
2. I can add type-checks for every single field
3. I can start wrapping code with try/catch

Option 2 can be stated differently as "add validation for each input record": this isn't the same kind of type check as a library function picking up coding mistakes, it's handling of untrusted input.

Option 3 can be thought of as the transactional approach: if an error happens within a logical record, you can log that record to a reject file and continue with the next record.

There is definitely more effort to doing one of these compared to not doing it, but both seem preferable to letting bad data through, whether the language forces you to do something or not.

Regards,

--
Rowan Collins
[IMSoP]