Merge pull request #1391 from infection/replace-coalesce-mutator

Remove redundant Coalesce Mutator and rename FlipCoalesce to Coalesce

resources/schema.json

@@ -248,7 +248,6 @@
                 "Throw_": { "$ref": "#/definitions/default-mutator-config" },
                 "Finally_": { "$ref": "#/definitions/default-mutator-config" },
                 "Coalesce": { "$ref": "#/definitions/default-mutator-config" },
-                "FlipCoalesce": { "$ref": "#/definitions/default-mutator-config" },
                 "PregQuote": { "$ref": "#/definitions/default-mutator-config" },
                 "PregMatchMatches": { "$ref": "#/definitions/default-mutator-config" },
                 "ArrayItemRemoval": {

src/Mutator/Operator/Coalesce.php

@@ -52,20 +52,11 @@ public static function getDefinition(): ?Definition
     {
         return new Definition(
             <<<'TXT'
-Replaces a null coalescing operator (`??`) with its second operand. For example:
-
-```php
-$x = $y ?? $z;
-```
-
-Will be mutated to:
-
-```php
-$x = $z;
-```
+Swaps the coalesce operator (`??`) operands,
+e.g. replaces `$a ?? $b` with `$b ?? $a` or `$a ?? $b ?? $c` with `$b ?? $a ?? $c` and `$a ?? $c ?? $b`.
 TXT
             ,
-            MutatorCategory::SEMANTIC_ADDITION,
+            MutatorCategory::ORTHOGONAL_REPLACEMENT,
             null
         );
     }
@@ -79,11 +70,21 @@ public static function getDefinition(): ?Definition
      */
     public function mutate(Node $node): iterable
     {
-        yield $node->right;
+        $left = $node->left;
+        $right = $node->right;
+
+        if ($right instanceof Node\Expr\BinaryOp\Coalesce) {
+            $left = new Node\Expr\BinaryOp\Coalesce($node->left, $right->right, $right->getAttributes());
+            $right = $right->left;
+        }
+
+        yield new Node\Expr\BinaryOp\Coalesce($right, $left, $node->getAttributes());
     }
 
     public function canMutate(Node $node): bool
     {
-        return $node instanceof Node\Expr\BinaryOp\Coalesce;
+        return $node instanceof Node\Expr\BinaryOp\Coalesce
+            && !$node->left instanceof Node\Expr\ConstFetch
+            && !$node->left instanceof Node\Expr\ClassConstFetch;
     }
 }

src/Mutator/ProfileList.php

@@ -156,7 +156,6 @@ final class ProfileList
         Mutator\Operator\Coalesce::class,
         Mutator\Operator\Continue_::class,
         Mutator\Operator\Finally_::class,
-        Mutator\Operator\FlipCoalesce::class,
         Mutator\Operator\Spread::class,
         Mutator\Operator\Ternary::class,
         Mutator\Operator\Throw_::class,
@@ -346,7 +345,6 @@ final class ProfileList
         'Coalesce' => Mutator\Operator\Coalesce::class,
         'Continue_' => Mutator\Operator\Continue_::class,
         'Finally_' => Mutator\Operator\Finally_::class,
-        'FlipCoalesce' => Mutator\Operator\FlipCoalesce::class,
         'Spread' => Mutator\Operator\Spread::class,
         'Ternary' => Mutator\Operator\Ternary::class,
         'Throw_' => Mutator\Operator\Throw_::class,

tests/phpunit/Mutator/Operator/CoalesceTest.php

@@ -51,93 +51,116 @@ public function test_it_can_mutate(string $input, $expected = []): void
 
     public function mutationsProvider(): iterable
     {
-        yield 'Mutate coalesce with scalar values' => [
-            <<<PHP
+        yield 'Mutate coalesce and flip operands' => [
+            <<<'PHP'
 <?php
 
-'value' ?? 'otherValue';
+$foo = 'foo';
+$bar = 'bar';
+$foo ?? $bar;
 PHP
             ,
-            <<<PHP
+            <<<'PHP'
 <?php
 
-'otherValue';
+$foo = 'foo';
+$bar = 'bar';
+$bar ?? $foo;
 PHP
         ];
 
-        yield 'Mutate coalesce when left argument is variable' => [
+        yield 'Mutate more than one coalesce operators and flip operands' => [
             <<<'PHP'
 <?php
 
-$foo = 'value';
-$foo ?? 'otherValue';
+$foo = 'foo';
+$bar = 'bar';
+$baz = 'baz';
+$foo ?? $bar ?? $baz;
 PHP
             ,
-            <<<'PHP'
-<?php
-
-$foo = 'value';
-'otherValue';
-PHP
-        ];
-
-        yield 'Mutate coalesce with expression' => [
-            <<<PHP
+            [
+                <<<'PHP'
 <?php
 
-'value' . 'withConcat' ?? 'otherValue';
+$foo = 'foo';
+$bar = 'bar';
+$baz = 'baz';
+$foo ?? $baz ?? $bar;
 PHP
-            ,
-            <<<PHP
+                ,
+                <<<'PHP'
 <?php
 
-'otherValue';
+$foo = 'foo';
+$bar = 'bar';
+$baz = 'baz';
+$bar ?? $foo ?? $baz;
 PHP
+            ],
         ];
 
-        yield 'Mutate coalesce with expression as second param' => [
-            <<<PHP
+        yield 'Mutate more than two coalesce operators and flip operands' => [
+            <<<'PHP'
 <?php
 
-'value' ?? 'value' . 'withConcat';
+$foo = 'foo';
+$bar = 'bar';
+$baz = 'baz';
+$foo ?? $bar ?? $baz ?? 'oof';
 PHP
             ,
-            <<<PHP
+            [
+                <<<'PHP'
 <?php
 
-'value' . 'withConcat';
+$foo = 'foo';
+$bar = 'bar';
+$baz = 'baz';
+$foo ?? $bar ?? 'oof' ?? $baz;
 PHP
-        ];
-
-        yield 'Mutate coalesce with variable as second argument' => [
-            <<<'PHP'
+                ,
+                <<<'PHP'
 <?php
 
-$foo = 5;
-'value' ?? $foo;
+$foo = 'foo';
+$bar = 'bar';
+$baz = 'baz';
+$foo ?? $baz ?? $bar ?? 'oof';
 PHP
-            ,
-            <<<'PHP'
+                ,
+                <<<'PHP'
 <?php
 
-$foo = 5;
-$foo;
+$foo = 'foo';
+$bar = 'bar';
+$baz = 'baz';
+$bar ?? $foo ?? $baz ?? 'oof';
 PHP
+            ],
         ];
 
-        yield 'Mutate coalesce with constants in a conditional' => [
+        yield 'It does not mutate when left operator is constant defined through `define` function' => [
             <<<'PHP'
 <?php
 
-if ('value' ?? 5) {
-}
+define('FOO', 'foo');
+FOO ?? 'bar';
 PHP
-            ,
+        ];
+
+        yield 'It does not mutate when left operator is constant defined in class' => [
             <<<'PHP'
 <?php
 
-if (5) {
-}
+new class {
+    private const FOO = 'foo';
+
+    public function getFoo(): string
+    {
+        return self::FOO ?? 'bar';
+    }
+};
 PHP
         ];
     }