php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #81417 Option to determine if reading an undefined array key should throw a warning
Submitted: 2021-09-05 12:20 UTC Modified: 2022-02-10 11:59 UTC
Votes:5
Avg. Score:4.2 ± 1.6
Reproduced:3 of 4 (75.0%)
Same Version:1 (33.3%)
Same OS:0 (0.0%)
From: nicolas at gestixi dot com Assigned:
Status: Wont fix Package: *Configuration Issues
PHP Version: 8.0.10 OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: nicolas at gestixi dot com
New email:
PHP Version: OS:

 

 [2021-09-05 12:20 UTC] nicolas at gestixi dot com
Description:
------------
In some languages, such as JavaScript, accessing an undefined array key can be performed silently without throwing an error. 

We have a 10 years old code base which does conditions like this everywhere: `if ($array['key'])`.

With PHP 8, this coding style throws a warning if the key is not defined. Replacing it by `if (array_key_exists('key', $array) && $array['key'])` would take us weeks... and, I actually prefer our actual syntax which I found more readable.

For these reasons, I do not plan to upgrade all our code base to avoid this warning. And I do not want to ignore all warnings just for this one...

Would it be possible to add a configuration setting to determine if this coding style should throw a warning, a notice or be silently ignored?

Another solution I see would be to be able to ignore specific warnings/notices.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-09-05 12:22 UTC] nicolas at gestixi dot com
-Summary: Add a configuration option to determine if reading an undefined array key shoul +Summary: Option to determine if reading an undefined array key should throw a warning
 [2021-09-05 12:22 UTC] nicolas at gestixi dot com
Adjust the summary which was too long.
 [2021-09-05 14:39 UTC] php-bugs at allenjb dot me dot uk
You can already silence this warning in your own code using a custom error handler. See https://www.php.net/set_error_handler

You should however keep in mind that warnings tend to get escalated to errors over time (this was a notice since at least 5.4)

The null coalescing operator also provides an alternative "fix" that you may find more readable - like isset() it silences warnings about undefined indexes (and properties or variables): https://www.php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op

In the case you gave, you would write: if ($array['key'] ?? false)
 [2021-09-05 18:41 UTC] nicolas at gestixi dot com
I tried with a custom error handler. Something like:

if (strpos($errstr, 'Undefined index:') !== false OR strpos($errstr, 'Undefined offset:') !== false) return true;
else return false;

But it exhausts the memory before the end of the request.
 [2021-09-06 01:50 UTC] requinix@php.net
-Status: Open +Status: Wont fix
 [2021-09-06 01:50 UTC] requinix@php.net
As a rule of thumb, adding another php.ini option is not the right answer. What's more, adding one that tells PHP to ignore certain types of warnings/errors is opening a very big and very dangerous door that really ought to remain closed.

If your codebase is 10 years old then you'll have to accept that you cannot simply upgrade PHP versions (especially majors) and expect everything to work as it did back in 2011. PHP 7.4 is the last of the 7.x series and will remain in active support for an extended period - use that time to upgrade your codebase to be compatible with 8.x.
 [2022-02-08 17:20 UTC] k dot andris at gmail dot com
"is opening a very big and very dangerous door that really ought to remain closed"... funny. It's a door that's been wide open for many years. It would be helpful requinix, if you'd point us to an RFC or somethine where making this backward incompatible change has been decide instead of lecturing loyal developers. Also keeping them from running away to other languages as I did. Thanks.
 [2022-02-09 07:00 UTC] nicolas at gestixi dot com
Indeed it would be great to know why it is such a very dangerous door...

We still have not started to upgrade our codebase. We have 138627 lines of code to review just because of this "useless" warning. I will have to find a way to automate most of it, or spend weeks doing it…
 [2022-02-09 09:02 UTC] requinix@php.net
Maybe I should have tried asking for more information before closing it. I'll start with a few easy questions.

#1. I have here a sample of 15 different warnings and notices: https://3v4l.org/l8pSY
Which of those should be suppressable by new php.ini settings? Or did I misunderstand, and the request is only about one very specific warning that should be handled with one very specific setting, with a steadfast opposition to the unintentional emergence of a "well we did it for X so why not do it for Y"-type precedent?

#2. Hypothetically, what about all the other sorts of warning and notices that are currently being raised throughout PHP? Are any of them also causing you undue problems as you attempt to update a 10 year old application that was developed in the days of PHP 5.3?
https://github.com/php/php-src/search?l=PHP&q=warning

#3. Given PHP's trend of turning suitable warnings into exceptions, should the settings also extend to suppressing the throwing of those exceptions? Or does it make more sense to spend time adding try/catch blocks to userland code (or, perhaps more sensibly, to disregard the possibility of exceptions entirely) than to wrap questionable array accesses in calls to empty()?

#4. Considering that you feel it is appropriate to ignore at least one type of warning across your application, do you think that PHP should simply remove nuisance warnings entirely and thus allow developers to ignore the sorts of problems they were intended to point out? Or are you concerned that there may be some number of unknown bugs that could be swept under the rug by a unilateral move to remove warnings for the sake of convenience?

#5. Have you heard of the @ operator? Unlike everything else in this reply, I really do mean to ask this without being facetious, as I believe this is likely the most appropriate solution to the problem.

Also, my apologies on behalf of all the PHP developers here for making the sudden decision that an attempt to access an undefined array key should now result in a warning, and for doing so without observing the RFC process by holding a public vote-- oh, wait, no, my bad, actually there was one about this.
https://wiki.php.net/rfc/engine_warnings

Unfortunately the discussion for that RFC was conducted privately between individuals with exclusive access to a secret mailing list that-- oh, nope, I'm sorry, that's not right either.
https://externals.io/message/106713
 [2022-02-09 16:43 UTC] nicolas at gestixi dot com
Thank you a lot for coming back to this.

#1. For me it is only the "Undefined array key" warning which is problematic. The best for me would be to simply return undefined (like in javascript) without warning or notice.

#2. Nope, only this one.

#3. For me, no matter if it is an exception, a warning or a notice, it should have a setting to accept this behavior.

#4. For this warning, I think that PHP should simply remove nuisance warnings entirely and thus allow developers to ignore the sorts of problems they were intended to point out.

#5. Yes, and it is not appropriate for us as we would have to use it at hundreds of places. We just want to be able to check if a property exists by accessing it, like this: `if ($array['key'])`. And this has been used everywhere in our code base. 

Here https://wiki.php.net/rfc/engine_warnings, we can see the 'Undefined array index' error was controversial. For us having them as notice is also problematic as we have to completely disable notices.

Thanks in advance for considering this request. I should have woken up earlier. If I can be of any help with this, please don't hesitate to ask.
 [2022-02-10 11:59 UTC] cmb@php.net
If you want that warning to be (optionally) removed, please start
a discussion on the internals mailing list, and pursue the RFC
process[1].

[1] <https://wiki.php.net/rfc/howto>
 [2022-02-15 11:49 UTC] imsop@php.net
Another alternative way of writing the same thing without warnings is using the "empty" pseudo-function:

if ( ! empty($array['key']) )

Like isset() and "??", it suppresses warnings for undefined vars and elements, and uses the same rules to define "emptiness" as the implicit boolean cast used in an if statement.

I think this version has some value in readability, particularly if $array['key'] is not in fact a boolean, e.g. if you're actually checking for "missing or empty string", or "uninitialised or empty list".
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 18 01:01:28 2024 UTC