Add new RequestResponseNamedArgs route strategy

[adoy]

PHP8.0 added support for named parameters https://wiki.php.net/rfc/named_params
This new strategy will map route to parameters name.

$app->get('/{greeting}/{name}', function (ServerRequestInterface $request, ResponseInterface $response, $name, $greeting) {
    $response->getBody()->write("{$greeting} {$name}");
    return $response;
});

[l0gicgate]

I like this strategy, it sucks that named parameter mapping isn't available in 7.4 though.

@akrabat what do you think of adding this? It's only available for PHP 8.0 which makes me reluctant. Maybe this should be a Slim 5 feature instead.

[adoy]

I fixed the CS issue in the test file. Sorry about that.

Also, I agree that it sucks that this is only available for 8.0+ but 7.4 is now in security fixes only.
My personnal input on this is that I think it's better to penalize people who aren't using the latest versions of PHP rather than penalize people who update.
Projects that I create today are using PHP8.1 and Slim4. I could take advantage of this feature today, but if I wait for slim5 I will also have to migrate my code.

[adoy]

We could make the basic use case work with some reflection like this :

public function __invoke(
    callable $callable,
    ServerRequestInterface $request,
    ResponseInterface $response,
    array $routeArguments
): ResponseInterface {
    if (PHP_VERSION_ID >= 80000) {
        return $callable($request, $response, ...$routeArguments);
    }

    $args = [ $request, $response ];
    $rf = new ReflectionFunction($callable);
    $parameters = $rf->getParameters();
    $parametersCount = count($parameters);
    $hasVariadic = false;

    for ($i = 2; $i < $parametersCount; $i++) {
        if ($parameters[$i]->isVariadic()) {
            $hasVariadic = true;
            $args = array_merge($args, array_values($routeArguments));
        } else {
            $parameterName = $parameters[$i]->getName();
            if (isset($routeArguments[$parameterName])) {
                $args[] = $routeArguments[$parameterName];
                unset($routeArguments[$parameterName]);
            } elseif ($parameters[$i]->isOptional()) {
                $args[] = $parameters[$i]->getDefaultValue();
            } else {
                throw new \Exception(sprintf('Argument #%d ($%s) not passed', $i, $parameterName));
            }
        }
    }

    if (!$hasVariadic && !empty($routeArguments)) {
        $parameterName = key($routeArguments);
        throw new \Exception(sprintf('Unknown named parameter $%s', $parameterName));
    }

    return $callable(...$args);
}

If you like this (I'm not a fan of it) we would have to discuss :

• What do we do if the user give a variadic function (it becomes impossible to identify parameters)
• We would have to make the two exceptions in my exemple consistant with PHP8 (meaning catching PHP8 exceptions and throw custom exceptions to be consistant across all versions).

I don't think it's worth it for a version that will not be supported in less than a year. But I'm just giving the different solutions so that you can take the one that feet the best with your vision.

[coveralls]

Coverage decreased (-0.1%) to 99.901% when pulling 65b1ad4 on adoy:request-response-namedarg-strategy into 6aa522d on slimphp:4.x.

[l0gicgate]

My personnal input on this is that I think it's better to penalize people who aren't using the latest versions of PHP rather than penalize people who update.

I agree with this wholeheartedly actually.

I'm fine with merging this after some thought.

Can we write a test for the constructor exception please?

[adoy]

Thanks. I added some tests for both 8.0+ and 7.4 but the coverage will still decrease since the coverage run on 8.1.

[l0gicgate]

@adoy you should be able to override PHP_VERSION_ID. If not, we will have to create a helper function to return current PHP version instead of using the constant directly which we can stub during testing.

[adoy]

Just to make sure to understand, isn't it enough to have one test for PHP7.4 to make sure the exception is thrown and other for PHP8.X to make sure the behaviour is correct ? If I create an Helper function, we'll have the same problem to test this function, if I use the constant inside this helper function. Is it only to have the 100% coverage under PHP8.1 ? As for the constant it is a PHP defined constant so I can not overwrite it, and if I create it only in the used namespace, I will only have the oportunity to define it once, so for only one use case.

Thanks for your insight, it will help me find the best way to resolve it :-)

[l0gicgate]

Just one little thing then I'll merge this.

I noticed that the coverage went down because the exception thrown test is not being run since we run code coverage on php 8. It's fine though.

Thanks

[l0gicgate]

Can we add the use RuntimeException; statement on line 16 and remove the \ here please

[adoy]

Sure !

[l0gicgate]

Thank you for this contribution!