Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117784
Message-ID: <bc7aebf4-e909-d4cc-ff2e-44f8524a012b@gmx.net>
Date: Tue, 24 May 2022 16:05:42 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Content-Language: en-US
To: Dan Ackroyd <Danack@basereality.com>
Cc: internals@lists.php.net
References: <eb2bef11-4027-8555-93e3-89c2cc4f5a18@gmx.net>
 <CA+kxMuSSMTs-S0m1D3LnGWxvh4WwVPS595Aqr-UcnNxUoF+6Ew@mail.gmail.com>
In-Reply-To: <CA+kxMuSSMTs-S0m1D3LnGWxvh4WwVPS595Aqr-UcnNxUoF+6Ew@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [Discussion] Stricter implicit boolean coercions
From: a.leathley@gmx.net (Andreas Leathley)

On 24.05.22 15:33, Dan Ackroyd wrote:
> "When discussion ends, and a minimum period of two weeks has passed"
> Fyi, the two weeks is a minimum, and almost certainly not enough time
> for subtle BC breaking RFCs like this.

I explicitely stated that to make it clear that this should be
considered a discussion period. I am not trying to circumvent anything,
just make it clear that now is the time to discuss this. And I am ready
to fully discuss this, as hopefully I have shown so far.

I am also not sure what you mean with "subtle BC breaking RFCs". How is
this RFC subtly BC breaking? It introduces a deprecation notice, that is
it - for one that is not subtle, it is explicit, and it does not break
BC except for introducing said notice. Is that not the very minimal BC
break you can achieve to highlight a possible problem in code?


> I think you should really explicitly list what the changes are in a
> single table rather than in sentences.

What do you think is unclear, and what would you include in such a
table? In the Proposal section I listed all values which are considered
allowed, and I tried to include more examples there too to highlight the
outcome. The behavior is not changed by this RFC.

>
>> Would you want to know when a value like -375, =E2=80=9Cfalse=E2=80=9D =
or NaN
>> is given to a typed boolean (and coerced to true) in a codebase?
> Yes.
>
> Which is why I always use both PHPStan and Psalm to detect those types
> of things where those types of bugs are important.
>
> Also, strict mode is great.

Even when using PHPStan and Psalm you can encounter such a value, for
example coming from a form, or an API. Using strict mode is a
possibility, but also a big hammer - so how do you coerce a value coming
from a form in strict mode? With an explicit coercion like (bool) or
boolval()? Because there you might also be losing information
unexpectedly - I do have some ideas to enable implicit coercions in
strict mode, because you might not want to convert "failed" from an API
to true by using explicit coercions, in my applications I would prefer
to at least notice or even throw an exception in such cases, and the
programming language streamlining this could be helpful.

>
>> In one application recently I actually had the string "false" (coming
>> from a form transmission) being passed to a boolean argument and leadin=
g
>> to true, which definitely was unintended.
> But "false" is a perfectly sensible thing to pass as a string in an
> API (as HTTP is a string based protocol). As is 'faux' if your API is
> used by French speaking programmers.
>
> You need an layer of code that converts from strings to the precise
> types your API needs, rather than just passing values straight
> through.

That sounds good in theory, but it is the exact thing that is so hard in
practice, when dealing with forms, APIs, different programming
languages, different human languages, different input formats, changing
code, etc. I am not against writing great code and checking all values
all the time, but I do think this is not the reality.

> Although this change would probably detect some bugs, it's not at all
> obvious that the amount of pain would be worth it.
>
> A lot of applications out there aren't being constantly developed.
> Instead they are in maintenance mode, where there isn't a programmer
> dedicated to constantly work on it. There would be lots of function
> calls to check, and some of them would need code to be modified to
> maintain the existing behaviour. And all of that would seem to be
> subtle work, prone to mistakes.

Earlier you argued that there is no need to detect -375, "false" or NaN
being coerced to a boolean type, because one can use strict mode and
static analyzers, now you argue that legacy applications would have too
much work generated by this RFC. But I don't quite understand why: This
RFC introduces deprecation notices, not TypeErrors. It does not force
legacy applications to change anything, it just points out boolean
coercions that seem dodgy. If these deprecation notices provide little
value in the upcoming years, there is no need to promote them to a
TypeError, they could even be removed again - but I do think they will
provide value and often point out bugs, so much so that a TypeError will
seem reasonable at some point in the future, but there really is no
hurry in escalating it. For now just being able to notice these boolean
coercions would make a huge difference.