8000 [DependencyInjection] Added support for variadics in named arguments by PabloKowalczyk · Pull Request #24937 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] Added support for variadics in named arguments #24937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 20 commits into from
Closed

[DependencyInjection] Added support for variadics in named arguments #24937

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1e0d7e4
[DependencyInjection] Added support for variadics in named arguments
Nov 12, 2017
1859650
[DependencyInjection] Added support for variadics in named arguments
Nov 12, 2017
31b8935
Removed variadic argument from NamedArgumentsDummy
Nov 12, 2017
625b3d3
Introduce NamedArgumentsVariadicsDummy to test variadics named argume…
Nov 12, 2017
3b55332
Test variadics named arguments only on PHP 5.6 and above, also don't …
Nov 12, 2017
f0f4c22
Removed blank line.
Nov 12, 2017
9dbaa09
Removed "strict_types".
Nov 13, 2017
e2c3e22
Hook into $parameters foreach loop instead of "last argument" enforcing.
Nov 13, 2017
28305f2
Removed unused "use".
Nov 13, 2017
25cb9bb
CS fixes.
Nov 14, 2017
62db734
Do not require PHP 5.6 version.
Nov 15, 2017
9961ea5
[DependencyInjection] Added support for variadics in named arguments
Nov 12, 2017
c849e5a
[DependencyInjection] Added support for variadics in named arguments
Nov 12, 2017
44f71c8
Removed variadic argument from NamedArgumentsDummy
Nov 12, 2017
8e55fb2
Removed unused "use".
Nov 13, 2017
6c0a574
CS fixes.
Nov 14, 2017
c749c0c
Do not require PHP 5.6 version.
Nov 15, 2017
2e8ebf7
Added another constructor parameter to improve test.
Nov 19, 2017
86c2db2
CS fix.
PabloKowalczyk Nov 19, 2017
ca5453e
Removed PHP requirement.
PabloKowalczyk Nov 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[DependencyInjection] Added support for variadics in named arguments
  • Loading branch information
@PabloKowalczyk
pkowalczyk authored and PabloKowalczyk committed Nov 20, 2017
commit 9961ea561b543feffd7dcca990d493583863a3d3
20 changes: 20 additions & 0 deletions src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\LazyProxy\ProxyHelper;
use Symfony\Component\DependencyInjection\Reference;

Expand Down Expand Up @@ -87,6 +88,25 @@ protected function processValue($value, $isRoot = false)
}
}

$lastResolvedArgument = \end($resolvedArguments);

if (\is_array($lastResolvedArgument) && !empty($lastResolvedArgument)) {
try {
$reflection = $this->getReflectionMethod($value, $method);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better implementation would hook into the existing foreach loop on line 56-57.
We just need a if (\PHP_VERSION_ID >= 50600 && $p->isVariadic() check inside the loop.
No need for enforcing this is the "last argument", PHP already did so.
And the key of the added params must be explicitly set ($resolvedArguments[$j++] instead of $resolvedArguments[], which can be wrong)

$parameters = $reflection->getParameters();
$lastParam = \end($parameters);

if ($lastParam->isVariadic()) {
\array_pop($resolvedArguments);

foreach ($lastResolvedArgument as $argument) {
$resolvedArguments[] = $argument;
}
}
} catch (RuntimeException $runtimeException) {
}
}

if ($resolvedArguments !== $call[1]) {
ksort($resolvedArguments);
$calls[$i][1] = $resolvedArguments;
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/Tests/Fixtures/NamedArgumentsDummy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/
class NamedArgumentsDummy
{
public function __construct(CaseSensitiveClass $c, $apiKey, $hostName)
public function __construct(CaseSensitiveClass $c, $apiKey, $hostName, ...$variadics)
{
}

Expand Down
0