On Sun, Jan 19, 2020 at 4:53 PM Mike Schinkel <m...@newclarity.net> wrote:
> Also, Rowan Collins mentioned that checks in Go can be disabled for
> runtime checking; maybe we could support an option that disables said
> checking so that production sites could run w/o checks but we could run
> checks in development, testing and staging. We could also have an option to
> disable checking of array types above a given size of array, maybe
> defaulting to 1024? Clearly both of these would be no worse than what we
> have today.
>
You are getting into static analysis territory here with that. There are
already static analysis tools that do exactly this type of array type
checking during development. For example, there are three type mistakes in
this code:
1 <?php
2 class C {
3 /**
4 * @param int[] $ints
5 * @param string[] $strings
6 * @return array<int,string>
7 */
8 static function f(array $ints, array $strings):array {
9 return array_combine($strings, $ints);
10 }
11 }
12 print_r(C::f([3,2,'1'], ['abc', 'def', 42]));
Running Phan on it produces:
array.php:9 PhanTypeMismatchReturn Returning type array<string,int> but f()
is declared to return array<int,string>
array.php:12 PhanTypeMismatchArgument Argument 1 ($ints) is
array{0:3,1:2,2:'1'} but \C::f() takes int[] defined at array.php:8
array.php:12 PhanTypeMismatchArgument Argument 2 ($strings) is
array{0:'abc',1:'def',2:42} but \C::f() takes string[] defined at
array.php:8
The code itself would run in production without errors, of course, and
would produce:
Array
(
[abc] => 3
[def] => 2
[42] => 1
)
But at Etsy, at least, this code would never make it to production because
static analysis checks are run by all developers and also run automatically
during staging prior to a production push.
Really expensive checks like this belong at the static analysis stage. And
yes, it would be amazing to have a static analyzer built into PHP, which is
basically what you are asking for here, but that is a huge task and goes
way beyond just this particular check.
-Rasmus
