FunctionLikeAnalyzer: params & param_lookup are not always symetrical

[caugner]

I stumbled upon the following error in the context of psalm-plugin-laravel development (see psalm/psalm-plugin-laravel#160):

In FunctionLikeAnalyzer.php line 819:
                                                                                                                                                                                                         
  Uncaught ErrorException: Undefined array key 2 in /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php:819                          
  Stack trace:                                                                                                                                                                                           
  #0 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php(819): Laravel\Lumen\Application->Laravel\Lumen\Concerns\{closure}()         
  #1 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php(607): Psalm\Internal\Analyzer\FunctionLikeAnalyzer->checkParamReferences()  
  #2 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClassAnalyzer.php(1779): Psalm\Internal\Analyzer\FunctionLikeAnalyzer->analyze()                     
  #3 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClassAnalyzer.php(1447): Psalm\Internal\Analyzer\ClassAnalyzer->analyzeClassMethod()                 
  #4 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ClassAnalyzer.php(416): Psalm\Internal\Analyzer\ClassAnalyzer->analyzeTraitUse()                     
  #5 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/FileAnalyzer.php(214): Psalm\Internal\Analyzer\ClassAnalyzer->analyze()                              
  #6 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(348): Psalm\Internal\Analyzer\FileAnalyzer->analyze()                                   
  #7 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(609): Psalm\Internal\Codebase\Analyzer->Psalm\Internal\Codebase\{closure}()             
  #8 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/Analyzer.php(277): Psalm\Internal\Codebase\Analyzer->doAnalysis()                                    
  #9 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php(642): Psalm\Internal\Codebase\Analyzer->analyzeFiles()                           
  #10 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/src/Psalm/Internal/Cli/Psalm.php(361): Psalm\Internal\Analyzer\ProjectAnalyzer->check()                                         
  #11 /path/to/psalm-plugin-laravel/vendor/vimeo/psalm/psalm(4): Psalm\Internal\Cli\Psalm::run()                                                                                       
  #12 {main}                                                                                                                                                                                             
    thrown                                                                                                                                                                                                                                                                                                                                                                                  

This is the corresponding code location:

psalm/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php

Lines 814 to 822 in bee4e83

	$position = array_search(substr($var_name, 1), array_keys($storage->param_lookup), true);
	if ($position === false) {
	throw new \UnexpectedValueException('$position should not be false here');
	}
	if ($storage->params[$position]->by_ref) {
	continue;
	}

Here's what the variable state looked like at the crash:

$var_name = '$state';
$storage->param_lookup = ['parameters', 'count', state']
$position = 2
$storage->params = [
  0 => ['name' => 'count', /* ... */],
  1 => ['name' => 'state', /* ... */],
]

So basically param_lookup considered an additional parameter, coming from the parent implementation, while params only had the current implementation's parameters.

The causing code was this stub for Laravel:

<?php

namespace Illuminate\Database\Eloquent\Factories;

trait HasFactory
{
    /**
     * @template TCountOrState of callable|array|int|null
     *
     * @param  TCountOrState  $count
     * @param  callable|array  $state
     *
     * @return (
        TCountOrState is int
            ? FactoryBuilder<static, TCountOrState>
            : FactoryBuilder<static, 1>
        )
     */
    public static function factory($count = null, $state = [])
    {

    }
}

The real HasFactory trait in Laravel 8 looks like this though:

<?php

namespace Illuminate\Database\Eloquent\Factories;

trait HasFactory
{
    /**
     * Get a new factory instance for the model.
     *
     * @param  mixed  $parameters
     * @return \Illuminate\Database\Eloquent\Factories\Factory
     */
    public static function factory(...$parameters)
    {
        $factory = static::newFactory() ?: Factory::factoryForModel(get_called_class());

        return $factory
                    ->count(is_numeric($parameters[0] ?? null) ? $parameters[0] : null)
                    ->state(is_array($parameters[0] ?? null) ? $parameters[0] : ($parameters[1] ?? []));
    }

  // ...
}

I tried to "overwrite" the spread $parameters to determine the return type, but unfortunately Psalm crashed as a consequence.

[caugner]

In Reflection, $storage->params gets updated at two locations without updating $storage->param_lookup:

psalm/src/Psalm/Internal/Codebase/Reflection.php

Lines 283 to 298 in 9acbb60

	$storage->params = [];
	foreach ($callables[0]->params as $param) {
	if ($param->type) {
	$param->type->queueClassLikesForScanning($this->codebase);
	}
	}
	$storage->params = $callables[0]->params;
	$storage->return_type = $callables[0]->return_type;
	$storage->return_type->queueClassLikesForScanning($this->codebase);
	} else {
	$params = $method->getParameters();
	$storage->params = [];

psalm/src/Psalm/Internal/Codebase/Reflection.php

Lines 370 to 378 in 9acbb60

	$storage->params = $callmap_callable->params;
	$storage->return_type = $callmap_callable->return_type;
	} else {
	$reflection_params = $reflection_function->getParameters();
	foreach ($reflection_params as $param) {
	$param_obj = $this->getReflectionParamData($param);
	$storage->params[] = $param_obj;
	}

Maybe it shouldn't be updated directly, but rather via two methods FunctionLikeStorage->setParams() and addParam() so that param_lookup can be maintained directly by FunctionLikeStorage.