On Mon, Oct 21, 2019 at 3:52 PM David Negrier < d.negr...@thecodingmachine.com> wrote:
> Hey list, > > TL;DR: > > I'm considering writing an RFC (and a patch) to turn some warning into > exceptions in a number of PHP functions. > I would first like to gather some feedback here. > > The long version: > > It is the first time I'm posting on internals. > Some of you may already know me. I'm one of the authors of > thecodingmachine/safe, a library that wraps all PHP functions that return > "false" on error in similar functions that will throw on exception on > error: https://github.com/thecodingmachine/safe/ > > Surprisingly enough, I've had a huge positive feedback with Safe (I reached > 1000 stars on Github in a few weeks, so I think I can say this is something > that bothers a lot of people). > > Of course, I could simply have written an error handler that would turn > E_WARNING into exceptions but: > > - For some libs, I wanted to write code that would behave consistently > (whatever the settings of error_reporting) > - More than all, having a return type that can be "string|false", or > "object|false" is a true annoyance when you want to write type-safe code > (which is more and more common with the advent of tools like PHPStan or > Psalm). > > For instance, "sprintf" can return "string" or "false" (in case the format > is incorrect). If the string passed to "sprintf" is invalid, I would > certainly prefer knowing right now (with an exception) than risking having > the warning sleep under my nose. > > Needless to say: a good practice in error handling is "fail loud, fail > fast". A warning at some point will most certainly lead to an error a few > lines later if error handling is not done properly. > > If you look at the frameworks out there, I believe all of them are shipping > with an error handler that throws exceptions by default (even on notices). > > Also, my team and I are starting to spend a lot of time maintaining Safe. > So I started wondering if rather than spending time maintaining a patch in > user land, we could not instead spend time fixing things in the core. > > So here we are, my team and I started playing with php-src. We are not > regular C developers, but we managed to write a small patch to turn > "sprintf" warnings into exceptions. > > This is far from ready but the PR is here: > https://github.com/php/php-src/pull/4837 > > We would be happy to promote more warnings to exceptions as those: > - are more predictable > - can be more easily caught / handled > - enhance the type system by removing the "false" return type > > This is going in the same direction as Nikita RFC's regarding reclassifying > "Engine Warnings". > > I understand there have been a lot of discussions going on recently > regarding maintaining backward compatibility vs evolving/sanitizing the > language and I know this is a sensitive topic. > > That being said: > > - most frameworks are already providing an error handler that turns warning > into exceptions (so they should not be impacted by the change) > - there are a huge number of warnings that can be turned into exceptions > with minimal impact ("sprintf" E_WARNING are clearly a good first > candidate) > > Ok, so my team and I are willing to put some time to turn some of those > warnings into exceptions. I understand there are some questions to be > answered regarding the nature of the exception (what class, what class > hierarchy...) > > My questions: > > 1- Do you think there is a chance an RFC on this topic might be accepted? > Shall I start working on it? > 2- Shall we start working on a list of candidate functions? > > David > As mentioned on the PR, changing warnings to Error exceptions in library functions doesn't need an RFC as long as the conversion satisfies the following principle: The error condition must indicate a programming error and it can be reasonably assumed that this error condition is not being handled by checking the return value. Example 1: fopen() on a non-existent file throws a warning and returns false. This absolutely *can not* be converted into an exception, because both a) it does not indicate a programming error (it is a totally expected environment condition) and b) we expect people to check the return value of the function for a "false" value. Example 2: count_chars() accepts a "mode" parameter that can only have the value 0, 1, 2, 3 or 4. Using an out-of-range value results in a warning and false return value. This warning *can* be converted into an exception (and in fact is a ValueError in PHP 8), because passing an invalid mode is an unambiguous programming error. It is not always as obvious into which category a particular error condition falls, this is decided on a case-by-case basis. Regards, Nikita
