Narrow ArraySpreadInsteadOfArrayMergeRector to numeric-arrays only

composer.json

@@ -49,7 +49,6 @@
             "Rector\\Symfony\\": "packages/Symfony/src",
             "Rector\\CakePHP\\": "packages/CakePHP/src",
             "Rector\\Php\\": "packages/Php/src",
-            "Rector\\Jms\\": "packages/Jms/src",
             "Rector\\RemovingStatic\\": "packages/RemovingStatic/src",
             "Rector\\Silverstripe\\": "packages/Silverstripe/src",
             "Rector\\Sensio\\": "packages/Sensio/src",
@@ -85,7 +84,6 @@
             "Rector\\DomainDrivenDesign\\Tests\\": "packages/DomainDrivenDesign/tests",
             "Rector\\Guzzle\\Tests\\": "packages/Guzzle/tests",
             "Rector\\Php\\Tests\\": "packages/Php/tests",
-            "Rector\\Jms\\Tests\\": "packages/Jms/tests",
             "Rector\\RemovingStatic\\Tests\\": "packages/RemovingStatic/tests",
             "Rector\\Symfony\\Tests\\": "packages/Symfony/tests",
             "Rector\\Silverstripe\\Tests\\": "packages/Silverstripe/tests",

packages/Php/src/Rector/FuncCall/ArraySpreadInsteadOfArrayMergeRector.php

@@ -9,20 +9,25 @@
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\Ternary;
 use PhpParser\Node\Expr\Variable;
+use PHPStan\Type\Constant\ConstantArrayType;
+use PHPStan\Type\Constant\ConstantStringType;
 use Rector\Rector\AbstractRector;
 use Rector\RectorDefinition\CodeSample;
 use Rector\RectorDefinition\RectorDefinition;
 
 /**
  * @see https://wiki.php.net/rfc/spread_operator_for_array
+ * @see https://twitter.com/nikita_ppv/status/1126470222838366209
  */
 final class ArraySpreadInsteadOfArrayMergeRector extends AbstractRector
 {
     public function getDefinition(): RectorDefinition
     {
-        return new RectorDefinition('Change array_merge() to spread operator', [
-            new CodeSample(
-                <<<'CODE_SAMPLE'
+        return new RectorDefinition(
+            'Change array_merge() to spread operator, except values with possible string key values',
+            [
+                new CodeSample(
+                    <<<'CODE_SAMPLE'
 class SomeClass
 {
     public function run($iter1, $iter2)
@@ -37,21 +42,22 @@ public function run($iter1, $iter2)
     }
 }
 CODE_SAMPLE
-                ,
-                <<<'CODE_SAMPLE'
+                    ,
+                    <<<'CODE_SAMPLE'
 class SomeClass
 {
     public function run($iter1, $iter2)
     {
-        $values = [ ...$iter1, ...$iter2 ];
+        $values = [...$iter1, ...$iter2];
 
         // Or to generalize to all iterables
-        $anotherValues = [ ...$iter1, ...$iter2 ];
+        $anotherValues = [...$iter1, ...$iter2];
     }
 }
 CODE_SAMPLE
-            ),
-        ]);
+                ),
+            ]
+        );
     }
 
     /**
@@ -111,12 +117,17 @@ private function resolveValue(Expr $expr): Expr
         return $expr;
     }
 
-    private function refactorArray(Node $node): Array_
+    private function refactorArray(FuncCall $funcCall): ?Array_
     {
         $array = new Array_();
 
-        foreach ($node->args as $arg) {
+        foreach ($funcCall->args as $arg) {
             $value = $arg->value;
+
+            if ($this->shouldSkipArrayForInvalidTypeOrKeys($value)) {
+                return null;
+            }
+
             $value = $this->resolveValue($value);
 
             $array->items[] = $this->createUnpackedArrayItem($value);
@@ -137,4 +148,24 @@ private function createUnpackedArrayItem(Expr $expr): ArrayItem
     {
         return new ArrayItem($expr, null, false, [], true);
     }
+
+    private function shouldSkipArrayForInvalidTypeOrKeys(Expr $expr): bool
+    {
+        // we have no idea what it is → cannot change it
+        if (! $this->isArrayType($expr)) {
+            return true;
+        }
+
+        $arrayStaticType = $this->getStaticType($expr);
+        if ($arrayStaticType instanceof ConstantArrayType) {
+            foreach ($arrayStaticType->getKeyTypes() as $keyType) {
+                // key cannot be string
+                if ($keyType instanceof ConstantStringType) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
 }

packages/Php/tests/Rector/FuncCall/ArraySpreadInsteadOfArrayMergeRector/ArraySpreadInsteadOfArrayMergeRectorTest.php

@@ -16,7 +16,10 @@ public function test(): void
         $this->doTestFiles([
             __DIR__ . '/Fixture/fixture.php.inc',
             __DIR__ . '/Fixture/iterator_to_array.php.inc',
-            __DIR__ . '/Fixture/simple_array_merge.php.inc',
+            __DIR__ . '/Fixture/integer_keys.php.inc',
+            // see caveat: https://twitter.com/nikita_ppv/status/1126470222838366209
+            __DIR__ . '/Fixture/skip_simple_array_merge.php.inc',
+            __DIR__ . '/Fixture/skip_string_keys.php.inc',
         ]);
     }
 

packages/Php/tests/Rector/FuncCall/ArraySpreadInsteadOfArrayMergeRector/Fixture/integer_keys.php.inc

@@ -0,0 +1,33 @@
+<?php
+
+namespace Rector\Php\Tests\Rector\FuncCall\ArraySpreadInsteadOfArrayMergeRector\Fixture;
+
+class IntegerKeys
+{
+    public function run()
+    {
+        $iter1 = [0 => 'two'];
+        $iter2 = [1 => 'four'];
+
+        return array_merge($iter1, $iter2);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Php\Tests\Rector\FuncCall\ArraySpreadInsteadOfArrayMergeRector\Fixture;
+
+class IntegerKeys
+{
+    public function run()
+    {
+        $iter1 = [0 => 'two'];
+        $iter2 = [1 => 'four'];
+
+        return [...$iter1, ...$iter2];
+    }
+}
+
+?>

packages/Php/tests/Rector/FuncCall/ArraySpreadInsteadOfArrayMergeRector/Fixture/skip_simple_array_merge.php.inc

@@ -0,0 +1,11 @@
+<?php
+
+namespace Rector\Php\Tests\Rector\FuncCall\ArraySpreadInsteadOfArrayMergeRector\Fixture;
+
+class SkipSimpleArrayMerge
+{
+    public function run($iter1, $iter2)
+    {
+        $values = array_merge($iter1, $iter2);
+    }
+}

packages/Php/tests/Rector/FuncCall/ArraySpreadInsteadOfArrayMergeRector/Fixture/skip_string_keys.php.inc

@@ -0,0 +1,22 @@
+<?php
+
+namespace Rector\Php\Tests\Rector\FuncCall\ArraySpreadInsteadOfArrayMergeRector\Fixture;
+
+class SkipStringKeys
+{
+    public function run()
+    {
+        $iter1 = ['one' => 'two'];
+        $iter2 = ['three' => 'four'];
+
+        return array_merge($iter1, $iter2);
+    }
+
+    public function go()
+    {
+        $iter1 = [1 => 'two'];
+        $iter2 = ['three' => 'four'];
+
+        return array_merge($iter1, $iter2);
+    }
+}

phpstan.neon

@@ -77,7 +77,6 @@ parameters:
 
         # false positive, resolved in previous method
         - '#Parameter (.*?) of method Rector\\PhpParser\\Node\\Manipulator\\IdentifierManipulator\:\:(.*?)\(\) expects PhpParser\\Node\\Expr\\ClassConstFetch\|PhpParser\\Node\\Expr\\MethodCall\|PhpParser\\Node\\Expr\\PropertyFetch\|PhpParser\\Node\\Expr\\StaticCall\|PhpParser\\Node\\Stmt\\ClassMethod, PhpParser\\Node given#'
-        - '#Parameter \#1 \$callables of method Rector\\Collector\\CallableCollectorPopulator::populate\(\) expects (.*?) given#'
 
         # intentionally incorrect - part of the test
         - '#Parameter \#2 \$codeSamples of class Rector\\RectorDefinition\\RectorDefinition constructor expects array<Rector\\Contract\\RectorDefinition\\CodeSampleInterface>, array<int, stdClass> given#'
@@ -150,12 +149,10 @@ parameters:
         - '#Method Rector\\NodeContainer\\ParsedNodesByType\:\:(.*?)\(\) should return PhpParser\\Node\\Stmt\\(.*?)\|null but returns PhpParser\\Node\|null#'
         - '#Method Rector\\NodeContainer\\ParsedNodesByType\:\:findImplementersOfInterface\(\) should return array<PhpParser\\Node\\Stmt\\Interface_\> but returns array<int, PhpParser\\Node\>#'
         - '#PHPDoc tag @param for parameter \$classLike with type PhpParser\\Builder\\Trait_\|PhpParser\\Node\\Stmt\\Interface_ is not subtype of native type PhpParser\\Node\\Stmt\\ClassLike#'
-        - '#Method Rector\\CodingStyle\\Rector\\Namespace_\\ImportFullyQualifiedNamesRector\:\:getShortName\(\) should return string but returns string\|false#'
         - '#Access to an undefined property PhpParser\\Node\\Expr\\Error\|PhpParser\\Node\\Expr\\Variable\:\:\$name#'
         - '#Empty array passed to foreach#'
         - '#Method Rector\\RemovingStatic\\UniqueObjectFactoryFactory\:\:resolveClassShortName\(\) should return string but returns string\|false#'
         - '#Strict comparison using \=\=\= between PhpParser\\Node\\Expr\\ArrayItem and null will always evaluate to false#'
-        - '#Anonymous function should have native typehint "string"#'
         - '#Parameter \#2 \.\.\.\$args of function array_merge expects array, array<int, string\>\|false given#'
         - '#Method Rector\\Collector\\CallableCollectorPopulator\:\:populate\(\) should return array<Closure\> but returns array<int\|string, callable\>#'
         - '#Access to an undefined property PhpParser\\Node\\Expr\:\:\$args#'