Skip to content

Commit c49b473

Browse files
committed
refactor: share callable param classification between router and dispatcher
1 parent 26d6898 commit c49b473

File tree

7 files changed

+123
-46
lines changed

7 files changed

+123
-46
lines changed

system/CodeIgniter.php

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
use CodeIgniter\HTTP\ResponsableInterface;
3232
use CodeIgniter\HTTP\ResponseInterface;
3333
use CodeIgniter\HTTP\URI;
34+
use CodeIgniter\Router\CallableParamClassifier;
35+
use CodeIgniter\Router\ParamKind;
3436
use CodeIgniter\Router\RouteCollectionInterface;
3537
use CodeIgniter\Router\Router;
3638
use Config\App;
@@ -727,36 +729,37 @@ private function resolveCallableParams(ReflectionFunctionAbstract $reflection, a
727729
$routeIndex = 0;
728730

729731
foreach ($reflection->getParameters() as $param) {
730-
// Inject FormRequest subclasses regardless of position.
731-
$formRequestClass = FormRequest::getFormRequestClass($param);
732-
733-
if ($formRequestClass !== null) {
734-
$resolved[] = $this->resolveFormRequest($formRequestClass);
735-
736-
continue;
737-
}
738-
739-
// Variadic parameter - consume all remaining route segments.
740-
if ($param->isVariadic()) {
741-
while (array_key_exists($routeIndex, $routeParams)) {
742-
$resolved[] = $routeParams[$routeIndex++];
743-
}
744-
745-
break;
746-
}
747-
748-
// Consume the next route segment if one is available.
749-
if (array_key_exists($routeIndex, $routeParams)) {
750-
$resolved[] = $routeParams[$routeIndex++];
751-
752-
continue;
753-
}
754-
755-
// No more route segments. Required params stop iteration so that
756-
// PHP throws an ArgumentCountError on the call site. Optional
757-
// params are omitted - PHP then applies their declared default value.
758-
if (! $param->isOptional()) {
759-
break;
732+
[$kind, $formRequestClass] = CallableParamClassifier::classify($param);
733+
734+
switch ($kind) {
735+
case ParamKind::FormRequest:
736+
// Inject FormRequest subclasses regardless of position.
737+
$resolved[] = $this->resolveFormRequest($formRequestClass);
738+
739+
continue 2;
740+
741+
case ParamKind::Variadic:
742+
// Consume all remaining route segments.
743+
while (array_key_exists($routeIndex, $routeParams)) {
744+
$resolved[] = $routeParams[$routeIndex++];
745+
}
746+
break 2;
747+
748+
case ParamKind::Scalar:
749+
// Consume the next route segment if one is available.
750+
if (array_key_exists($routeIndex, $routeParams)) {
751+
$resolved[] = $routeParams[$routeIndex++];
752+
753+
continue 2;
754+
}
755+
756+
// No more route segments. Required params stop iteration so
757+
// that PHP throws an ArgumentCountError on the call site.
758+
// Optional params are omitted - PHP then applies their
759+
// declared default value.
760+
if (! $param->isOptional()) {
761+
break 2;
762+
}
760763
}
761764
}
762765

system/Commands/Generators/Views/formrequest.tpl.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function rules(): array
2121
// /**
2222
// * Custom error messages keyed by field.rule.
2323
// *
24-
// * @return array<string, array<string, string>|string>
24+
// * @return array<string, array<string, string>>
2525
// */
2626
// public function messages(): array
2727
// {

system/HTTP/FormRequest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ abstract public function rules(): array;
7474
* 'title' => ['required' => 'Post title cannot be empty.'],
7575
* ];
7676
*
77-
* @return array<string, array<string, string>|string>
77+
* @return array<string, array<string, string>>
7878
*/
7979
public function messages(): array
8080
{

system/Router/AutoRouterImproved.php

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
namespace CodeIgniter\Router;
1515

1616
use CodeIgniter\Exceptions\PageNotFoundException;
17-
use CodeIgniter\HTTP\FormRequest;
1817
use CodeIgniter\Router\Exceptions\MethodNotFoundException;
1918
use Config\Routing;
2019
use ReflectionClass;
@@ -443,22 +442,24 @@ private function checkParameters(): void
443442
throw new MethodNotFoundException();
444443
}
445444

446-
// FormRequest parameters are injected by the framework and do not
447-
// consume URI segments, so exclude them from the count.
448-
$nonFormRequestParams = array_values(array_filter(
449-
$refParams,
450-
static fn ($p): bool => FormRequest::getFormRequestClass($p) === null,
451-
));
452-
453-
// A variadic parameter absorbs any number of trailing URI segments,
454-
// so there is no upper bound to enforce.
455-
foreach ($nonFormRequestParams as $p) {
456-
if ($p->isVariadic()) {
445+
// Count only parameters that consume one URI segment each. A variadic
446+
// parameter absorbs any number of trailing segments, so there is no
447+
// upper bound to enforce.
448+
$scalarCount = 0;
449+
450+
foreach ($refParams as $param) {
451+
[$kind] = CallableParamClassifier::classify($param);
452+
453+
if ($kind === ParamKind::Variadic) {
457454
return;
458455
}
456+
457+
if ($kind === ParamKind::Scalar) {
458+
$scalarCount++;
459+
}
459460
}
460461

461-
if (count($nonFormRequestParams) < count($this->params)) {
462+
if ($scalarCount < count($this->params)) {
462463
throw new PageNotFoundException(
463464
'The param count in the URI are greater than the controller method params.'
464465
. ' Handler:' . $this->controller . '::' . $this->method
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
9+
*
10+
* For the full copyright and license information, please view
11+
* the LICENSE file that was distributed with this source code.
12+
*/
13+
14+
namespace CodeIgniter\Router;
15+
16+
use CodeIgniter\HTTP\FormRequest;
17+
use ReflectionParameter;
18+
19+
/**
20+
* Single source of truth for how the auto-router and the dispatcher
21+
* interpret a reflected callable parameter with respect to URI segment
22+
* consumption. Keeps AutoRouterImproved::checkParameters() and
23+
* CodeIgniter::resolveCallableParams() aligned on "FormRequest",
24+
* "variadic", and "scalar URI consumer".
25+
*/
26+
final class CallableParamClassifier
27+
{
28+
/**
29+
* Returns the param kind and, when the kind is FormRequest, the resolved
30+
* FormRequest class name so the caller does not need to re-inspect the
31+
* parameter's type to inject it.
32+
*
33+
* @return array{ParamKind, class-string<FormRequest>|null}
34+
*/
35+
public static function classify(ReflectionParameter $param): array
36+
{
37+
$formRequestClass = FormRequest::getFormRequestClass($param);
38+
39+
if ($formRequestClass !== null) {
40+
return [ParamKind::FormRequest, $formRequestClass];
41+
}
42+
43+
if ($param->isVariadic()) {
44+
return [ParamKind::Variadic, null];
45+
}
46+
47+
return [ParamKind::Scalar, null];
48+
}
49+
}

system/Router/ParamKind.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
9+
*
10+
* For the full copyright and license information, please view
11+
* the LICENSE file that was distributed with this source code.
12+
*/
13+
14+
namespace CodeIgniter\Router;
15+
16+
/**
17+
* Classifies how a reflected callable parameter consumes URI segments
18+
* during auto-routing and dispatch.
19+
*/
20+
enum ParamKind
21+
{
22+
case FormRequest;
23+
case Variadic;
24+
case Scalar;
25+
}

tests/system/HTTP/FormRequestTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,6 @@ public function testResolveRequestReturnsResponseOnInvalidData(): void
359359
$response = $formRequest->resolveRequest();
360360

361361
$this->assertInstanceOf(ResponseInterface::class, $response);
362-
$this->assertInstanceOf(ResponseInterface::class, $response);
363362
}
364363

365364
public function testFailedValidationResponseContainsErrors(): void

0 commit comments

Comments
 (0)