Re: [PHP-DEV] Passing null to parameter
> On 10 Nov 2023, at 20:00, Rowan Tommins wrote: > [...] Wherever it is used, "null" is a confusing and often controversial > concept. In different contexts, it is used for different things, and has > different ideal behaviours. It's a whole debate on its own, and bringing in > other types of coercion just confuses the conversation. Thanks for the background detail Rowan, that is useful (I have re-read a few times). I know I won't have a 2/3 support for NULL coercion with function parameters, so I'll just wait until 9.0 is made available, and assuming I'm correct (hopefully I'm not), we can address the BC problems then. For now, I'll just try to remember to do things like `(trim(strval($search)) != '')`, and hope that static analysis tools move their NULL type check down to their lowest level: https://github.com/phpstan/phpstan/issues/10127 Craig -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Passing null to parameter
On 10/11/2023 11:29, Craig Francis wrote: strict_types=1 enables strict type checking While it's not exactly false, this statement is misleading in a really important way. That's not your fault, it's just that "strict_types" is, in hindsight, a really terrible name. If I had a time machine (and a lot of patience to wade back into the debate), I would advocate naming the modes as follows: * declare(scalar_params=coerce) * declare(scalar_params=assert) This would have made several things clearer. 1) Although asserting a type is in a sense "more strict" than coercing it, the "coerce" mode actually rejects more inputs than an explicit cast would. 2) The settings were never intended as a change to "the type system"; they don't change the behaviour of anything other than certain arguments passed into functions. 3) They only change the behaviour of a small number of new type keywords added to the language at that time: "int", "float", "string", and "bool". It governs the way these types interact with each other, and not anything about any other type. Most relevantly for this discussion, that list does not include "null". At the time, the language already had type declarations on parameters for "array" and class/interface names, and those already disallowed null values. Note that this isn't a given, as many languages treat "null" as a valid value for any object type; recent developments in C# suggest PHP got it right for once by disallowing that from the start. I never understood why NULL was considered a complex value like a class/callable/array, when it's more like a simple bool/int/float/string I think you are misreading the quoted passage. It's not saying that null is similar to the complex types, it's saying that the handling of null values should be consistent between parameters marked with those keywords, and parameters marked with the new keywords "int", "float", "string", and "bool". In actual fact, we could probably have narrowed the scope of the setting even further, because there are two cases that most people care about: * coercing int to and from float (which is allowed in both modes anyway) * coercing string to and from int or float (e.g. getting an ID or a price from a URL or API response) So maybe we could have had: * declare(numeric_params=coerce) * declare(numeric_params=assert) That would make it even clearer that the question of whether trim(null) is a meaningful operation is a completely separate debate than anything that the declare() covers. Wherever it is used, "null" is a confusing and often controversial concept. In different contexts, it is used for different things, and has different ideal behaviours. It's a whole debate on its own, and bringing in other types of coercion just confuses the conversation. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Passing null to parameter
On Fri, Nov 10, 2023 at 1:33 PM Craig Francis wrote: > > On 10 Nov 2023, at 12:16, Andreas Leathley wrote: > > Also note that in your example $q could be an array (leading to a fatal > > error in the code)from the request data, which is why checking types > > thoroughly (not just coercing them with strval) can be helpful in avoiding > > unexpected situations and deciding how to handle such situations, instead > > of yolo-ing it. > > Yep, and I completely agree, but I'm not focusing on my code, I'm looking at > the code the vast majority of developers write... and I do not want PHP 9 to > be so difficult to use that people stay on PHP 8 forever. > > Craig > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > Personally, I find the whole strict types + nullable thing really bad. (string) null is "", which is hopefully pretty well known. But with strict types, you may not even know you have a null value because casting is used all over the code base. I constantly have to point out this in code reviews and suggest this: // val is an int|string|null func((string) ($val ?? throw new LogicException("val should not be null"))); which just feels dirty. At least in non-strict type mode, we would get "null catching" for free (at the expense of deterministic coercion), and is one reason we're actually considering to switch back to non-strict types; a very simple coercion system for everything but null. So, really, I think it might be a good idea to rethink the whole type system at some point. There isn't really any ideal solution atm for handling nulls + strict types. Robert Landers Software Engineer Utrecht NL -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Passing null to parameter
On 10 Nov 2023, at 12:16, Andreas Leathley wrote: > Also note that in your example $q could be an array (leading to a fatal error > in the code)from the request data, which is why checking types thoroughly > (not just coercing them with strval) can be helpful in avoiding unexpected > situations and deciding how to handle such situations, instead of yolo-ing it. Yep, and I completely agree, but I'm not focusing on my code, I'm looking at the code the vast majority of developers write... and I do not want PHP 9 to be so difficult to use that people stay on PHP 8 forever. Craig -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Passing null to parameter
On 10 Nov 2023, at 11:47, Alex Wells wrote: > It fails to correctly change the type of the variable to nullable due to the > coalescing operator. This is a bug in PHPStan, Cool, and Kamil notes that bleeding edge has fixed this, that's good... but how many developers do you think are using level 9? it's not picked up at level 8. Craig -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Passing null to parameter
On 10 Nov 2023, at 11:11, Kamil Tekiela wrote: >> That's what I thought, but for some reason, casting null is deprecated only >> when it's provided as an argument to functions, all other cases it's fine... >> weird right? > > > No, passing null to non-nullable parameters is already an error. It has been > for a long while now. The only exception are some built-in functions that > silently ignored that and cast the value to another type. Not sure I'm following, I said "an argument to functions", the other cases I was referring to was string concatenation, == comparisons, arithmetics, sprintf, print, echo, array keys, etc. > However, passing null to non-nullable param was never allowed. For user defined functions... but there are at least 335 parameters negatively affected by this deprecation: https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md And that's ignoring the 104 questionable and 558 problematic parameters where NULL (or empty string) probably shouldn't be allowed (like how $separator in explode() already has a “cannot be empty” Fatal Error). > but as I explained earlier it was always an error. That's how the language > was designed. I'm not sure many people see it like that though, most of the frameworks can return NULL when retrieving GET/POST/DB etc values, and developers simply pass those nullable values to functions like trim(), urlencode(), hash(), base64_encode(), strtoupper(), preg_match(), etc. > Finding it is difficult, I agree, but that's the whole purpose of the > deprecation. It's supposed to help you find all the mistakes. The mistakes > are only with built-in functions, so the scope is very limited. Imagine a developer creating a simple search , so the browser makes a request to "/search/?q=x", and the developer works with the q value, maybe they use `htmlspecialchars`, or `preg_split` to separate the words, etc... during all of their (probably manual) testing, they are always providing a value, as they are using the website as they expect it to be used, but a few months later, when it's in production, a user types in the "/search/" URL, or edits the URL, or the page with the form on failed to load properly, or a browser extension did something to it; today the script would receive a NULL value, and it would be treated like an empty string (as expected and documented), but with PHP 9, this will cause a fatal error? > Casting null to other types is not a mistake, but passing null to functions > that don't expect it, is a mistake. But that's what the type coercion is supposed to do, like how a function that expects an integer will accept the string '5'. > Just think about a simple example and how ambiguous it would be. > > function abc(string|int $p) {} > abc(null); > > What should null be converted into and why? int(0), because union types already define an "order of preference": https://wiki.php.net/rfc/union_types_v2#coercive_typing_mode Like this: ``` function abc(string|int $p) { var_dump($p); // int(0) } abc(false); ``` Craig
Re: [PHP-DEV] Passing null to parameter
On 10.11.23 12:33, Craig Francis wrote: On 10 Nov 2023, at 10:54, Alex Wells wrote: PHPStan does find them:https://phpstan.org/r/38fc1545-2567-49b9-9937-f275dcfff6f5 It does not: https://phpstan.org/r/c533ff42-80e4-4309-9751-1ec79e359946 Psalm does give you a PossiblyInvalidArgument here, PHPStan will likely detect this soon too, as both static analyzers are covering more and more ground. Also note that in your example $q could be an array (leading to a fatal error in the code)from the request data, which is why checking types thoroughly (not just coercing them with strval) can be helpful in avoiding unexpected situations and deciding how to handle such situations, instead of yolo-ing it.
Re: [PHP-DEV] Passing null to parameter
On 10.11.2023 12:11, Kamil Tekiela wrote: function abc(string|int $p) {} abc(null); What should null be converted into and why? If you as a developer know then you can cast the argument to an appropriate type but on the language level, it's not really possible. So, in conclusion, we are not supporting you because we are stubborn or that we want to force strict types, but because resolving this problem in a way you want is extremely difficult and borderline impossible. Then throw an error when it is not possible, but in most cases it will be, I assume. Why strtoupper(0) works but strtoupper(null) throws a warning? The argument is defined as string, so the former should be a warning too, if we were to be consistent. It's a rethorical question, I know arguments of both sides. As OP, I think that this behavior should be controlled by strict types. I.e. in strict_types=1 throw an error, in strict_types=0 use null coercion if possible (w/o warnings). Of course, the inconsistency/confusion can be fixed in different ways, but what's proposed here is best for BC reasons, imo. Other options would make the language more strict. That being said, I'm also aware that currently, most likely, none of such proposals would get 2/3 of votes. -- Aleksander Machniak Kolab Groupware Developer[https://kolab.org] Roundcube Webmail Developer [https://roundcube.net] PGP: 19359DC1 # Blog: https://kolabian.wordpress.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Passing null to parameter
You must enable bleeding edge.
Re: [PHP-DEV] Passing null to parameter
On Fri, Nov 10, 2023 at 1:33 PM Craig Francis wrote: > On 10 Nov 2023, at 10:54, Alex Wells wrote: > > PHPStan does find them: > https://phpstan.org/r/38fc1545-2567-49b9-9937-f275dcfff6f5 > > > > It does not: > > https://phpstan.org/r/c533ff42-80e4-4309-9751-1ec79e359946 > > It fails to correctly change the type of the variable to nullable due to the coalescing operator. This is a bug in PHPStan, but if you manually specify the type of that variable as nullable, it starts to correctly report an error on line 5. So it's not that PHPStan can't find places where a null is passed into a non-nullable parameter, but rather PHPStan failing to infer the variable type correctly, thinking it's actually not nullable.
Re: [PHP-DEV] Passing null to parameter
On 10 Nov 2023, at 10:54, Alex Wells wrote: > PHPStan does find them: > https://phpstan.org/r/38fc1545-2567-49b9-9937-f275dcfff6f5 It does not: https://phpstan.org/r/c533ff42-80e4-4309-9751-1ec79e359946
Re: [PHP-DEV] Passing null to parameter
First, thanks Rowan (same to you Kamil), I do appreciate your thoughts on this... On 9 Nov 2023, at 20:01, Rowan Tommins wrote: > On 09/11/2023 14:58, Craig Francis wrote: >> We might as well make the PHP 9 upgrade as hard as possible, just to force a >> little bit of `strict_types=1` on everyone. > > > Just to be clear, strict_types has nothing to do with this; strict_types=1 enables strict type checking, which adds fatal type errors instead of coercion (all fine, all good); but for everyone not using strict_types=1, there is a fatal type error when NULL is passed to a function argument, while all other type coercions (e.g. string '5' to int) work? > changing it does not allow you to pass nulls to typed parameters, and never > did: https://3v4l.org/atT0B Yep, specifically for user defined function parameters, but NULL coercion works with string concatenation, == comparisons, arithmetics, sprintf, print, echo, array keys? That said, with the original RFC: https://wiki.php.net/rfc/scalar_type_hints_v5 > "The only exception to this is the handling of NULL: in order to be > consistent with our existing type declarations for classes, callables and > arrays, NULL is not accepted by default" I never understood why NULL was considered a complex value like a class/callable/array, when it's more like a simple bool/int/float/string, where NULL can be coerced (and NULL is documented as being coercible in all other contexts, like concatenation). Also... > “it should be possible for existing userland libraries to add scalar type > declarations without breaking compatibility” But that's not the case; if you add types to a legacy projects functions, fatal type errors happen instead of accepting NULL and coercing it as needed (it's why I never bothered adding types to the legacy project I still look after). > Nor has strict_types=0 ever been aligned to the loosest coercion rules used > in other contexts; for instance, an empty string was never an acceptable > input for an integer parameter, even in versions where it was an acceptable > operand for addition: https://3v4l.org/khD32 Fair, but that makes a little bit more sense to me (although I'd assume an empty string would be coerced to 0). > As for your previous example: > > > redirect('/thank-you/?ref=' . urlencode($ref)); > > If $ref isn't set, any of these might be the correct URL: "/thank-you/", > "/thank-you/?ref=", "/thank-you/?ref=default", ... The language can only > guess one of those. NULL has always been coerced to an empty string with urlencode(), it happens in a lot of projects, and everyone seemed to be fine with it? > A similar example I've come across is in manually escaped SQL (yes, I know, > use parameters instead...): > > $sql = "Insert Into blah ( whatever ) Values ( '" . sql_escape($someVar) . "' > )"; > > Nine times out of ten, if the PHP variable is null, you want an SQL null, not > ''; but if the [imaginary] sql_escape function doesn't reject nulls, you may > not notice the bug until you've ended up with garbage in your DB. Fortunately I only have to maintain 1 legacy project, and I've only had to make 29 edits so far, but every single one involved me adding strval(), it's certainly not making the code better, and I know there are still more to find, but I'll have to wait for customers to trip them. Craig
Re: [PHP-DEV] Passing null to parameter
Hi Craig, Don't get me wrong, but I have a feeling you still didn't understand fully my first message. That's what I thought, but for some reason, casting null is deprecated only > when it's provided as an argument to functions, all other cases it's > fine... weird right? > No, passing null to non-nullable parameters is already an error. It has been for a long while now. The only exception are some built-in functions that silently ignored that and cast the value to another type. > > Instead, if you want, submit a proposal to reverse the direction. > > No point, people will vote against, and will just entrench their position > (people like to think they are right, this would involve them changing > their mind; and because everyone voting is almost certainly using > strict_types=1, it won't affect them, and they will see it as a way of > pushing "lesser developers" to using strict types). > I don't use strict types and I am pretty sure there are many more people that don't. I started using them more recently but I don't use them everywhere yet. However this discussion doesn't have much to do with strict types. > Such a change could also be complicated to implement given how ambiguous > it would be. > > It always worked that way (for internal functions) - for arguments that > accept a string, cast null to an empty string; if it accepts an integer > then it's 0, etc. > It only worked like that for some built-in functions. That's because built-in functions didn't (and some still don't) respect the PHP type system. However, passing null to non-nullable param was never allowed. > So unless it's causing some serious problems for PHP developers or users, > I think it's too late to change the design of the language. > > At the moment it's causing deprecation log entries (which are ignored), > but the problems are everywhere, like the following (Laravel), is this > really that unreasonable? > > ``` > $a = trim($request->input('a')); > ``` > > This will start getting Fatal Type Errors in 9.0... and finding these > "mistakes" is close to impossible (phpstan doesn't find them; psalm > requires high levels 1-3). > It's only throwing a deprecation because we started to fix the built-in functions so they behave like the rest of the language. That's a bug fix, but as I explained earlier it was always an error. That's how the language was designed. Finding it is difficult, I agree, but that's the whole purpose of the deprecation. It's supposed to help you find all the mistakes. The mistakes are only with built-in functions, so the scope is very limited. With all other functions the error is caught during development because of the error. Casting null to other types is not a mistake, but passing null to functions that don't expect it, is a mistake. There's no defined behavior in PHP as to what should happen. Allowing passing null to non-nullable parameters is a big feature request that never was part of PHP. It's not something that was removed, it's something that never existed. Just think about a simple example and how ambiguous it would be. function abc(string|int $p) {} abc(null); What should null be converted into and why? If you as a developer know then you can cast the argument to an appropriate type but on the language level, it's not really possible. So, in conclusion, we are not supporting you because we are stubborn or that we want to force strict types, but because resolving this problem in a way you want is extremely difficult and borderline impossible. Regards, Kamil >
Re: [PHP-DEV] Passing null to parameter
On Fri, Nov 10, 2023 at 12:47 PM Craig Francis wrote: > This will start getting Fatal Type Errors in 9.0... and finding these > "mistakes" is close to impossible (phpstan doesn't find them; psalm > requires high levels 1-3) > PHPStan does find them: https://phpstan.org/r/38fc1545-2567-49b9-9937-f275dcfff6f5 Finding these mistakes is easy if you either have tests or use static analysis tools.
Re: [PHP-DEV] Passing null to parameter
On 9 Nov 2023, at 16:08, Kamil Tekiela wrote: > Automatic casting of null to other types is a handy feature and deprecating > it brings no benefit to anyone. That's what I thought, but for some reason, casting null is deprecated only when it's provided as an argument to functions, all other cases it's fine... weird right? > Instead, if you want, submit a proposal to reverse the direction. No point, people will vote against, and will just entrench their position (people like to think they are right, this would involve them changing their mind; and because everyone voting is almost certainly using strict_types=1, it won't affect them, and they will see it as a way of pushing "lesser developers" to using strict types). > This should only apply in loose-type mode. Yep, of course. > and it would make nullable types useless. I don't think so, nullable would allow the NULL value, and the non-nullable would be coerced, just like the other types: ``` function example(string $b, ?string $c, int|float $a) { var_dump($b); // string(0) "" var_dump($c); // NULL var_dump($a); // int(5) } example(NULL, NULL, '5'); ``` > Such a change could also be complicated to implement given how ambiguous it > would be. It always worked that way (for internal functions) - for arguments that accept a string, cast null to an empty string; if it accepts an integer then it's 0, etc. > But if you can outline tangible benefits and write a PR to show how it would > work, I think the community would review the RFC. I can't be trusted to write C, so can't do a PR (other than to revert the last commit, which isn't quite right)... I did draft an RFC, but I just got complaints: https://wiki.php.net/rfc/null_coercion_consistency > As for the negative responses, it's probably because it's been many PHP > versions since the nullable types were introduced. People accepted it and are > generally satisfied with how it works. People here are likely to be using `strict_types=1`, but the majority of developers use PHP as a simple scripting language, and don't see the benefit to strict type checks (just to note, I do, but I'm not talking about me, I'm looking at the amount of code that will not be able to upgrade to PHP 9). > So unless it's causing some serious problems for PHP developers or users, I > think it's too late to change the design of the language. At the moment it's causing deprecation log entries (which are ignored), but the problems are everywhere, like the following (Laravel), is this really that unreasonable? ``` $a = trim($request->input('a')); ``` This will start getting Fatal Type Errors in 9.0... and finding these "mistakes" is close to impossible (phpstan doesn't find them; psalm requires high levels 1-3). Craig -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Passing null to parameter
On 29 Oct 2023, at 11:14, Kamil Tekiela wrote: > A code like this already throws a fatal error. > > function enc(string $a){} > enc(null); > > The only thing remaining to be fixed in PHP 9 is to make this error > consistent on all function invocations. Or, be consistent with all of the other type coercions? ``` function user_function(string $s, int $i, float $f, bool $b) { var_dump($s, $i, $f, $b); } user_function('1', '1', '1', '1'); // string(1) "1" / int(1) / float(1) / bool(true) user_function(2, 2, 2, 2); // string(1) "2" / int(2) / float(2) / bool(true) user_function(3.3, 3.3, 3.3, 3.3); // string(3) "3.3" / int(3), lost precision / float(3.3) / bool(true) user_function(false, false, false, false); // string(0) "" / int(0) / float(0) / bool(false) ``` The documentation does clearly say how NULL should be coerced: https://www.php.net/manual/en/language.types.string.php "null is always converted to an empty string." https://www.php.net/manual/en/language.types.integer.php "null is always converted to zero (0)." https://www.php.net/manual/en/language.types.float.php "For values of other types, the conversion is performed by converting the value to int first and then to float" https://www.php.net/manual/en/language.types.boolean.php "When converting to bool, the following values are considered false [...] the special type NULL" Maybe documentation should be amended to say "except when being passed to a function"? Or, should we have fatal errors with the following as well: ``` $nullable = NULL; print($nullable); echo $nullable; printf('%s', $nullable); var_dump('A' . $nullable); var_dump(3 + $nullable); var_dump($nullable / 6); var_dump('' == $nullable); ``` > Without it, you would never know that you have a logical error in your code. But it's not an error? Craig
Re: [PHP-DEV] Passing null to parameter
A code like this already throws a fatal error. function enc(string $a){} enc(null); The only thing remaining to be fixed in PHP 9 is to make this error consistent on all function invocations. > I didn't realise the payment gateway doesn't always provide the order reference Well, there you go. This deprecation message actually informed you of something. Without it, you would never know that you have a logical error in your code. It works as intended. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php