Hi Internals,
I'm getting really annoyed at the "Passing null to parameter" problem, and
it happens with new code as well.
I know you have distain for websites that don't use strict types, or static
analysis at the strictest level, but yesterday I was working on a 15 year
old website, it still works well, it uses PHP as a simple scripting
language... because the payment gateway returns a user to the websites
thank-you page with a POST request, and customers leave this page open,
browsers complain (asking to re-submit data), so I just needed to change it
to a GET request, with the order reference in the URL, so I stupidly added
this:
if (($_SERVER['REQUEST_METHOD'] ?? '') == 'POST') {
redirect('/thank-you/?ref=' . urlencode($ref));
}
I didn't realise the payment gateway doesn't always provide the order
reference, so the function to get the $ref from the POST request returns
NULL... at the moment that's just an annoying deprecation, but does it
really need to become a fatal error in PHP 9?
Should I be using Rector to update this to urlencode((string) $ref)?
Keep in mind, we can still compare NULL to an empty string, concatenate
NULL onto a string, add NULL to an integer/float, and echo/print/sprintf
NULL, we can even use NULL for an array key (as in, it get coerced to an
empty string)... and type coercion happens in other contexts as well, e.g.
passing the string "5" to an integer argument... but NULL will be special,
and cause a fatal type error? really?
Craig
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.
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
I'm getting really annoyed at the "Passing null to parameter" problem,
As I'm getting the usual negative response (-3), should I write an RFC to deprecate NULL coercion in all contexts?
At the moment it's weird that print($nullable) isn't deprecated.
And it's weird that I can do htmlspecialchars('Hi ' . $nullable) but not 'Hi ' . htmlspecialchars($nullable).
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.
Craig
I don't think that would actually be wise. Automatic casting of null to
other types is a handy feature and deprecating it brings no benefit to
anyone.
Instead, if you want, submit a proposal to reverse the direction. Make it
so that null is cast to an appropriate type in params and return values.
This should only apply in loose-type mode and it would make nullable types
useless. Such a change could also be complicated to implement given how
ambiguous it would be. 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.
But as I expressed before, the current path seems to be the most logical.
Despite the decision to introduce nullable types in PHP 7, which as you
pointed out creates an incompatibility with all other type juggling rules,
this is the cleanest and least ambiguous solution.
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. Only after the last bit was
decided to be fixed in PHP 9.0, you started a discussion pointing out that
on paper it's misaligned with other parts of the language. Just because
null is converted to string in one context and throws an error in another
is not necessarily a bad thing. 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.
Regards,
Kamil
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
On Fri, Nov 10, 2023 at 12:47 PM Craig Francis craig@craigfrancis.co.uk
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.
PHPStan does find them: https://phpstan.org/r/38fc1545-2567-49b9-9937-f275dcfff6f5
It does not:
On Fri, Nov 10, 2023 at 1:33 PM Craig Francis craig@craigfrancis.co.uk
wrote:
PHPStan does find them:
https://phpstan.org/r/38fc1545-2567-49b9-9937-f275dcfff6f5It does not:
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.
You must enable bleeding edge.
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
PHPStan does find them:https://phpstan.org/r/38fc1545-2567-49b9-9937-f275dcfff6f5
It does not:
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.
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
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
--
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
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
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
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 <form>, 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
We might as well make the PHP 9 upgrade as hard as possible, just to force a little bit of
strict_types=1on everyone.
Just to be clear, strict_types has nothing to do with this; changing it
does not allow you to pass nulls to typed parameters, and never did:
https://3v4l.org/atT0B
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
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.
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.
Regards,
--
Rowan Tommins
[IMSoP]
First, thanks Rowan (same to you Kamil), I do appreciate your thoughts on this...
We might as well make the PHP 9 upgrade as hard as possible, just to force a little bit of
strict_types=1on 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,
NULLis 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
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.
-
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. -
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. -
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
NULLwas 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]
[...] 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