Merge pull request #317 from rectorphp/argument-replacer-value-object-typing

Argument replacer value object typing

README.md

@@ -208,23 +208,26 @@ rectors:
                 'code': 'string'
 ```
 
-### Change a argument value
+### Change argument value or remove argument
 
 ```yml
 rectors:
     Rector\Rector\Dynamic\ArgumentReplacerRector:
-        # class
-        'Symfony\Component\DependencyInjection\ContainerBuilder':
-            # method
-            'compile':
-                # argument position
-                0:
-                    # added default value
-                    '~': false
-                    # or remove completely
-                    '~': ~
-                    # or replace by new value
-                    'Symfony\Component\DependencyInjection\ContainerBuilder\ContainerBuilder::SCOPE_PROTOTYPE': false
+        -
+            class: 'Symfony\Component\DependencyInjection\ContainerBuilder'
+            method: 'compile'
+            position: 0
+            # change default value
+            type: 'changed'
+            default_value: false
+
+            # or remove
+            type: 'removed'
+
+            # or replace old default value by new one
+            type: 'replace_default_value'
+            replace_map:
+                'Symfony\Component\DependencyInjection\ContainerBuilder::SCOPE_PROTOTYPE': false
 ```
 
 ### Replace the underscore naming `_` with namespaces `\`

src/Rector/Dynamic/ArgumentReplacerRector.php

@@ -8,20 +8,20 @@ use PhpParser\Node;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\StaticCall;
-use PhpParser\Node\Identifier;
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\ClassMethod;
 use Rector\NodeAnalyzer\ClassMethodAnalyzer;
 use Rector\NodeAnalyzer\MethodCallAnalyzer;
 use Rector\NodeAnalyzer\StaticMethodCallAnalyzer;
 use Rector\Rector\AbstractRector;
+use Rector\Rector\Dynamic\Configuration\ArgumentReplacerRecipe;
 
 final class ArgumentReplacerRector extends AbstractRector
 {
     /**
-     * @var mixed[]
+     * @var ArgumentReplacerRecipe[]
      */
-    private $argumentChangesMethodAndClass = [];
+    private $argumentReplacerRecipes = [];
 
     /**
      * @var MethodCallAnalyzer
@@ -29,9 +29,9 @@ final class ArgumentReplacerRector extends AbstractRector
     private $methodCallAnalyzer;
 
     /**
-     * @var mixed[][]
+     * @var ArgumentReplacerRecipe[]
      */
-    private $activeArgumentChangesByPosition = [];
+    private $activeArgumentReplacerRecipes = [];
 
     /**
      * @var ClassMethodAnalyzer
@@ -58,7 +58,7 @@ final class ArgumentReplacerRector extends AbstractRector
         StaticMethodCallAnalyzer $staticMethodCallAnalyzer,
         ConstExprEvaluator $constExprEvaluator
     ) {
-        $this->argumentChangesMethodAndClass = $argumentChangesByMethodAndType;
+        $this->loadArgumentReplacerRecipes($argumentChangesByMethodAndType);
         $this->methodCallAnalyzer = $methodCallAnalyzer;
         $this->classMethodAnalyzer = $classMethodAnalyzer;
         $this->staticMethodCallAnalyzer = $staticMethodCallAnalyzer;
@@ -67,40 +67,18 @@ final class ArgumentReplacerRector extends AbstractRector
 
     public function isCandidate(Node $node): bool
     {
-        $this->activeArgumentChangesByPosition = $this->matchArgumentChanges($node);
+        $this->activeArgumentReplacerRecipes = $this->matchArgumentChanges($node);
 
-        return (bool) $this->activeArgumentChangesByPosition;
+        return (bool) $this->activeArgumentReplacerRecipes;
     }
 
     /**
      * @param MethodCall|StaticCall|ClassMethod $node
      */
-    public function refactor(Node $node): ?Node
+    public function refactor(Node $node): Node
     {
         $argumentsOrParameters = $this->getNodeArgumentsOrParameters($node);
-
-        foreach ($this->activeArgumentChangesByPosition as $position => $argumentChange) {
-            $key = key($argumentChange);
-            $value = array_shift($argumentChange);
-
-            if ($key === '~') {
-                if ($value === null) { // remove argument
-                    unset($argumentsOrParameters[$position]);
-                } else { // new default value
-                    $argumentsOrParameters[$position] = BuilderHelpers::normalizeValue($value);
-                }
-            } else {
-                // replace old value with new one
-                /** @var Arg $argumentOrParameter */
-                $argumentOrParameter = $argumentsOrParameters[$position];
-
-                $resolvedValue = $this->constExprEvaluator->evaluateDirectly($argumentOrParameter->value);
-
-                if ($resolvedValue === $key) {
-                    $argumentsOrParameters[$position] = BuilderHelpers::normalizeValue($value);
-                }
-            }
-        }
+        $argumentsOrParameters = $this->processArgumentNodes($argumentsOrParameters);
 
         $this->setNodeArgumentsOrParameters($node, $argumentsOrParameters);
 
@@ -108,7 +86,7 @@ final class ArgumentReplacerRector extends AbstractRector
     }
 
     /**
-     * @return mixed[][]
+     * @return ArgumentReplacerRecipe[]
      */
     private function matchArgumentChanges(Node $node): array
     {
@@ -116,17 +94,15 @@ final class ArgumentReplacerRector extends AbstractRector
             return [];
         }
 
-        foreach ($this->argumentChangesMethodAndClass as $type => $argumentChangesByMethod) {
-            $methods = array_keys($argumentChangesByMethod);
-            if ($this->isTypeAndMethods($node, $type, $methods)) {
-                /** @var Identifier $identifierNode */
-                $identifierNode = $node->name;
+        $argumentReplacerRecipes = [];
 
-                return $argumentChangesByMethod[$identifierNode->toString()];
+        foreach ($this->argumentReplacerRecipes as $argumentReplacerRecipe) {
+            if ($this->isNodeToRecipeMatch($node, $argumentReplacerRecipe)) {
+                $argumentReplacerRecipes[] = $argumentReplacerRecipe;
             }
         }
 
-        return [];
+        return $argumentReplacerRecipes;
     }
 
     /**
@@ -158,19 +134,70 @@ final class ArgumentReplacerRector extends AbstractRector
         }
     }
 
-    /**
-     * @param string[] $methods
-     */
-    private function isTypeAndMethods(Node $node, string $type, array $methods): bool
+    private function isNodeToRecipeMatch(Node $node, ArgumentReplacerRecipe $argumentReplacerRecipe): bool
     {
-        if ($this->methodCallAnalyzer->isTypeAndMethods($node, $type, $methods)) {
+        $type = $argumentReplacerRecipe->getClass();
+        $method = $argumentReplacerRecipe->getMethod();
+
+        if ($this->methodCallAnalyzer->isTypeAndMethods($node, $type, [$method])) {
             return true;
         }
 
-        if ($this->staticMethodCallAnalyzer->isTypeAndMethods($node, $type, $methods)) {
+        if ($this->staticMethodCallAnalyzer->isTypeAndMethods($node, $type, [$method])) {
             return true;
         }
 
-        return $this->classMethodAnalyzer->isTypeAndMethods($node, $type, $methods);
+        return $this->classMethodAnalyzer->isTypeAndMethods($node, $type, [$method]);
+    }
+
+    /**
+     * @param mixed[] $configurationArrays
+     */
+    private function loadArgumentReplacerRecipes(array $configurationArrays): void
+    {
+        foreach ($configurationArrays as $configurationArray) {
+            $this->argumentReplacerRecipes[] = ArgumentReplacerRecipe::createFromArray($configurationArray);
+        }
+    }
+
+    /**
+     * @param mixed[] $argumentNodes
+     * @return mixed[]
+     */
+    private function processArgumentNodes(array $argumentNodes): array
+    {
+        foreach ($this->activeArgumentReplacerRecipes as $argumentReplacerRecipe) {
+            $type = $argumentReplacerRecipe->getType();
+            $position = $argumentReplacerRecipe->getPosition();
+
+            if ($type === ArgumentReplacerRecipe::TYPE_REMOVED) {
+                unset($argumentNodes[$position]);
+            } elseif ($type === ArgumentReplacerRecipe::TYPE_CHANGED) {
+                $argumentNodes[$position] = BuilderHelpers::normalizeValue(
+                    $argumentReplacerRecipe->getDefaultValue()
+                );
+            } elseif ($type === ArgumentReplacerRecipe::TYPE_REPLACED_DEFAULT_VALUE) {
+                $argumentNodes[$position] = $this->processReplacedDefaultValue(
+                    $argumentNodes[$position],
+                    $argumentReplacerRecipe
+                );
+            }
+        }
+
+        return $argumentNodes;
+    }
+
+    private function processReplacedDefaultValue(Arg $argNode, ArgumentReplacerRecipe $argumentReplacerRecipe): Arg
+    {
+        $resolvedValue = $this->constExprEvaluator->evaluateDirectly($argNode->value);
+
+        $replaceMap = $argumentReplacerRecipe->getReplaceMap();
+        foreach ($replaceMap as $oldValue => $newValue) {
+            if ($resolvedValue === $oldValue) {
+                return new Arg(BuilderHelpers::normalizeValue($newValue));
+            }
+        }
+
+        return $argNode;
     }
 }

src/Rector/Dynamic/Configuration/ArgumentReplacerRecipe.php

@@ -0,0 +1,159 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Rector\Dynamic\Configuration;
+
+use Rector\Exception\Rector\InvalidRectorConfigurationException;
+use Rector\Rector\Dynamic\ArgumentReplacerRector;
+
+final class ArgumentReplacerRecipe
+{
+    /**
+     * @var string
+     */
+    public const TYPE_REMOVED = 'removed';
+
+    /**
+     * @var string
+     */
+    public const TYPE_CHANGED = 'changed';
+
+    /**
+     * @var string
+     */
+    public const TYPE_REPLACED_DEFAULT_VALUE = 'replace_default_value';
+
+    /**
+     * @var string
+     */
+    private $class;
+
+    /**
+     * @var string
+     */
+    private $method;
+
+    /**
+     * @var int
+     */
+    private $position;
+
+    /**
+     * @var string
+     */
+    private $type;
+
+    /**
+     * @var mixed
+     */
+    private $defaultValue;
+
+    /**
+     * @var string[]
+     */
+    private $replaceMap = [];
+
+    /**
+     * @param mixed $defaultValue
+     * @param string[] $replaceMap
+     */
+    private function __construct(
+        string $class,
+        string $method,
+        int $position,
+        string $type,
+        $defaultValue = null,
+        array $replaceMap
+    ) {
+        $this->class = $class;
+        $this->method = $method;
+        $this->position = $position;
+        $this->type = $type;
+        $this->defaultValue = $defaultValue;
+        $this->replaceMap = $replaceMap;
+    }
+
+    /**
+     * @param mixed[] $data
+     */
+    public static function createFromArray(array $data): self
+    {
+        self::validateArrayData($data);
+
+        return new self(
+            $data['class'],
+            $data['method'],
+            $data['position'],
+            $data['type'],
+            $data['default_value'] ?? null,
+            $data['replace_map'] ?? []
+        );
+    }
+
+    public function getClass(): string
+    {
+        return $this->class;
+    }
+
+    public function getMethod(): string
+    {
+        return $this->method;
+    }
+
+    public function getPosition(): int
+    {
+        return $this->position;
+    }
+
+    public function getType(): string
+    {
+        return $this->type;
+    }
+
+    /**
+     * @return mixed|null
+     */
+    public function getDefaultValue()
+    {
+        return $this->defaultValue;
+    }
+
+    /**
+     * @return string[]
+     */
+    public function getReplaceMap(): array
+    {
+        return $this->replaceMap;
+    }
+
+    /**
+     * @param mixed[] $data
+     */
+    private static function ensureHasKey(array $data, string $key): void
+    {
+        if (isset($data[$key])) {
+            return;
+        }
+
+        throw new InvalidRectorConfigurationException(sprintf(
+            'Configuration for "%s" Rector should have "%s" key, but is missing.',
+            ArgumentReplacerRector::class,
+            $key
+        ));
+    }
+
+    /**
+     * @param mixed[] $data
+     */
+    private static function validateArrayData(array $data): void
+    {
+        self::ensureHasKey($data, 'class');
+        self::ensureHasKey($data, 'class');
+        self::ensureHasKey($data, 'method');
+        self::ensureHasKey($data, 'position');
+        self::ensureHasKey($data, 'type');
+
+        if ($data['type'] === self::TYPE_REPLACED_DEFAULT_VALUE) {
+            self::ensureHasKey($data, 'replace_map');
+        }
+    }
+}

src/config/level/doctrine/dotrine25.yml

@@ -8,7 +8,8 @@ rectors:
                 'em': 'Doctrine\ORM\EntityManagerInterface'
 
     Rector\Rector\Dynamic\ArgumentReplacerRector:
-        'Doctrine\ORM\Persisters\Entity\AbstractEntityInheritancePersister':
-            'getSelectJoinColumnSQL':
-                4:
-                    '~': ~
+        -
+            class: 'Doctrine\ORM\Persisters\Entity\AbstractEntityInheritancePersister'
+            method: 'getSelectJoinColumnSQL'
+            position: 4
+            type: 'removed'

src/config/level/symfony/symfony33.yml

@@ -1,20 +1,25 @@
 rectors:
     # dependency-injection
     Rector\Rector\Dynamic\ArgumentReplacerRector:
-        'Symfony\Component\DependencyInjection\ContainerBuilder':
-            'compile':
-                0:
-                    # added default value
-                    '~': false
-            'addCompilerPass':
-                2:
-                    # added default value
-                    '~': 0
-        'Symfony\Component\DependencyInjection\Compiler\ServiceReferenceGraph':
-            'connect':
-                6:
-                    # added default value
-                    '~': false
+        # added default value
+        -
+            class: 'Symfony\Component\DependencyInjection\ContainerBuilder'
+            method: 'compile'
+            type: 'changed'
+            position: 2
+            default_value: 0
+        -
+            class: 'Symfony\Component\DependencyInjection\ContainerBuilder'
+            method: 'addCompilerPass'
+            type: 'changed'
+            position: 2
+            default_value: 0
+        -
+            class: 'Symfony\Component\DependencyInjection\Compiler\ServiceReferenceGraph'
+            method: 'connect'
+            position: 6
+            type: 'changed'
+            default_value: false
 
     Rector\Rector\Contrib\Symfony\Console\ConsoleExceptionToErrorEventConstantRector: ~
 

tests/Rector/Dynamic/ArgumentReplacerRector/config/rector.yml

@@ -1,23 +1,33 @@
 rectors:
     Rector\Rector\Dynamic\ArgumentReplacerRector:
-        'Symfony\Component\DependencyInjection\ContainerBuilder':
-            'compile':
-                0:
-                    # added default value
-                    '~': false
-            'addCompilerPass':
-                2:
-                    # added default value
-                    '~': 0
+        # added default value
+        -
+            class: 'Symfony\Component\DependencyInjection\ContainerBuilder'
+            method: 'compile'
+            type: 'changed'
+            position: 0
+            default_value: false
 
-        'Doctrine\ORM\Persisters\Entity\AbstractEntityInheritancePersister':
-            'getSelectJoinColumnSQL':
-                4:
-                    # remove completely
-                    '~': ~
+        # added default value
+        -
+            class: 'Symfony\Component\DependencyInjection\ContainerBuilder'
+            method: 'addCompilerPass'
+            type: 'changed'
+            position: 2
+            default_value: 0
 
-        'Symfony\Component\DependencyInjection\Definition':
-            'setScope':
-                0:
-                    # replace by new value
-                    'Symfony\Component\DependencyInjection\ContainerBuilder::SCOPE_PROTOTYPE': false
+        # remove argument
+        -
+            class: 'Doctrine\ORM\Persisters\Entity\AbstractEntityInheritancePersister'
+            method: 'getSelectJoinColumnSQL'
+            position: 4
+            type: 'removed'
+
+        # replace default value
+        -
+            class: 'Symfony\Component\DependencyInjection\Definition'
+            method: 'setScope'
+            position: 0
+            type: 'replace_default_value'
+            replace_map:
+                'Symfony\Component\DependencyInjection\ContainerBuilder::SCOPE_PROTOTYPE': false