Re: [PHP-DEV] Passing null to parameter

2023-11-14 Thread Craig Francis
> 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

2023-11-10 Thread Rowan Tommins

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

2023-11-10 Thread Robert Landers
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

2023-11-10 Thread Craig Francis
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

2023-11-10 Thread Craig Francis
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

2023-11-10 Thread Craig Francis
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

2023-11-10 Thread Andreas Leathley

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

2023-11-10 Thread Aleksander Machniak

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

2023-11-10 Thread Kamil Tekiela
You must enable bleeding edge.


Re: [PHP-DEV] Passing null to parameter

2023-11-10 Thread Alex Wells
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

2023-11-10 Thread Craig Francis
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

2023-11-10 Thread Craig Francis
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

2023-11-10 Thread Kamil Tekiela
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

2023-11-10 Thread Alex Wells
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

2023-11-10 Thread Craig Francis
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

2023-10-29 Thread Craig Francis
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

2023-10-29 Thread Kamil Tekiela
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