Resolve sprintf pre-assign case for StringToArrayArgumentProcessRector

ecs.yml

@@ -37,6 +37,7 @@ services:
             - 'PhpParser\NodeVisitor\NameResolver'
             - 'Rector\Application\Error'
             - 'Rector\DependencyInjection\Loader\*'
+            - 'Symfony\Component\Console\Input\StringInput'
 
     Symplify\CodingStandard\Fixer\Naming\PropertyNameMatchingTypeFixer:
         extra_skipped_classes:
@@ -84,6 +85,7 @@ parameters:
             - 'src/Php/TypeAnalyzer.php'
             # exclusive static config for type support
             - 'src/Php/PhpTypeSupport.php'
+            - 'src/Util/RectorStrings.php'
 
         Symplify\CodingStandard\Fixer\Naming\PropertyNameMatchingTypeFixer:
             - 'packages/NodeTypeResolver/src/PHPStan/Scope/NodeScopeResolver.php'

packages/Symfony/src/Rector/New_/StringToArrayArgumentProcessRector.php

@@ -2,15 +2,21 @@
 
 namespace Rector\Symfony\Rector\New_;
 
-use Nette\Utils\Strings;
 use PhpParser\Node;
+use PhpParser\Node\Arg;
 use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\BinaryOp\Concat;
+use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\New_;
 use PhpParser\Node\Scalar\String_;
+use Rector\PhpParser\Node\BetterNodeFinder;
+use Rector\PhpParser\NodeTransformer;
 use Rector\Rector\AbstractRector;
 use Rector\RectorDefinition\CodeSample;
 use Rector\RectorDefinition\RectorDefinition;
+use Rector\Util\RectorStrings;
 
 /**
  * @see https://github.com/symfony/symfony/pull/27821/files
@@ -27,10 +33,24 @@ final class StringToArrayArgumentProcessRector extends AbstractRector
      */
     private $processHelperClass;
 
+    /**
+     * @var BetterNodeFinder
+     */
+    private $betterNodeFinder;
+
+    /**
+     * @var NodeTransformer
+     */
+    private $nodeTransformer;
+
     public function __construct(
+        BetterNodeFinder $betterNodeFinder,
+        NodeTransformer $nodeTransformer,
         string $processClass = 'Symfony\Component\Process\Process',
         string $processHelperClass = 'Symfony\Component\Console\Helper\ProcessHelper'
     ) {
+        $this->betterNodeFinder = $betterNodeFinder;
+        $this->nodeTransformer = $nodeTransformer;
         $this->processClass = $processClass;
         $this->processHelperClass = $processHelperClass;
     }
@@ -90,13 +110,61 @@ CODE_SAMPLE
             return null;
         }
 
-        if (! $firstArgument instanceof String_) {
-            return null;
+        if ($firstArgument instanceof String_) {
+            $parts = RectorStrings::splitCommandToItems($firstArgument->value);
+            $node->args[$argumentPosition]->value = $this->createArray($parts);
         }
 
-        $parts = Strings::split($firstArgument->value, '# #');
-        $node->args[$argumentPosition]->value = $this->createArray($parts);
+        // type analyzer
+        if ($this->isStringType($firstArgument)) {
+            $this->processStringType($node, $argumentPosition, $firstArgument);
+        }
 
         return $node;
     }
+
+    private function findPreviousNodeAssign(Node $node, Node $firstArgument): ?Assign
+    {
+        return $this->betterNodeFinder->findFirstPrevious($node, function (Node $checkedNode) use ($firstArgument) {
+            if (! $checkedNode instanceof Assign) {
+                return null;
+            }
+
+            if (! $this->areNodesEqual($checkedNode->var, $firstArgument)) {
+                return null;
+            }
+
+            // @todo check out of scope assign, e.g. in previous method
+
+            return $checkedNode;
+        });
+    }
+
+    /**
+     * @param New_|MethodCall $node
+     */
+    private function processStringType(Node $node, int $argumentPosition, Node $firstArgument): void
+    {
+        if ($firstArgument instanceof Concat) {
+            $arrayNode = $this->nodeTransformer->transformConcatToStringArray($firstArgument);
+            if ($arrayNode) {
+                $node->args[$argumentPosition] = new Arg($arrayNode);
+            }
+        }
+
+        /** @var Assign|null $createdNode */
+        $createdNode = $this->findPreviousNodeAssign($node, $firstArgument);
+        if ($createdNode === null) {
+            return;
+        }
+
+        if (! $createdNode->expr instanceof FuncCall || ! $this->isName($createdNode->expr, 'sprintf')) {
+            return;
+        }
+
+        $arrayNode = $this->nodeTransformer->transformSprintfToArray($createdNode->expr);
+        if ($arrayNode) {
+            $createdNode->expr = $arrayNode;
+        }
+    }
 }

packages/Symfony/tests/Rector/New_/StringToArrayArgumentProcessRector/Fixture/fixture.php.inc

@@ -1,10 +1,19 @@
 <?php
 
+namespace Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Fixture;
+
 use Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Source\Process;
 
 function stringToArgumentArray()
 {
     $process = new Process('ls -l');
+    $process = new Process('ls  -l');
+
+    $commitHash = 'abc';
+    $process = new Process('sleep ' . (5 / 1000));
+    $process = new Process('git describe --contains ' . $commitHash);
+    $process = new Process($commitHash . 'git describe --contains');
+    $process = new Process($commitHash . 'git describe --contains' . $commitHash);
 
     $process = new Process(['ls', '-l']);
 
@@ -16,11 +25,20 @@ function stringToArgumentArray()
 -----
 <?php
 
+namespace Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Fixture;
+
 use Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Source\Process;
 
 function stringToArgumentArray()
 {
     $process = new Process(['ls', '-l']);
+    $process = new Process(['ls', '-l']);
+
+    $commitHash = 'abc';
+    $process = new Process(['sleep', 5 / 1000]);
+    $process = new Process(['git', 'describe', '--contains', $commitHash]);
+    $process = new Process([$commitHash, 'git', 'describe', '--contains']);
+    $process = new Process([$commitHash, 'git', 'describe', '--contains', $commitHash]);
 
     $process = new Process(['ls', '-l']);
 

packages/Symfony/tests/Rector/New_/StringToArrayArgumentProcessRector/Fixture/fixture3.php.inc

@@ -0,0 +1,34 @@
+<?php
+
+use Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Source\Process;
+
+function stringToArgumentArray3()
+{
+    $source = '/src';
+
+    $command = sprintf(
+        '%s check %s --config %s --fix',
+        ECS_BIN_PATH,
+        implode(' ', $source),
+        ECS_AFTER_RECTOR_CONFIG
+    );
+
+    $process = new Process($command);
+}
+
+?>
+-----
+<?php
+
+use Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Source\Process;
+
+function stringToArgumentArray3()
+{
+    $source = '/src';
+
+    $command = [ECS_BIN_PATH, 'check', implode(' ', $source), '--config', ECS_AFTER_RECTOR_CONFIG, '--fix'];
+
+    $process = new Process($command);
+}
+
+?>

packages/Symfony/tests/Rector/New_/StringToArrayArgumentProcessRector/Fixture/fixture4.php.inc

@@ -0,0 +1,25 @@
+<?php
+
+namespace Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Fixture;
+
+use Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Source\Process;
+
+function stringToArgumentArray4()
+{
+    $process = new Process('git log --tags --simplify-by-decoration --pretty="format:%ai %d"');
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Fixture;
+
+use Rector\Symfony\Tests\Rector\New_\StringToArrayArgumentProcessRector\Source\Process;
+
+function stringToArgumentArray4()
+{
+    $process = new Process(['git', 'log', '--tags', '--simplify-by-decoration', '--pretty=format:%ai %d']);
+}
+
+?>

packages/Symfony/tests/Rector/New_/StringToArrayArgumentProcessRector/StringToArrayArgumentProcessRectorTest.php

@@ -11,7 +11,12 @@ final class StringToArrayArgumentProcessRectorTest extends AbstractRectorTestCas
 {
     public function test(): void
     {
-        $this->doTestFiles([__DIR__ . '/Fixture/fixture.php.inc', __DIR__ . '/Fixture/fixture2.php.inc']);
+        $this->doTestFiles([
+            __DIR__ . '/Fixture/fixture.php.inc',
+            __DIR__ . '/Fixture/fixture2.php.inc',
+            __DIR__ . '/Fixture/fixture3.php.inc',
+            __DIR__ . '/Fixture/fixture4.php.inc',
+        ]);
     }
 
     protected function getRectorClass(): string

phpstan.neon

@@ -89,6 +89,7 @@ parameters:
         # known values
         - '#Access to an undefined property PHPStan\\PhpDocParser\\Ast\\Node::\$name#' # 2
         - '#Cannot access property \$value on PhpParser\\Node\\Expr\\ArrayItem\|null#'
+        - '#Method Rector\\Symfony\\Rector\\New_\\StringToArrayArgumentProcessRector::findPreviousNodeAssign\(\) should return PhpParser\\Node\\Expr\\Assign\|null but returns PhpParser\\Node\|null#'
 
         # use of 3rd party factory that returns general type
         - '#Method Rector\\Node\\NodeFactory::(.*?)\(\) should return PhpParser\\Node\\(.*?) but returns PhpParser\\Node(.*?)#' # 1

src/PhpParser/NodeTransformer.php

@@ -0,0 +1,113 @@
+<?php declare(strict_types=1);
+
+namespace Rector\PhpParser;
+
+use Nette\Utils\Strings;
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\BinaryOp\Concat;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Scalar\String_;
+
+final class NodeTransformer
+{
+    /**
+     * From:
+     * - sprintf("Hi %s", $name);
+     *
+     * to:
+     * - ["Hi %s", $name]
+     */
+    public function transformSprintfToArray(FuncCall $sprintfFuncCall): ?Array_
+    {
+        [$arrayItems, $stringArgument] = $this->splitMessageAndArgs($sprintfFuncCall);
+        if (! $stringArgument instanceof String_) {
+            // we need to know "%x" parts → nothing we can do
+            return null;
+        }
+
+        $message = $stringArgument->value;
+        $messageParts = $this->splitBySpace($message);
+
+        foreach ($messageParts as $key => $messagePart) {
+            // is mask
+            if (Strings::match($messagePart, '#^%\w$#')) {
+                $messageParts[$key] = array_shift($arrayItems);
+            } else {
+                $messageParts[$key] = new String_($messagePart);
+            }
+        }
+
+        return new Array_($messageParts);
+    }
+
+    public function transformConcatToStringArray(Concat $concatNode): ?Array_
+    {
+        $arrayItems = $this->transformConcatToItems($concatNode);
+
+        return new Array_($arrayItems);
+    }
+
+    /**
+     * @return Node[]|null[]
+     */
+    private function splitMessageAndArgs(FuncCall $sprintfFuncCall): array
+    {
+        $stringArgument = null;
+        $arrayItems = [];
+        foreach ($sprintfFuncCall->args as $i => $arg) {
+            if ($i === 0) {
+                $stringArgument = $arg->value;
+            } else {
+                $arrayItems[] = $arg->value;
+            }
+        }
+
+        return [$arrayItems, $stringArgument];
+    }
+
+    /**
+     * @return Node[]|string[]
+     */
+    private function transformConcatItemToArrayItems(Expr $node): array
+    {
+        if ($node instanceof Concat) {
+            return $this->transformConcatToItems($node);
+        }
+
+        if (! $node instanceof String_) {
+            return [$node];
+        }
+
+        $arrayItems = [];
+        $parts = $this->splitBySpace($node->value);
+        foreach ($parts as $part) {
+            if (trim($part)) {
+                $arrayItems[] = new String_($part);
+            }
+        }
+
+        return $arrayItems;
+    }
+
+    /**
+     * @return mixed[]
+     */
+    private function transformConcatToItems(Concat $concatNode): array
+    {
+        $arrayItems = $this->transformConcatItemToArrayItems($concatNode->left);
+
+        return array_merge($arrayItems, $this->transformConcatItemToArrayItems($concatNode->right));
+    }
+
+    /**
+     * @return string[]
+     */
+    private function splitBySpace(string $value): array
+    {
+        $value = str_getcsv($value, ' ');
+
+        return array_filter($value);
+    }
+}

src/Rector/NodeFactoryTrait.php

@@ -79,7 +79,7 @@ trait NodeFactoryTrait
     }
 
     /**
-     * @param Node[] $nodes
+     * @param Node[]|mixed[] $nodes
      */
     protected function createArray(array $nodes): Array_
     {

src/Util/RectorStrings.php

@@ -0,0 +1,19 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Util;
+
+use Symfony\Component\Console\Input\StringInput;
+use Symplify\PackageBuilder\Reflection\PrivatesCaller;
+
+final class RectorStrings
+{
+    /**
+     * @return string[]
+     */
+    public static function splitCommandToItems(string $command): array
+    {
+        $privatesCaller = new PrivatesCaller();
+
+        return $privatesCaller->callPrivateMethod(new StringInput(''), 'tokenize', $command);
+    }
+}