Greetings,
A week or so ago, G.P.B. submitted a pull request that contained a
declare that would automatically elevate certain errors to Error
exceptions (https://github.com/php/php-src/pull/4549).
As part of the discussion, Nikic stated the following:
"I believe it's okay to convert this kind of error condition to an Error
unconditionally (I've been doing this occasionally). Basically any error
condition that indicates a programming error and no reasonable code
should handle locally."
Since then, G.P.B. and I have independently started looking at various
extensions to see what the scale of the task is to convert as many
errors as possible, and you might have noticed the PR's starting to pile up.
Before I go any further with this, and because I'm brand new to
contributing to php-src, I think we need to discuss if we're going to
need an API to handle these changes, particularly in respect to
consistent formatting of error messages and error types.
As mentioned in the various PRs, there's a several situations to consider.
-
Invalid parameters where ZPP accepts mixed but internally it requires
a specific type (example: str_replace). -
Invalid parameters where ZPP passes but there are additional
constraints on the value of those parameters (example: hash_xxx with
non-cryptographic hashes). -
Errors as the result of global state (example: session changes when a
session is active).
For 1) and 2) there is additionally the option of where the parameter
index is known (inside PHP_FUNCTION or passing
INTERNAL_FUNCTION_PARAMETERS) and where it is not (where the error
occurs within a helper function).
For my own contributions, I have added a helper function
php_error_parameter_validation(NULL, arg_index, format, ...) that will
format a consistent message based on its argument number and the
formatted text.
I've got it currently throwing Error exceptions but I can forsee
replacing certain ones with TypeError when dealing with option 1.
Before I continue, can we have a quick discussion about any helpers or
APIs that we may want to make this more consistent, and then we can whip
up a PR that we can then use as the basis for the rest of our changes.
Also:
As part of that PR I would also like to include a universal test script
called trycatch_dump(...) that accepts a list of closures and runs each
one in turn, either printing the var_dump of the result, or printing the
error message of the exception, prefixed with the exception name.
For test cases, which are going to be the biggest part of all this, it
reduces a 5 line try-catch per statement down to:
trycatch_dump(
fn() => func_to_test("hello"),
fn() => func_to_test(1234),
fn() => func_to_test(false)
);
I am however, unsure where this script should be placed in the codebase.
Mark Randall