On Wed, 20 Apr 2022 at 20:02, Andreas Leathley <a.leath...@gmx.net> wrote:
> I don't get why you would add strval everywhere. Why are you getting null > everywhere? As to adding `strval($var)`, or `(string) $var`, or `$var ?? ""` everywhere... that's because we (or frameworks) cannot simply change the defaults for these "Common sources of NULL", nor can everyone simply add strval() around these sources, because NULL can still be useful, with possible NULL checks later (e.g. if you need to distinguish between a missing value and an empty string)... the only safe/easy way to do it is at the sinks, and there are lots of them. Unfortunately we cannot magically update all of the existing code in the world... these changes have to be done manually (note the lack of tooling), and the time taken should be justified (like how the removal of magic quotes was justified). > In most of the examples in your RFC "null" is specifically chosen as a > default value and could easily be an empty string (why do something like > "$_POST['value'] ?? null" if you actually want an empty string?). You could, but these are common and useful patterns/sources... and most importantly, NULL has been the default for years. For the framework examples, the second argument of these functions is the > default value - it does not have to be null. > Yes, but we are talking about backwards compatibility... and while most developers could always specify the default (second argument) to be an empty string, that's not always appropriate (frameworks default to NULL so developers can tell when a value has not been provided, which can be useful in some cases). > And "filter_input" I have not seen in a codebase yet, probably because of > its weird double function as retrieving an input variable and also > filtering it, with both null, false and the result as a possible return > value. > Sorry, but it is used, and it can return NULL. The few cases where I encountered the deprecation notice it was always a > mistake and easy to fix, and I was glad that was pointed out to me. > Lucky you... and as noted with Rowan, getting to look at your code again can be useful. I also note that "making each change is fairly easy", but you will have much better luck with static analysis (where variables are reliably checked if they can be nullable). But the important bit is that these problems "are difficult to find" and "there are many of them". If you want a very simple public example: https://github.com/PHPCompatibility/PHPCompatibility/commit/ab7b0b82d95c82e62f9cffb0f1960f3ccbf0805e Or, if you want to do some digging: https://twitter.com/mwop/status/1441044164880355342 There is another 3.5 years until PHP 9 is likely to come out, which is a > lot of time for people to adjust their codebase. I could even see an > argument for not promoting it to a fatal error in 9.0 if so many people > need more time. If it's deprecated, that is an intent to break... and if no other solutions present themselves, and the vote for this RFC fails... why would it be delayed? It will then be clear the Internals developers want this (they would have made an informed choice, and put their name to it). Removing the deprecation and changing fundamental aspects about the type > system in PHP by auto-coercing null just so people do not need to update > their codebase seems like the worst possible way forward. > Many times NULL coercion is fine, like how the integer 5 can be coerced to the string "5"... and it's worth remembering this is how it's always worked (when not using `strict_types=1`). So far, your RFC does not mention the arguments made against your proposed > changes in an understandable way. Are you going to suggest any improvements? what have I missed? I'm trying to keep it short, because I know long RFC's can be a problem. George Peter Banyard wrote some extensive arguments and you have only > included one sentence without any of his arguments and try to refute it in > the very next sentence as not really an argument, The "one sentence" you refer to, I assume that's "Userland scalar types [...] did not include coercion from NULL for very good reasons"? Unfortunately the email does not say what these "good reasons" are, so I could only go with the Scalar Type Declarations RFC, where I summarised the only paragraph that talks about NULL (in regards to weak type checks). I think “to be consistent with our existing type declarations” is a pretty good quote summary of that paragraph, and I think it's fair for me to add "no further details given". The comment that NULL was "never accepted for scalar type declarations" has already been discussed, and does not add anything extra. https://wiki.php.net/rfc/scalar_type_hints_v5#behaviour_of_weak_type_checks As to the 2 emails from George... I read them very carefully, several times, I have covered those details in my RFC, and I did reply... but just to be absolutely sure I haven't missed anything: https://externals.io/message/117501#117515 1) The Type Error when coercing NULL seeming to be an oversight, I replied with an explanation, and tweaked the wording of that paragraph: https://wiki.php.net/rfc/null_coercion_consistency?do=diff&rev2%5B0%5D=1649688632&rev2%5B1%5D=1649694771&difftype=sidebyside 2) Java's "nullable by default" is one way NULL is used, I don't think this adds anything new. 3) The RFC from Zend, I covered in my reply, specifically the second rule "If the value can be coerced", which NULL can be (I will note that some developers see NULL as an invalid value, but that's already mentioned in my RFC). 4) "amending functions to have nullable arguments", it was the basis of my previous RFC, I've noted in this in this RFC, and in a few of my replies. As an aside, no one seems to agree that htmlspecialchars() should explicitly accept NULL (due to the reason noted in the second paragraph), let alone the other ~334 parameters. 5) "The breaking change was justified and explained", I've covered in my RFC and reply, the justification was about consistency between internal and external functions (which I talk about in my RFC, and agree with in spirit), and the impact was not explained (ref "the lack of discussion of the impact", and the single email from Craig Duncan). 6) "the breaking change has also been announced with ample time to fix the issues", see the "Temporary Solutions" section in my RFC, on how this is being ignored, and the complete lack of tooling to make these changes. https://externals.io/message/117501#117523 1) "strict_types were a mistake", yep, that's fine, I just don't think it adds anything to my RFC. 2) "making coercive typing mode more sensible", going through the quoted RFC from Girgias, that seems to be fine, and I agree with. 3) "Userland scalar types [...] did not include coercion from NULL for very good reasons.", as noted above, I added this to the Open Issues on the 15th April. 4) "general consensus that internals and userland should behave identically", I agree with in spirit, and has been included since my very first draft - where I said we must "keep user-defined and internal functions consistent". 5) "align internal behaviour with the accepted better design choice of userland", I'm not sure I can quote this, as "better" needs to be described and explained. In short, it's not adding anything that hasn't already been noted above. I don't think the rest of the second email raises any other points that are relevant to my RFC (it's more about how PHP is developed). I am still bothered by the language of the RFC in general where you often > write things like ".. it hasn't been the case for everyone else", "most > developers are not in a position" and so on. Assuming you know what > everyone is doing and how everyone is doing it in an RFC does not seem > constructive. All the numbers you cite are also circumstantial and not > about the supposed problem discussed in the RFC. I am human, and I wrote it in a way that I hope is understandable. That said, I'm getting the feeling you are the one intentionally quoting "out of context" (1st quote can only apply to those using `strict_types=1`, so by definition "it hasn't been the case for everyone else"; 2nd is about the problem of how "most developers are not in a position" to use "very strict Static Analysis", which I back up with the low percentage usage of `strict_types=1` and the quoted JetBrains developer survey). Admittedly I've only got the 2 stats in my RFC intro, but they are relevant, and I did say to you on 14 Apr 2022 "I would like to include more stats (maybe WordPress install numbers?), but I couldn't think of any which were easy to source (ref reliability), or be that useful to the discussion" ... do you have anything to add here? > - for example, you assume people not using static analysis are in favor of > your RFC, even though this is pure speculation. What? Where? Compared to all the other RFCs in the last 3 years I read through I don't > only disagree with this RFC, I also think it is not very well reasoned > about and does not convey the discussions that were held about the topic on > this mailing list. It mainly contains your opinion, which seems > insufficient for an RFC. I have spent considerable amount of time replying to your points, and as noted above, I have included peoples comments in my RFC. It is also my second attempt at solving the problem due to feedback. If you can give me something/anything to contribute to the RFC, I will happily add/update it; or if you can provide any other solutions to this problem (please don't pretend it doesn't exist), then I will happily discuss or even recommend them. Craig
