Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117152
MIME-Version: 1.0
References: <CAPzBOBMH15JHkbs+RNHsGbV50zEParhSTn=2P_wkiWM2N_29+A@mail.gmail.com>
 <621a56dd.1c69fb81.67b1.242aSMTPIN_ADDED_MISSING@mx.google.com>
 <CAPzBOBOZO=SKs0W9PK6UpbRAX6gzGq9DFa-2cHEgqYx33pqzOQ@mail.gmail.com>
 <621ab36a.1c69fb81.b1a5.462dSMTPIN_ADDED_MISSING@mx.google.com>
 <CAPzBOBNOZYtHOFJh-8B1kUaLKPad2UKkdJtQXb_Tr8UjucjmPA@mail.gmail.com> <621b6215.1c69fb81.ae9e6.4901SMTPIN_ADDED_MISSING@mx.google.com>
In-Reply-To: <621b6215.1c69fb81.ae9e6.4901SMTPIN_ADDED_MISSING@mx.google.com>
Date: Sun, 27 Feb 2022 16:11:27 +0100
Message-ID: <CAPzBOBNtHkFSw0837b00QUrJ_Z9D064mXxO0dvtYBYGUaKoX2Q@mail.gmail.com>
To: Mark Randall <marandall@php.net>
Cc: internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000037e27005d90159fb"
Subject: Re: [PHP-DEV] Re: Proposal for RFC to remove undefined
 arrayindexwarning when used ina ternary
From: landers.robert@gmail.com (Robert Landers)

--00000000000037e27005d90159fb
Content-Type: text/plain; charset="UTF-8"

On Sun, Feb 27, 2022 at 12:35 PM Mark Randall <marandall@php.net> wrote:

> On 27/02/2022 09:12, Robert Landers wrote:
> > I'd also venture that this warning has caused more harm than good, in
> that
> > writing "$var['something'] ?? null" is second nature when writing new
> > code, even if there is practically no chance for a non-existent key.
>
>
>
> Using null coalesce should only be used when you know you have the
> possibility of a missing key.
>

That can't always be known, or requires tracing the stack to see where
it comes from and what modifies the array before it is passed to the
function you are modifying, it is an array after all, not a formalized
object with well-defined properties (which I'd prefer, but we can't always
change what someone did 15 years ago just for the sake of changing it).


>
> For everything else you've probably entered an unexpected state and
> should display a warning (that should ideally be picked up by an
> exception handler and convert it to an exception).
>

Not always, and not in cases where people are just coalescing null into
null to avoid a warning.


>
> When handling external data it is usually a good idea to use a
> validation step before using it, such as JSON schema.
>

I don't usually write code dealing with handling external API requests in
my day-to-day work, so I can't really speak to JSON handling, specifically.
I have had to unserialize objects from JSON files from other internal
systems but that was back when there was only the Elvis operator (PHP
5.6ish), so it was used quite liberally.


>
> An even better idea is to parse it into a specific type, rather than
> just validating it, at which point you're going to be using a lot of
> isset, null coalesces, array_key_exists, property_exists etc anyway and
> sucking in some default other than null is almost always the wrong thing
> to do.
>

I'm only suggesting this apply to ternary tests, not in general, so that
code can be easier to read and grok. I believe null is a sensible default
in cases when you are only establishing the truthiness of a value, and
whether or not the key exists is irrelevant (otherwise there's the null
coalesce operator and isset()). There may be times when you need more
validation, but those aren't usually handled in ternaries. In other words,
I believe this should still cause a warning if it is unset: $arr['unset']
?: $arr['unset']; because it is likely a bug and used outside of the
ternary test (the part before the "?" -- I'm not sure what the
technical name is for it). This should not, however: $arr['unset'] ?: 'some
default'; because it is only used in the test.

I also like the suggestion of creating some null-safe operator for arrays,
which might be a more general solution.


>
> As to your final point: PHP internals voted by a supermajority to raise
> this from a notice to a warning in PHP 8, it seems unlikely that 33% of
> people are now going to change their mind and vote for the opposite.
>
>
We've had to live with it for a little while now and we have a pretty good
idea of what it is doing to codebases and the time it takes to "fix"
codebases by null coalescing null to null. I tried searching GitHub for
this, but it looks like the search functionality removes the question marks
in my query.

--00000000000037e27005d90159fb--