Named arguments in refactorings

[mamazu]

Question

I have the following code I want to refactor:

function getRequestParameter(Request $request, string $parameter, bool $force = false, $default = null);

// Example:
$this->getRequestParameter($request, 'locale', true);
$this->getRequestParameter($request, 'page', false, '1');

This should be converted to Symfony's $request->get function.

The current approach

My current approach is to reorder and reuse the arguments based on their position in the call.=

        /** @var MethodCall $node */
        if ($this->nodeNameResolver->isName($node->var, '$this') || (string) $node->name !== 'getRequestParameter' ){ return; }

        $request = $node->args[0]->value;
        $parameterName = $node->args[1]->value;
        $force = $node->args[2]?->value ?? $this->nodeFactory->createFalse();
        $default = $node->args[3]?->value ?? $this->nodeFactory->createNull();

But this raises the question: What is the best way to cover named arguments. Calls like this:

$this->getRequestParameter($request, 'locale', default: 'en');

Is there already code in rector to handle this better? If not would be happy to provide an implementation.

[TomasVotruba]

You can set argument name like this:

$node->args[1]->name = '...'

See https://getrector.com/ast/defc4808206c4161b76f5bbff542c6a7dc90e4ec

[mamazu]

The problem is not the setting. The problem is getting the parameters from the old code. Because depending on the value of the "force" parameter in the function the rector should work different. But with named arguments I can't garantuee that in the old code the "force" parameter exists or is in the third position. Technically the old code could look like this:

$this->getRequestParameter(force: true, parameter: 'locale', default: 'en', request: $request);

[mamazu]

This could then also allow us to make this work: Removing parameters by name instead of positions: https://getrector.com/demo/a0594762-5100-4e64-a761-74adc9eed071

[TomasVotruba]

In that case, you need to do a reflection on getRequestParameter() method. Using PHPStan\Reflection\ReflectionProvider service

[samsonasik]

You can enforce to remove named argument first, by using DowngradeNamedArgumentRector rule first, then it always sorted argument, see

https://getrector.com/demo/70dddd06-9db6-442b-a58a-4fc7a49a059f

after it sorted, use your custom rule to remove specific position.

[mamazu]

I don't understand how you would use the ReflectionProvider. If I do I get a MethodReflection where most of the important things are private: https://apiref.phpstan.org/1.8.x/PHPStan.Reflection.Php.PhpMethodReflection.html

And as far as the DowngradeNamedArgumentRector that sounds like a doable solution. However I think having a service that could just take a list of arguments and convert them into a map, would make things easier. I also found other code which requires named arguments.

[mamazu]

If you're interested what solution I went with it's something like this:

    /**
     * Turns a list of parameters into a hash map of named parameters.
     * If the parameter is not specified, it will use the default value from the definitions array.
     *
     * @param array<Arg> $arguments
     * @param array<string, Expr|null> $definitions
     *
     * @return array<string, Expr|null>
     */
    private function arguments(array $arguments, array $definitions): array
    {
        $currentArgumentPosition = 0;
        $names = \array_keys($definitions);
        $result = [];
        foreach ($arguments as $argument) {
            if (null === $argument->name) {
                // Get the argument name based on the position
                $name = $names[$currentArgumentPosition++];
            } else {
                // Get the argument name from the named argument
                $name = $this->getName($argument->name);
            }
            $result[$name] = $argument->value;
            \unset($definitions[$name]);
        }

        // Concatenate the remaining defaulted arguments
        return [...$definitions, ...$result];
    }

and you can use it like that:

        $arguments = $this->arguments($args, [
            'request' => null,
            'parameterName' => null,
            'force' => $this->nodeFactory->createFalse(),
            'default' => null,
        ]);

The keys are the parameter names the function should have and the values are the default values of this function. I'm sure this could be generalized with Reflection, but as I said I couldn't figure out how this would work with the phpstan reflections.

[staabm]

You might be interested in PHPStan ArgumentsNormalizer which primary job is to re-write a named arguments list into a regular one

https://github.com/phpstan/phpstan-src/blob/2.1.x/src/Analyser/ArgumentsNormalizer.php