Revisiting RFC: Engine Warnings -- Undefined array index

2 years ago by Derick Rethans — view source

unread

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide for
themselves how they want this situation to be handled. Given that:

PHP has been treating this as an E_NOTICE for over 20 years

But not in the last three years.

the change is a breaking change affecting many large codebases
ubiquitously (unless one were to unadvisedly suppress E_WARNING errors)

No, code doesn't break. It now shows a warning instead of an error. There is no behavioural change.

7.4 is now deprecated

Yes, as each release does 2+1 years. Which also means you've had 3 years to address this in your code base(s) already.

I think making the error level of "Undefined index" configurable is a very
reasonable suggestion, and I support it.

Adding a configuration setting to make this a notice on some installations and not on others, would just mean that anything that needs to be able to run everywhere needs to take care to not rely on the setting either way, making it harder to develop portable code.

We're also not generally anything near keen on adding settings that change behaviour , and suggesting to add one to make a warning a notice seems very far short of a bar that needs to be reached before many of us would agree to add a setting to make PHP less portable.

Alternatively you can just fix your code.

Are we able to revisit this topic as a community and potentially bring in
more PHP developers from around the world to weigh in?

You're always free to follow the RFC process, but I think you'll just up wasting your, and everybody else's, time with it.

I can't see this being reversed, nor a setting added for, through an RFC process.

cheers
Derick

2 years ago by Thomas Hruska — view source

unread

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide for
themselves how they want this situation to be handled. Given that:

PHP has been treating this as an E_NOTICE for over 20 years

But not in the last three years.

PHP 8.x is only starting to roll out in Linux LTS distro packages at the
end-user level. I've moved to 8.x internally but I am, in particular,
waiting for Ubuntu Server 22.04.2 to officially drop before beginning
meticulously planned upgrades to mission critical systems. I suspect
many people are in a similar holding pattern who are currently running
packaged 7.4.x and are just now discovering all of the changes for PHP
8.x as they are planning out their system upgrade paths in the coming
months. While you probably wish everyone marched in step with
PHP-latest, that's simply not feasible in reality.

The type of post that the OP sent to internals is likely to become more
common in the next few months as PHP 8.x starts rolling out globally.
Most businesses are waiting for at least the holidays to conclude before
performing any major system upgrades if not longer for specific OS
releases to drop.

--
Thomas Hruska
CubicleSoft President

CubicleSoft has over 80 original open source projects and counting.
Plus a couple of commercial/retail products.

What software are you looking to build?

2 years ago by Robert Landers — view source

unread

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide for
themselves how they want this situation to be handled. Given that:

PHP has been treating this as an E_NOTICE for over 20 years

But not in the last three years.

PHP 8.x is only starting to roll out in Linux LTS distro packages at the
end-user level. I've moved to 8.x internally but I am, in particular,
waiting for Ubuntu Server 22.04.2 to officially drop before beginning
meticulously planned upgrades to mission critical systems. I suspect
many people are in a similar holding pattern who are currently running
packaged 7.4.x and are just now discovering all of the changes for PHP
8.x as they are planning out their system upgrade paths in the coming
months. While you probably wish everyone marched in step with
PHP-latest, that's simply not feasible in reality.

The type of post that the OP sent to internals is likely to become more
common in the next few months as PHP 8.x starts rolling out globally.
Most businesses are waiting for at least the holidays to conclude before
performing any major system upgrades if not longer for specific OS
releases to drop.

--
Thomas Hruska
CubicleSoft President

CubicleSoft has over 80 original open source projects and counting.
Plus a couple of commercial/retail products.

What software are you looking to build?

--

To unsubscribe, visit: https://www.php.net/unsub.php

Hello all,

The recent change to elevating "Undefined index" from E_NOTICE to E_WARNING
set forth and passed by https://wiki.php.net/rfc/engine_warnings to me
seems antithetical to what PHP has been and what has made it a choice for
developers over the last few decades.

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

2 years ago by Andreas Leathley — view source

unread

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.
I think it is quite the opposite: calling the error handler because of
E_NOTICE (or now E_WARNING) is quite the additional overhead, and if
projects just ignore E_NOTICE their applications will be slowed down
because of that, while fixing the code will avoid the error handler
altogether. Just because something is longer to write in a programming
language also does not make it necessarily slower. But if you have any
actual measurements why using the null coalescing operator would slow
down code in general that would be something useful to share.

2 years ago by Robert Landers — view source

unread

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.
I think it is quite the opposite: calling the error handler because of
E_NOTICE (or now E_WARNING) is quite the additional overhead, and if
projects just ignore E_NOTICE their applications will be slowed down
because of that, while fixing the code will avoid the error handler
altogether. Just because something is longer to write in a programming
language also does not make it necessarily slower. But if you have any
actual measurements why using the null coalescing operator would slow
down code in general that would be something useful to share.

--

To unsubscribe, visit: https://www.php.net/unsub.php

Ah, that's a good point. I ran some tests and was surprised that it
was slower due to needing to call the error handler. I even disabled
E_WARNING in my php.ini to see if that made a difference (it did, but
still not the same quite as fast as using null coalesce).

Results are here:

https://gist.github.com/withinboredom/bd7276b7703663c26c12a18820747bcb

Interestingly the null coalesces used more memory. That surprised me
and was unexpected. Apparently using coalesce requires some temporary
variables to work. Perhaps there would be a benefit for a
FETCH_DIM_IS_COALESCE opcode?

2 years ago by Craig Francis — view source

unread

I suspect many people are in a similar holding pattern who are currently running packaged 7.4.x and are just now discovering all of the changes for PHP 8.x as they are planning out their system upgrade paths in the coming months. While you probably wish everyone marched in step with PHP-latest, that's simply not feasible in reality.

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);

?

2 years ago by Rowan Tommins — view source

unread

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).
[...]
The null value can come from many sources (e.g. GET/POST/COOKIE/databases)

These two examples are interesting in conjunction: $_GET, $_POST, and
$_COOKIE will never contain null values unless you have code writing
directly to them, because HTTP has no representation for it, only empty
strings. So the only context where you'll get a null from them is
because you mishandled an undefined array key, which as you say is often
an indicator of a mistake.

Data retrieved from a database is probably the most common source of
actual null values. Sometimes, it's important to distinguish those nulls
from empty strings, sometimes you don't care - but that's true of
undefined array keys as well. Which is ultimately the problem with all
of these discussions - some people will see more benefits from catching
extra mistakes, others will see more cost from fixing code that's lazy
but not actually broken.

An interesting example I came across recently was a user-defined
function for SQL string escaping (I'd like to stress legacy; I know that
proper parameters avoid this issue). The function always expected a
string, but didn't check that, so if given a PHP null, it would put ''
in the SQL rather than NULL, leading to data corruption. I have
effectively added the same notice to it as PHP has to built-in
functions, to track down where else this might be causing problems
without escalating to an error immediately.

Regards,

--
Rowan Tommins
[IMSoP]

2 years ago by Craig Francis — view source

unread

The null value can come from many sources (e.g. GET/POST/COOKIE/databases)

These two examples are interesting in conjunction: $_GET, $_POST, and $_COOKIE will never contain null values unless you have code writing directly to them, because HTTP has no representation for it, only empty strings.

Most frameworks return NULL when the user has not provided the value:

$search = $request->input('q'); // Laravel
$search = $request->get('q'); // Symfony
$search = $this->request->getQuery('q'); // CakePHP
$search = $request->getGet('q'); // CodeIgniter

This is also common, to avoid undefined indexes (as you suggested earlier):

$search = ($_GET['q'] ?? NULL);

And some developers use:

$search = filter_input(INPUT_GET, 'q');

That said, I'm glad you have found a positive example from this change.

Craig

2 years ago by Rowan Tommins — view source

unread

Most frameworks return NULL when the user has not provided the value:

 $search = $request->input('q'); // Laravel
 $search = $request->get('q'); // Symfony
 $search = $this->request->getQuery('q'); // CakePHP
 $search = $request->getGet('q'); // CodeIgniter

Fair enough.

Although presumably they return null rather than an empty string
precisely so that users can check if the value was provided, without
providing an extra method equivalent to isset($_GET['q']), e.g.

if ( $search === null ) {
    render_search_form();
}
elseif ( trim($search) === '' ) {
    show_validation_error();
}
else {
    perform_search($search);
}

For cases where you don't need that distinction, Laravel, Symfony, and
CakePHP all allow a default to be passed as the second parameter.

Regards,

--
Rowan Tommins
[IMSoP]

2 years ago by Craig Francis — view source

unread

Although presumably they return null rather than an empty string precisely so that users can check if the value was provided, without providing an extra method equivalent to isset($_GET['q']), e.g.

[...]

For cases where you don't need that distinction, Laravel, Symfony, and CakePHP all allow a default to be passed as the second parameter.

Yep, that's right... but it's not easy to know if that distinction is needed for every single variable (why it's easier to work around this issue at the sinks, as Rector now does).

I assume the next step is for PHP is to deprecate null coercion for all contests; e.g. concat ('Search: ' . $search), comparisons ('' != null), arithmetic, and the remaining functions like print()/echo()/sprintf('%s')... and then deprecate all other forms of coercion (e.g. 5 + '3')... because that will be fun :-)

Craig

2 years ago by Derick Rethans — view source

unread

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide
for themselves how they want this situation to be handled. Given
that:

PHP has been treating this as an E_NOTICE for over 20 years

But not in the last three years.

<snip>

The type of post that the OP sent to internals is likely to become
more common in the next few months as PHP 8.x starts rolling out
globally.

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

cheers,
Derick

https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
Host of PHP Internals News: https://phpinternals.news

mastodon: @derickr@phpc.social @xdebug@phpc.social
twitter: @derickr and @xdebug

2 years ago by Thomas Hruska — view source

unread

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide
for themselves how they want this situation to be handled. Given
that:
- PHP has been treating this as an `E_NOTICE` for over 20 years
But not in the last three years.
<snip>

The type of post that the OP sent to internals is likely to become
more common in the next few months as PHP 8.x starts rolling out
globally.

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

cheers,
Derick

Not for those of us that run the package managed version of PHP in
Ubuntu Server LTS it hasn't.

https://wiki.ubuntu.com/Releases

PHP 8.x is brand new as of next year from the perspective of those who
patiently wait for certain OS level updates because overall system
stability is important for critical production servers. The x.x.2 for
the Ubuntu LTS series tends to be sufficiently stable for most
environments but 22.04.2 hasn't been released yet. Hence my earlier
comment.

--
Thomas Hruska
CubicleSoft President

CubicleSoft has over 80 original open source projects and counting.
Plus a couple of commercial/retail products.

What software are you looking to build?

2 years ago by Lynn — view source

unread

On Tue, Dec 13, 2022 at 4:55 PM Thomas Hruska thruska@cubiclesoft.com
wrote:

Not for those of us that run the package managed version of PHP in
Ubuntu Server LTS it hasn't.

https://wiki.ubuntu.com/Releases

PHP 8.x is brand new as of next year from the perspective of those who
patiently wait for certain OS level updates because overall system
stability is important for critical production servers. The x.x.2 for
the Ubuntu LTS series tends to be sufficiently stable for most
environments but 22.04.2 hasn't been released yet. Hence my earlier
comment.

Lagging behind like that is a choice though.

2 years ago by Larry Garfield — view source

unread

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide
for themselves how they want this situation to be handled. Given
that:
- PHP has been treating this as an `E_NOTICE` for over 20 years
But not in the last three years.
<snip>

The type of post that the OP sent to internals is likely to become
more common in the next few months as PHP 8.x starts rolling out
globally.

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

cheers,
Derick
Not for those of us that run the package managed version of PHP in
Ubuntu Server LTS it hasn't.

https://wiki.ubuntu.com/Releases

PHP 8.x is brand new as of next year from the perspective of those who
patiently wait for certain OS level updates because overall system
stability is important for critical production servers. The x.x.2 for
the Ubuntu LTS series tends to be sufficiently stable for most
environments but 22.04.2 hasn't been released yet. Hence my earlier
comment.

Lagging behind on distro LTS releases for production is one thing. But if the dev team and/or their managers aren't even looking at PHP 8 for 2 years, and then getting shocked at deprecations or known-bad-practices like undefined vars becoming a louder message, that's entirely on them.

Developing under E_ALL has been the recommendation for... 15 years? 20 years? If someone has been doing that at any point since George W. Bush's first term, the undefined vars error messages would have long-since been fixed and 8.0 would be a cakewalk.

Anyone surprised by any of the changes in recent versions that boil down to "know your types and know if you actually have a variable" has been living under a rock for the last 20 years. None of this has been a secret. The language has been providing more and more tools to know-your-types for years.

And yes, I used to work on a system that didn't do that properly. (TYPO3, ~800,000 LOC) It took me a few weeks, but we still managed to fix all of these issues in mostly one-person-month, mostly with dropping "?? null" around in various places. It's not the world-destroyer people make it out to be.

--Larry Garfield

2 years ago by Deleu — view source

unread

Anyone surprised by any of the changes in recent versions that boil down
to "know your types and know if you actually have a variable" has been
living under a rock for the last 20 years. None of this has been a
secret. The language has been providing more and more tools to
know-your-types for years.

I have seen this type of comment here many times and I don't understand why
it keeps being repeated. The world is made up of many rocks and we're all
living under one of them. This whole thread is nothing but
my-rock-is-the-only-rock-everyone-else-should-live-under type of comments.

And yes, I used to work on a system that didn't do that properly. (TYPO3,
~800,000 LOC) It took me a few weeks, but we still managed to fix all of
these issues in mostly one-person-month, mostly with dropping "?? null"
around in various places. It's not the world-destroyer people make it out
to be.

Do you honestly think that everyone with a small/medium PHP project
(500k~1M LOC) out there has someone as competent as you capable of
achieving what you did? That is definitely a dream rock to live under.

--
Marco Deleu

2 years ago by Larry Garfield — view source

unread

And yes, I used to work on a system that didn't do that properly. (TYPO3,
~800,000 LOC) It took me a few weeks, but we still managed to fix all of
these issues in mostly one-person-month, mostly with dropping "?? null"
around in various places. It's not the world-destroyer people make it out
to be.

Do you honestly think that everyone with a small/medium PHP project
(500k~1M LOC) out there has someone as competent as you capable of
achieving what you did? That is definitely a dream rock to live under.

In all modesty, 95% of the changes I made to get TYPO3 ready for 8.0 could have been made by a junior dev with a checklist and a few patterns to copy and paste. "Run tests, look at logs, fix a few lines, repeat." It was hardly the most intellectually stimulating work I've done.

--Larry Garfield

2 years ago by Derick Rethans — view source

unread

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide
for themselves how they want this situation to be handled. Given
that:
- PHP has been treating this as an `E_NOTICE` for over 20 years
But not in the last three years.
<snip>

The type of post that the OP sent to internals is likely to become
more common in the next few months as PHP 8.x starts rolling out
globally.

PHP 8 has been rolling out for two years "globally" already.
Not for those of us that run the package managed version of PHP in Ubuntu Server LTS it hasn't.

That's your and their choice, and not a problem that we here should care about. I certainly don't.

There are also well established third party repositories (sury and remi) to cater for this use case as well.

From the PHP project's point of view, PHP 8 has been out for more than two years.

cheers
Derick

2 years ago by Dan Liebner — view source

unread

No, code doesn't break. It now shows a warning instead of an error. There
is no behavioural change.
It breaks my app. Does that count? And didn't you follow up by referencing
this as "adding settings that change behaviour"? Does it change behavior or
not?

Adding a configuration setting to make this a notice on some
installations and not on others, would just mean that anything that needs
to be able to run everywhere needs to take care to not rely on the setting
either way, making it harder to develop portable code.
What code now relies on this raising a warning rather than a notice that
can't consistently deal with it by using isset or the null coalescing
operator? Or are you subtly referring to the fact that this RFC is a
stepping stone to escalating the severity to throwing an error?
https://wiki.php.net/rfc/undefined_variable_error_promotion -> "This RFC
proposes that accessing an undefined variable is rendered illegal behaviour
in the next major version of PHP, and will result in an Error exception
being thrown if it occurs."

Here's another suggestion:
Make accesses to undefined array keys (objects, variables, etc) return a
new undefined primitive. That way, developers who are focused on writing
concise, readable code can continue to write and maintain code manageably,
and developers who are keen on writing a lot of code in a more "strict"
manner can handle undefined array key accesses consistently, without having
to rely on configuration settings.

just fix your code.
Practically speaking, I'd much more likely stay on 7.4 or migrate to Node.

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide for
themselves how they want this situation to be handled. Given that:

PHP has been treating this as an E_NOTICE for over 20 years

But not in the last three years.

the change is a breaking change affecting many large codebases
ubiquitously (unless one were to unadvisedly suppress E_WARNING errors)

No, code doesn't break. It now shows a warning instead of an error. There
is no behavioural change.

7.4 is now deprecated

Yes, as each release does 2+1 years. Which also means you've had 3 years
to address this in your code base(s) already.

I think making the error level of "Undefined index" configurable is a very
reasonable suggestion, and I support it.

Adding a configuration setting to make this a notice on some installations
and not on others, would just mean that anything that needs to be able to
run everywhere needs to take care to not rely on the setting either way,
making it harder to develop portable code.

We're also not generally anything near keen on adding settings that change
behaviour , and suggesting to add one to make a warning a notice seems very
far short of a bar that needs to be reached before many of us would agree
to add a setting to make PHP less portable.

Alternatively you can just fix your code.

Are we able to revisit this topic as a community and potentially bring in
more PHP developers from around the world to weigh in?

You're always free to follow the RFC process, but I think you'll just up
wasting your, and everybody else's, time with it.

I can't see this being reversed, nor a setting added for, through an RFC
process.

cheers
Derick

2 years ago by Rowan Tommins — view source

unread

It breaks my app. Does that count?

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,

--
Rowan Tommins
[IMSoP]

2 years ago by Andreas Leathley — view source

unread

It breaks my app. Does that count?

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.

Here's another suggestion:
Make accesses to undefined array keys (objects, variables, etc) return a
new undefined primitive. That way, developers who are focused on writing
concise, readable code can continue to write and maintain code manageably,
and developers who are keen on writing a lot of code in a more "strict"
manner can handle undefined array key accesses consistently, without having
to rely on configuration settings.

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.

2 years ago by Dan Liebner — view source

unread

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.

No, code doesn't break. It now shows a warning instead of an error.
There is no behavioural change.
It breaks my app. Does that count? And didn't you follow up by referencing
this as "adding settings that change behaviour"? Does it change behavior or
not?

Adding a configuration setting to make this a notice on some
installations and not on others, would just mean that anything that needs
to be able to run everywhere needs to take care to not rely on the setting
either way, making it harder to develop portable code.
What code now relies on this raising a warning rather than a notice that
can't consistently deal with it by using isset or the null coalescing
operator? Or are you subtly referring to the fact that this RFC is a
stepping stone to escalating the severity to throwing an error?
https://wiki.php.net/rfc/undefined_variable_error_promotion -> "This RFC
proposes that accessing an undefined variable is rendered illegal behaviour
in the next major version of PHP, and will result in an Error exception
being thrown if it occurs."

Here's another suggestion:
Make accesses to undefined array keys (objects, variables, etc) return a
new undefined primitive. That way, developers who are focused on writing
concise, readable code can continue to write and maintain code manageably,
and developers who are keen on writing a lot of code in a more "strict"
manner can handle undefined array key accesses consistently, without having
to rely on configuration settings.

just fix your code.
Practically speaking, I'd much more likely stay on 7.4 or migrate to Node.

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide for
themselves how they want this situation to be handled. Given that:

PHP has been treating this as an E_NOTICE for over 20 years

But not in the last three years.

the change is a breaking change affecting many large codebases
ubiquitously (unless one were to unadvisedly suppress E_WARNING
errors)

No, code doesn't break. It now shows a warning instead of an error. There
is no behavioural change.

7.4 is now deprecated

Yes, as each release does 2+1 years. Which also means you've had 3 years
to address this in your code base(s) already.

I think making the error level of "Undefined index" configurable is a
very
reasonable suggestion, and I support it.

Adding a configuration setting to make this a notice on some
installations and not on others, would just mean that anything that needs
to be able to run everywhere needs to take care to not rely on the setting
either way, making it harder to develop portable code.

We're also not generally anything near keen on adding settings that
change behaviour , and suggesting to add one to make a warning a notice
seems very far short of a bar that needs to be reached before many of us
would agree to add a setting to make PHP less portable.

Alternatively you can just fix your code.

Are we able to revisit this topic as a community and potentially bring in
more PHP developers from around the world to weigh in?

You're always free to follow the RFC process, but I think you'll just up
wasting your, and everybody else's, time with it.

I can't see this being reversed, nor a setting added for, through an RFC
process.

cheers
Derick

2 years ago by Rowan Tommins — view source

unread

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.

Can you expand a bit on how you think distinguishing "undefined" from
"null" would help? Let's keep the focus on possible solutions, not just
arguing in circles.

It's always been a source of confusion to me that JS has both, and the
syntax for working them seems far from elegant (unless things have
improved, and you no longer need to use typeof to detect undefined?);
but maybe I'm looking at it wrong.

Regards,

--
Rowan Tommins
[IMSoP]

2 years ago by Dan Liebner — view source

unread

Can you expand a bit on how you think distinguishing "undefined" from
"null" would help?

First off, I would be perfectly happy if undefined array/object index
accesses went back to raising E_NOTICE. Undefined variables already resolve
to NULL so in that sense NULL is already the "undefined" primitive.

That said, if PHP were to introduce an undefined primitive, here's what I
think would change:

NULL remains, and as such represents an "empty", yet defined, value.
Undefined variables and array/object accesses now resolve to "undefined"
$var = undefined is the equivalent of calling unset($var)
Biggest breaking change would probably be $undefinedVar === null is no
longer true

<?php

// Here's how I think it would look in code

null === undefined; // false
null == undefined; // true

isset($undy); // false
$undy; // undefined, no error
$undy === undefined; // true

$arr = [];
$arr['foo']; // undefined, no error

$obj = (object)[];
$obj->prop; // undefined, no error

$undy->method(); // fatal error

function($param = undefined) {} // fatal error

// more difficult scenarios...
// JavaScript would throw an error
// classic PHP behavior would resolve to null, therefore now undefined
// either is arguably valid, latter would create need for optional chaining
$undy['key']; // ?
$undy->prop; // ?

2 years ago by Rowan Tommins — view source

unread

Here's my biggest criticism: the change essentially forces this:
$arr['key']

to be rewritten as this (or some other intolerably bad alternative):
isset($arr) && isset($arr['key']) && $arr['key']

Fortunately, this is not the case, the actual replacement code is this:

$arr['key'] ?? null

There has previously been a suggestion of introducing an even shorter syntax, such as this:

$arr[?'key']

(With the difference that only one key is marked optional, whereas ?? allows any number of dimensions - $foo['bar']['baz'] ?? null would be equivalent to $foo[?'bar'][?'baz'].)

This would avoid the inconsistency of configurable behaviour, and allow users to make clear with a single keystroke that they know a key might be missing, while giving them useful feedback for typos and other mistakes that lead to a key being unexpectedly missing.

I think this is a general approach that should be more explicitly embraced: introduce convenient syntax for valid use cases, while making accidents obvious.

Regards,
Hi Dan,

Rowan Tommins
[IMSoP]

2 years ago by Derick Rethans — view source

unread

Here's my biggest criticism: the change essentially forces this:
$arr['key']

to be rewritten as this (or some other intolerably bad alternative):
isset($arr) && isset($arr['key']) && $arr['key']

Fortunately, this is not the case, the actual replacement code is this:

$arr['key'] ?? null

Or before ?? was a thing, like:

filter_input(INPUT_GET, 'key')

cheers,
Derick

--
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
Host of PHP Internals News: https://phpinternals.news

mastodon: @derickr@phpc.social @xdebug@phpc.social
twitter: @derickr and @xdebug

2 years ago by David Gebler — view source

unread

As the RFC points out, "Some languages, such as JavaScript, do not consider
accesses to undefined array keys to be an error condition at all, and allow
such an operation to be performed silently."

It is by definition an error condition though. You're trying to access
something which doesn't exist.

For the last 20 years, it's been my impression that PHP also embraced this
philosophy, and that raising an E_NOTICE for "Undefined index" was more or
less offered as a debugging tool, as a matter of practicality.

In the earlier days of PHP, it was more acceptable to allow garbage in
scripts. Generally, the community consensus has been that trending away
from that has been a positive.

It's strongly arguable that E_NOTICE was never appropriate for undefined
array index access, since it is not really something which can occur in
"the normal course of running a script" (since it necessarily means your
script is broken).

JavaScript returns the undefined primitive for accesses to undefined array
and object keys. PHP returns the NULL primitive respectively despite
raising an error.

Here's my biggest criticism: the change essentially forces this:
$arr['key']

to be rewritten as this (or some other intolerably bad alternative):
isset($arr) && isset($arr['key']) && $arr['key']

This is a major regression in my opinion as far as productivity and
readability are concerned.

It seems strange to quibble about readability and syntax when you're
talking about a situation in your code which is by definition an error
condition. Accessing something which doesn't exist isn't the same as
accessing something which does exist and it not having a value (i.e. the
value is a null).

Null = "Can I have the address for this customer please?", "No sorry, we
don't have an address on file for them.
"Undefined = "Can I have the gooble-gabble for this customer please?", "The
what?"

But sure, we don't always have a guarantee about the structure of some
data, especially in web where input usually comes from an HTTP request
which may or may not contain anything. But in those situations we can check
instead of making assumptions.

Good = "Hey, just checking, do we have something called gooble-gabble on
file for this customer?", "Sure, here it is" for one customer, "No, sorry
that's not a thing" for another (this is your array_key_exists / isset /
null coalesce / whatever)

Bad = "Can I have the gooble-gabble for this customer please?", "Sure, here
it is" for one customer, "The what?" for another

It has been proposed to make the error level of "Undefined index"
configurable so that teams and individual developers can decide for
themselves how they want this situation to be handled.

There's a much better way of allowing developers to decide for themselves
how to handle the gooble-gabble situation in a future major release; have
undefined index access throw an UndefinedIndexError which can be caught.

Then we can say

"Can I have the gooble-gabble for this customer please? And if
gooble-gabble isn't a thing, no problem, I can deal with that"

That's my two cents, cheers.

Revisiting RFC: Engine Warnings -- Undefined array index

cheers, Derick

Regards, Hi Dan,

cheers,
Derick

Regards,
Hi Dan,