Remove array_get

[jchaffraix]

The function cannot be properly typed as it
is and fixing this would take a while, removing
it is shorter and better.

[cpeel]

This is a lot and I will need to work my way through it.

I'm not against this change, but I do want to ask: does doing this help PHPStan check our code better than using array_get() without typing? I want to make sure there's some value-add and that we're not doing it just to do it. My comments on the array_get() function assume the answer is "yes, there is value".

[cpeel]

We should not remove this function yet because it's used by code that isn't checked into git (ie: noncvs code) and perhaps others who use our code.

Instead, we should add typing for the bits we know (array $arr), set it as @deprecated, and do a trigger_error("array_get() is deprecated, use the null coalescing operator instead", E_USER_DEPRECATED);.

[jchaffraix]

This is a lot and I will need to work my way through it.

I'm not against this change, but I do want to ask: does doing this help PHPStan check our code better than using array_get() without typing? I want to make sure there's some value-add and that we're not doing it just to do it. My comments on the array_get() function assume the answer is "yes, there is value".

Yes, there is value as PHPStan can infer the narrow type of the return/args when using ?? but won't with array_get. Even if we add types, those will have to be the super-types (array for the arg and mixed for the return).

Let's take those 2 equivalent statements: $test_var = array_get($_POST, 'test', null) and $test_var = _$POST['test'] ?? null.

In the first case, PHPStan doesn't know the return of array_get so it can't do any kind of validation on the usage of $test_var. If we added mixed as the return, PHPStan will only do minimal checks (mixed is a super-type for all types) and will have to ask us to cast the narrow type when needed (e.g. when doing a function call).
In the second case, PHPStan can infer that the variable is of type ?string and warn/error accordingly.

[cpeel]

Let's take those 2 equivalent statements: $test_var = array_get($_POST, 'test', null) and $test_var = _$POST['test'] ?? null.

In the first case, PHPStan doesn't know the return of array_get so it can't do any kind of validation on the usage of $test_var. If we added mixed as the return, PHPStan will only do minimal checks (mixed is a super-type for all types) and will have to ask us to cast the narrow type when needed (e.g. when doing a function call). In the second case, PHPStan can infer that the variable is of type ?string and warn/error accordingly.

$_POST by definition only contains strings so let's broaden it up a bit. If I have $test_var = $some_random_array[$some_var] ?? 42 is PHPStan able to infer the value type of that element of that array from other code introspection? Because if not it's no more useful than array_get($some_random_array, $some_var, 42) and that's what I'm trying to tease apart.

[jchaffraix]

Let's take those 2 equivalent statements: $test_var = array_get($_POST, 'test', null) and $test_var = _$POST['test'] ?? null.
In the first case, PHPStan doesn't know the return of array_get so it can't do any kind of validation on the usage of $test_var. If we added mixed as the return, PHPStan will only do minimal checks (mixed is a super-type for all types) and will have to ask us to cast the narrow type when needed (e.g. when doing a function call). In the second case, PHPStan can infer that the variable is of type ?string and warn/error accordingly.

$_POST by definition only contains strings so let's broaden it up a bit. If I have $test_var = $some_random_array[$some_var] ?? 42 is PHPStan able to infer the value type of that element of that array from other code introspection? Because if not it's no more useful than array_get($some_random_array, $some_var, 42) and that's what I'm trying to tease apart.

Yes, it should be able to with a caveat that it needs to know the narrow type of $some_random_array somehow. Unfortunately I think those will have to come through PHPStan annotations (like @param array<string, int|string> or @return array<int, ?Round>).

[cpeel]

@jchaffraix - are you intending on splitting this up into pieces to review separately or would you like help to do that? There's no rush, I just realized it wasn't clear who was taking point on next steps.