From a73cba5b6e26792896351f0033ac867104dee436 Mon Sep 17 00:00:00 2001 From: Alexandre Gomes Gaigalas Date: Sun, 28 Jun 2026 15:34:47 -0300 Subject: [PATCH] Match typed arguments by type regardless of position resolve() now binds typed parameters in a first pass: a positional object is claimed by the parameter declaring its type wherever it appears, so a leading untyped (scalar) parameter can no longer consume an object meant for a typed parameter declared after it. Untyped parameters then take the remaining positional arguments in declaration order. Add tests for out-of-order typed matching and update the README resolution order and a type-first example accordingly. --- README.md | 20 ++++- src/ContainerResolver.php | 130 ++++++++++++++++++++------- tests/unit/ContainerResolverTest.php | 60 +++++++++++++ 3 files changed, 175 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 35aace8..191df4f 100644 --- a/README.md +++ b/README.md @@ -15,13 +15,13 @@ composer require respect/parameter For each parameter the resolver tries, in order: 1. An explicit **named** argument (keyed by parameter name) -2. A **positional** argument already matching the parameter **type** +2. An argument matching the parameter **type** 3. The **container**, matched by **type** (non-builtin) -4. The next **positional** argument +4. The next remaining **positional** argument 5. The parameter's **default value** 6. `null` -A trailing **variadic** parameter receives a matching named argument (if any) followed by every remaining positional argument. +Typed parameters are bound first (steps 1–3), so a positional object is matched by type wherever it sits and an earlier untyped parameter can't consume it; untyped parameters then take the leftover positional arguments in declaration order (step 4). A trailing **variadic** parameter receives a matching named argument (if any) followed by every remaining positional argument. ```php use Respect\Parameter\ContainerResolver; @@ -43,6 +43,20 @@ notify(...$args); $reflection->newInstanceArgs($args); ``` +### Type-first matching + +A positional object is bound to the parameter that declares its type, wherever each sits in the list — +so an untyped parameter never accidentally swallows it: + +```php +function notify(string $subject, Mailer $mailer) { + // ... +} + +$args = $resolver->resolve(new ReflectionFunction('notify'), [$mailer, 'Hello']); +// ['Hello', Mailer] — $mailer matched by type, 'Hello' fell through to $subject +``` + ### Named arguments `resolve()` accepts named arguments too — keyed by parameter name, taking precedence over the diff --git a/src/ContainerResolver.php b/src/ContainerResolver.php index 16df916..3b7e7c5 100644 --- a/src/ContainerResolver.php +++ b/src/ContainerResolver.php @@ -19,6 +19,9 @@ use ReflectionParameter; use function array_key_exists; +use function array_key_last; +use function array_keys; +use function array_pop; use function array_values; use function assert; use function count; @@ -46,10 +49,11 @@ public function __construct(private ContainerInterface $container) * Resolve the arguments for a function/constructor. * * Provided arguments may be positional (int-keyed) or named (string-keyed by parameter name). - * For each parameter, in order: an explicit named argument wins; then a positional argument - * already matching the parameter type; then the container by type; then the next positional - * argument; then the parameter default; otherwise null. A trailing variadic parameter receives - * a matching named argument (if any) followed by every remaining positional argument. + * For each parameter, in order: an explicit named argument wins; then any not-yet-consumed + * positional argument matching the parameter type (regardless of its position); then the + * container by type; then the next not-yet-consumed positional argument; then the parameter + * default; otherwise null. A trailing variadic parameter receives a matching named argument + * (if any) followed by every remaining positional argument. * * @param array $arguments * @@ -72,51 +76,90 @@ public function resolve(ReflectionFunctionAbstract $reflection, array $arguments } } - $resolved = []; - $index = 0; - $count = count($positional); - - foreach ($parameters as $param) { - $name = $param->getName(); - - if ($param->isVariadic()) { - if (array_key_exists($name, $named)) { - $resolved[] = $named[$name]; - } - - while ($index < $count) { - $resolved[] = $positional[$index++]; - } + // A variadic parameter is always the trailing one in PHP, so pull it off once here + // instead of testing isVariadic() on every parameter inside the passes below. + $variadic = null; + $lastKey = array_key_last($parameters); + if ($parameters[$lastKey]->isVariadic()) { + $variadic = $parameters[$lastKey]; + array_pop($parameters); + } - break; - } + $slot = []; + $used = []; + $deferred = []; + // First pass: bind named arguments and typed parameters. A typed parameter claims any + // matching positional argument regardless of position, so a leading scalar parameter can + // never steal an object meant for a typed parameter declared after it. + foreach ($parameters as $index => $param) { + $name = $param->getName(); if (array_key_exists($name, $named)) { - $resolved[] = $named[$name]; + $slot[$index] = $named[$name]; continue; } $type = self::typeName($param); + if ($type !== null) { + $match = self::firstUnused($positional, $used, $type); + if ($match !== null) { + $slot[$index] = $positional[$match]; + $used[$match] = true; - if ($type !== null && isset($positional[$index]) && $positional[$index] instanceof $type) { - $resolved[] = $positional[$index++]; + continue; + } - continue; + if ($this->container->has($type)) { + $slot[$index] = $this->container->get($type); + + continue; + } } - if ($type !== null && $this->container->has($type)) { - $resolved[] = $this->container->get($type); + $deferred[$index] = $param; + } - continue; + // Second pass: fill the remaining parameters from leftover positional arguments in order, + // advancing a single cursor instead of rescanning from the start for each one. + $cursor = 0; + $total = count($positional); + foreach ($deferred as $index => $param) { + while ($cursor < $total && ($used[$cursor] ?? false)) { + $cursor++; } - if ($index < $count) { - $resolved[] = $positional[$index++]; + if ($cursor < $total) { + $slot[$index] = $positional[$cursor]; + $used[$cursor] = true; + $cursor++; } elseif ($param->isDefaultValueAvailable()) { - $resolved[] = $param->getDefaultValue(); + $slot[$index] = $param->getDefaultValue(); } else { - $resolved[] = null; + $slot[$index] = null; + } + } + + // Assemble the fixed parameters in declaration order (plain array reads, no reflection), + // then expand a trailing variadic from whatever named element and positional arguments + // remain unconsumed. + $resolved = []; + foreach (array_keys($parameters) as $index) { + $resolved[] = $slot[$index]; + } + + if ($variadic !== null) { + $name = $variadic->getName(); + if (array_key_exists($name, $named)) { + $resolved[] = $named[$name]; + } + + foreach ($positional as $i => $value) { + if ($used[$i] ?? false) { + continue; + } + + $resolved[] = $value; } } @@ -163,6 +206,29 @@ public static function acceptsType(ReflectionFunctionAbstract $reflection, strin return false; } + /** + * Index of the first positional argument not yet consumed, optionally constrained to one whose + * value is an instance of the given type. Returns null when no such argument remains. + * + * @param list $positional + * @param array $used + * @param class-string|null $type + */ + private static function firstUnused(array $positional, array $used, string|null $type): int|null + { + foreach ($positional as $i => $value) { + if ($used[$i] ?? false) { + continue; + } + + if ($type === null || $value instanceof $type) { + return $i; + } + } + + return null; + } + /** @return class-string|null */ private static function typeName(ReflectionParameter $param): string|null { diff --git a/tests/unit/ContainerResolverTest.php b/tests/unit/ContainerResolverTest.php index 27b8950..0526e06 100644 --- a/tests/unit/ContainerResolverTest.php +++ b/tests/unit/ContainerResolverTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; use ReflectionClass; use ReflectionFunction; use ReflectionMethod; @@ -43,6 +44,65 @@ public function itShouldResolveByType(): void self::assertSame([$service, 'hello', 42], $args); } + #[Test] + public function itShouldResolveByTypeDespiteOrder(): void + { + $service = new SampleService(); + $anotherService = new SampleService(); + $container = new ArrayContainer([ + SampleService::class => $service, + 'zoo' => $anotherService, + ]); + $resolver = new ContainerResolver($container); + + $args = $resolver->resolve($this->constructorOf(ServiceConsumer::class), ['hello', $container->get('zoo')]); + + self::assertSame([$anotherService, 'hello', 42], $args); + } + + #[Test] + public function itShouldResolveTypedArgumentDeclaredAfterScalar(): void + { + $service = new SampleService(); + $resolver = new ContainerResolver(new ArrayContainer()); + $fn = new ReflectionFunction( + static fn(string $value, SampleService $service): array => [$value, $service], + ); + + $args = $resolver->resolve($fn, [$service, 'hello']); + + self::assertSame(['hello', $service], $args); + } + + #[Test] + public function itShouldResolveMultipleTypedArgumentsOutOfOrder(): void + { + $service = new SampleService(); + $container = new ArrayContainer(); + $resolver = new ContainerResolver($container); + $fn = new ReflectionFunction( + static fn(SampleService $service, ContainerInterface $c, string $value): array => [$service, $c, $value], + ); + + $args = $resolver->resolve($fn, ['hello', $container, $service]); + + self::assertSame([$service, $container, 'hello'], $args); + } + + #[Test] + public function itShouldNotConsumeScalarForUnfilledTypedParameter(): void + { + $service = new SampleService(); + $resolver = new ContainerResolver(new ArrayContainer()); + $fn = new ReflectionFunction( + static fn(int $number, SampleService $service, string $value): array => [$number, $service, $value], + ); + + $args = $resolver->resolve($fn, [7, 'hello', $service]); + + self::assertSame([7, $service, 'hello'], $args); + } + #[Test] public function itShouldAllowUserOverride(): void {