On Tue, 20 Aug 2019 at 20:05, Mark Randall wrote:
> 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.
>
> 1) Invalid parameters where ZPP accepts mixed but internally it requires
> a specific type (example: str_replace).
>
> 2) Invalid parameters where ZPP passes but there are additional
> constraints on the value of those parameters (example: hash_xxx with
> non-cryptographic hashes).
>
> 3) 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
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Having in the mean time flooded the GitHub repo with pull requests about
promoting warnings to Errors it would be nice to have a discussion about a
couple of thing in my mind.
If we should add or not a dedicated API/helper as proposed by Mark.
If all of these Errors should just be the basic Error exception class or if
we should introduce dedicated exceptions, in the same vein as TypeError.
What should or should not be elevated to an exception, currently the line
I'm working on is that empty-like arguments, type mismatch and invalid
modes/flags.
However even for some cases where it should be obvious to elevated it to an
Error there may be some wider implications (see the PR about count() [1]).
Best regards
George P. Banyard
[1] https://github.com/php/php-src/pull/4572
