Skip to content

Commit

Permalink
[CodingStyle] Improve UnSpreadOperatorRector to join argument to array
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed Jan 9, 2021
1 parent c0f0603 commit 93fc000
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 160 deletions.
86 changes: 0 additions & 86 deletions packages/vendor-locker/src/VendorFileDetector.php

This file was deleted.

44 changes: 14 additions & 30 deletions rules/coding-style/src/NodeAnalyzer/SpreadVariablesCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,49 +4,33 @@

namespace Rector\CodingStyle\NodeAnalyzer;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Param;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\NodeTypeResolver\Node\AttributeKey;

final class SpreadVariablesCollector
{
/**
* @param Param[]|Arg[] $nodes
* @return Param[]|Arg[]
* @return array<int, Param>
*/
public function getSpreadVariables(array $nodes): array
public function resolveFromClassMethod(ClassMethod $classMethod): array
{
$spreadVariables = [];
$spreadParams = [];

foreach ($nodes as $key => $paramOrArg) {
if ($this->isVariadicParam($paramOrArg)) {
$spreadVariables[$key] = $paramOrArg;
foreach ($classMethod->params as $key => $param) {
// prevent race-condition removal on class method
$originalParam = $param->getAttribute(AttributeKey::ORIGINAL_NODE);
if (! $originalParam instanceof Param) {
continue;
}

if ($this->isVariadicArg($paramOrArg)) {
$spreadVariables[$key] = $paramOrArg;
if (! $originalParam->variadic) {
continue;
}
}

return $spreadVariables;
}

private function isVariadicParam(Node $node): bool
{
if (! $node instanceof Param) {
return false;
}

return $node->variadic && $node->type === null;
}

private function isVariadicArg(Node $node): bool
{
if (! $node instanceof Arg) {
return false;
$spreadParams[$key] = $param;
}

return $node->unpack && $node->value instanceof Variable;
return $spreadParams;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@
namespace Rector\CodingStyle\Rector\ClassMethod;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Identifier;
use PhpParser\Node\Param;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\CodingStyle\NodeAnalyzer\SpreadVariablesCollector;
use Rector\Core\Rector\AbstractRector;
use Rector\VendorLocker\VendorFileDetector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -20,21 +19,13 @@
*/
final class UnSpreadOperatorRector extends AbstractRector
{
/**
* @var VendorFileDetector
*/
private $vendorFileDetector;

/**
* @var SpreadVariablesCollector
*/
private $spreadVariablesCollector;

public function __construct(
VendorFileDetector $vendorFileDetector,
SpreadVariablesCollector $spreadVariablesCollector
) {
$this->vendorFileDetector = $vendorFileDetector;
public function __construct(SpreadVariablesCollector $spreadVariablesCollector)
{
$this->spreadVariablesCollector = $spreadVariablesCollector;
}

Expand Down Expand Up @@ -95,48 +86,68 @@ public function refactor(Node $node): ?Node

private function processUnspreadOperatorClassMethodParams(ClassMethod $classMethod): ?ClassMethod
{
if ($this->vendorFileDetector->isNodeInVendor($classMethod)) {
$spreadParams = $this->spreadVariablesCollector->resolveFromClassMethod($classMethod);
if ($spreadParams === []) {
return null;
}

$params = $classMethod->params;
if ($params === []) {
return null;
}

$spreadVariables = $this->spreadVariablesCollector->getSpreadVariables($params);
if ($spreadVariables === []) {
return null;
}

foreach (array_keys($spreadVariables) as $key) {
$classMethod->params[$key]->variadic = false;
$classMethod->params[$key]->type = new Identifier('array');
foreach ($spreadParams as $spreadParam) {
$spreadParam->variadic = false;
$spreadParam->type = new Identifier('array');
}

return $classMethod;
}

private function processUnspreadOperatorMethodCallArgs(MethodCall $methodCall): ?MethodCall
{
if ($this->vendorFileDetector->isNodeInVendor($methodCall)) {
$classMethod = $this->nodeRepository->findClassMethodByMethodCall($methodCall);
if ($classMethod === null) {
return null;
}

$args = $methodCall->args;
if ($args === []) {
$spreadParams = $this->spreadVariablesCollector->resolveFromClassMethod($classMethod);
if ($spreadParams === []) {
return null;
}

$spreadVariables = $this->spreadVariablesCollector->getSpreadVariables($args);
if ($spreadVariables === []) {
return null;
$firstSpreadParamPosition = array_key_first($spreadParams);
$variadicArgs = $this->resolveVariadicArgsByVariadicParams($methodCall, $firstSpreadParamPosition);

$hasUnpacked = false;

foreach ($variadicArgs as $position => $variadicArg) {
if ($variadicArg->unpack) {
$variadicArg->unpack = null;
$hasUnpacked = true;
$methodCall->args[$position] = $variadicArg;
}
}

foreach (array_keys($spreadVariables) as $key) {
$methodCall->args[$key]->unpack = false;
if ($hasUnpacked) {
return $methodCall;
}

$methodCall->args[$firstSpreadParamPosition] = new Arg($this->createArray($variadicArgs));
return $methodCall;
}

/**
* @return Arg[]
*/
private function resolveVariadicArgsByVariadicParams(MethodCall $methodCall, int $firstSpreadParamPosition): array
{
$variadicArgs = [];

foreach ($methodCall->args as $position => $arg) {
if ($position < $firstSpreadParamPosition) {
continue;
}

$variadicArgs[] = $arg;
unset($methodCall->args[$position]);
}

return $variadicArgs;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

namespace Rector\CodingStyle\Tests\Rector\ClassMethod\UnSpreadOperatorRector\Fixture;

final class TypedParamVariadic
{
public function run(int ...$var)
{
}
}

?>
-----
<?php

namespace Rector\CodingStyle\Tests\Rector\ClassMethod\UnSpreadOperatorRector\Fixture;

final class TypedParamVariadic
{
public function run(array $var)
{
}
}

?>
4 changes: 4 additions & 0 deletions src/PhpParser/Node/NodeFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,10 @@ private function createArrayItem($item, $key = null): ArrayItem
return new ArrayItem($itemValue);
}

if ($item instanceof Arg) {
return new ArrayItem($item->value);
}

throw new NotImplementedException(sprintf(
'Not implemented yet. Go to "%s()" and add check for "%s" node.',
__METHOD__,
Expand Down

0 comments on commit 93fc000

Please sign in to comment.