Revisiting RFC: Engine Warnings -- Undefined array index

On Tue, Dec 13, 2022 at 12:36 AM Thomas Hruska <thruska@cubiclesoft.com> wrote:

Hello all,

From my perspective, in the last few years, it seems that PHP has been trending towards "preventing dev mistakes" over anything else. IMHO, this has prevented us from getting some really nice language features, such as operator overloading (because "devs might make mistakes"). I think it is worth pointing out that PHP has always had this kind of interesting dichotomy towards letting devs do what they want and preventing them from doing 'bad' things. For example, even though you could divide by zero you could never create a socket with AF_PACKET and send raw IP packets from PHP. Is this a bad thing? I don't know, but it's interesting to watch the language evolve and so far the changes have revealed hidden bugs in existing code bases, at least in my experience. However, fixing those bugs has been a nightmare because some of them have been there for 10-15 years. Ancient silent bugs are suddenly showing up and you have to make a hard call on whether to fix the ancient mistake or keep the broken behavior. It isn't always a simple decision and may often be a business decision vs. a technical one. Interestingly, with the array access warning, you now have to write much more boilerplate code just to do something that was once the default behavior. Not only does this require much more typing, but it also introduces much more cognitive load as you have to figure out whether a key has been set before (or be defensive, which introduces potential performance implications). Even then, using ?? on array access can introduce more bugs than it is meant to prevent. Most devs find out the hard way that ?? has an unintuitive order of operations: typed: $a['foo'] ?? null || $a['bar'] ?? null intended: ($a['foo'] ?? null) || ($a['bar'] ?? null) Further, writing code like this increases the number of opcodes needed to perform relatively simple logic by ~150%, increasing end-user latency and spending CPU cycles somewhat needlessly. Personally, I'm a fan of some of the changes made to the language, and I look forward to PHP's future. I hope this "preventing dev mistake" mindset is just a phase though because, IMHO, it really has held back some fantastic proposals that people put a lot of time and effort into. It's also caused us to have to write much more boilerplate code for a debatable benefit. Anyway, as an outside observer, that's my 2¢ on the matter. Robert Landers Software Engineer Utrecht NL

On 12 Dec 2022, at 23:36, Thomas Hruska <thruska@cubiclesoft.com> wrote:

Hi Thomas, Dan, I know a few teams that have started using 8.x on their development servers (while 7.x is still on demo/live). Each team has their difficulties, but fortunately most issues can be ignored for now (as in, code works, and gives you time to make changes)... undefined indexes are a classic, but once you've found them all (the tricky bit), it often makes your code better, and helps identify mistakes (similar to undefined variables). But, the one change I still cannot justify, is the deprecation of null being passed to the ~335 functions like htmlspecialchars. The null value can come from many sources (e.g. GET/POST/COOKIE/databases), and while you can still use null coercion in contexts like string concat (empty string), arithmetic (the value 0), or a boolean (false); finding all the function calls is incredibly hard (one team I work with, who has a fairly good test coverage, is still finding these over a year later)... at least it can be ignored for 8.x, but it's going to be a nightmare for 9.0... I tried writing an RFC to revert this change (considering there was pretty much no discussion went into it, and I cannot find a benefit for breaking coercion in this one context), but it was met with a lot of hostility from internals, so I'm finding ways to work around it, because I don't like systems using old/unsupported software (and I've got some ugly hacks for it, like using the latest version of Rector to flood your code base with `(string) $var`, or to use a namespace). Craig https://wiki.php.net/rfc/null_coercion_consistency <https://wiki.php.net/rfc/null_coercion_consistency> https://externals.io/message/117501 <https://externals.io/message/117501> null_coercion.php <?php namespace null_coercion; function strlen(?string $string): int { return \strlen((string) $string); } // x335 functions ?> index.php <?php namespace null_coercion; require_once('null_coercion.php'); // Probably can be auto-loaded. echo strlen('AAA'); echo strlen(NULL); echo strlen(12); echo strlen(false); ?>

On Mon, 12 Dec 2022, Thomas Hruska wrote:

<snip>

PHP 8 has been rolling out for two years "globally" already. cheers, Derick

On 13/12/2022 12:53, Dan Liebner wrote:

The only way a Warning can break an application is if you're running a custom handler which promotes it to an error. If that is your case, then blaming PHP is a little bit like programming your car to do an emergency stop at every road sign, then complaining to the highways agency when someone runs into the back of you. On the other hand, in keeping with my earlier suggestion of making valid use cases easy, PHP could help by making it easier to filter messages in that callback. At the moment, you'd have to match the content of the message, which is not ideal; a unique code for each message type would be much nicer, but there's potentially quite a lot of initial work to assign codes to all the existing messages. That way, you could promote some warnings to errors, and ignore others; or log them with your own choice of severity, possibly not even limiting yourself to those that PHP provides. Of course, this won't help in the long run for things which eventually do become errors, but for everything below that level, it would be much more powerful than a one-off setting to change the severity of one specific message. Regards,

On 13.12.22 13:53, Dan Liebner wrote:

This sounds like you completely ignore notices in your application yet elevate warnings to exceptions/errors. It would be better to log both notices and warnings without generating exceptions/errors, and look through those logs from time to time, to see possible issues (and nothing would currently break that way). I used to suppress notices in my applications too, but after logging them it was my experience that notices can usually be easily avoided and often hint at oversights / possible improvements.

That would likely be a major change and break in the language (which is usually frowned upon), but if you have a clear concept, you can create an RFC for it and start a discussion. I do think that your view on "concise, readable code" would in my mind be code which is more prone to bugs and unclear in its intentions. If an array index possibly does not exist and you check its value, it would already be unclear to me if you mean to check for its existence, or for a null value, or for a non-empty string, or something else. Making it clear the index might not exist gives a reader more information, and making the expected types (and checks) clearer in general is not an exercise to be strict for fun, but to avoid bugs because of unexpected values.

Can I also just point out that by current definitions the "null coalescing operator" isn't properly named, it should be the "undefined/null coalescing operator". The only reason it is able to get away with not raising an error for undefined variables is that it's described as "syntactic sugar" for isset(), which is itself the exception to the rule for not raising undefined errors. Other languages, such as JavaScript, would raise an error for an expression such as `undefinedvar ?? 0`. And JavaScript conveniently offers the undefined primitive for precisely handling attempted accesses of undefined keys, while retaining the brevity and convenience of a truthy/falsy `obj.key` check. If you're set on raising errors and eventually exceptions for undefined key accesses, we need a concise, performant way to say `isset($varExpression) ? $varExpression : null` that's still way more concise than `$varExpression ?? null` that's baked into the language. On Tue, Dec 13, 2022 at 7:53 AM Dan Liebner <dliebner@gmail.com> wrote: