[DeadCode] Add RemoveDuplicatedInstanceOfRector (#1603)

[DeadCode] Add RemoveDuplicatedInstanceOfRector

config/set/dead-code/dead-code.yaml

@@ -22,3 +22,4 @@ services:
     Rector\CodeQuality\Rector\Return_\SimplifyUselessVariableRector: ~
     Rector\DeadCode\Rector\Plus\RemoveZeroAndOneBinaryRector: ~
     Rector\DeadCode\Rector\ClassMethod\RemoveDelegatingParentCallRector: ~
+    Rector\DeadCode\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector: ~

packages/DeadCode/src/Rector/Instanceof_/RemoveDuplicatedInstanceOfRector.php

@@ -0,0 +1,154 @@
+<?php declare(strict_types=1);
+
+namespace Rector\DeadCode\Rector\Instanceof_;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\BinaryOp;
+use PhpParser\Node\Expr\Instanceof_;
+use Rector\Rector\AbstractRector;
+use Rector\RectorDefinition\CodeSample;
+use Rector\RectorDefinition\RectorDefinition;
+
+final class RemoveDuplicatedInstanceOfRector extends AbstractRector
+{
+    /**
+     * @var string[]
+     */
+    private $duplicatedInstanceOfs = [];
+
+    public function getDefinition(): RectorDefinition
+    {
+        return new RectorDefinition('', [
+            new CodeSample(
+                <<<'CODE_SAMPLE'
+class SomeClass
+{
+    public function run($value)
+    {
+        $isIt = $value instanceof A || $value instanceof A;
+        $isIt = $value instanceof A && $value instanceof A;
+    }
+}
+CODE_SAMPLE
+                ,
+                <<<'CODE_SAMPLE'
+class SomeClass
+{
+    public function run($value)
+    {
+        $isIt = $value instanceof A;
+        $isIt = $value instanceof A;
+    }
+}
+CODE_SAMPLE
+            ),
+        ]);
+    }
+
+    /**
+     * @return string[]
+     */
+    public function getNodeTypes(): array
+    {
+        return [BinaryOp::class];
+    }
+
+    /**
+     * @param BinaryOp $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        $this->resolveDuplicatedInstancesOf($node);
+        if ($this->duplicatedInstanceOfs === []) {
+            return null;
+        }
+
+        return $this->traverseBinaryOpAndRemoveDuplicatedInstanceOfs($node);
+    }
+
+    private function resolveDuplicatedInstancesOf(Node $node): void
+    {
+        $this->duplicatedInstanceOfs = [];
+
+        /** @var Instanceof_[] $instanceOfs */
+        $instanceOfs = $this->betterNodeFinder->findInstanceOf([$node], Instanceof_::class);
+
+        $instanceOfsByClass = [];
+        foreach ($instanceOfs as $instanceOf) {
+            $variableClassKey = $this->createUniqueKeyForInstanceOf($instanceOf);
+            if ($variableClassKey === null) {
+                continue;
+            }
+
+            $instanceOfsByClass[$variableClassKey][] = $instanceOf;
+        }
+
+        foreach ($instanceOfsByClass as $variableClassKey => $instanceOfs) {
+            if (count($instanceOfs) < 2) {
+                unset($instanceOfsByClass[$variableClassKey]);
+            }
+        }
+
+        $this->duplicatedInstanceOfs = array_keys($instanceOfsByClass);
+    }
+
+    private function createUniqueKeyForInstanceOf(Instanceof_ $instanceof): ?string
+    {
+        if (! $instanceof->expr instanceof Node\Expr\Variable) {
+            return null;
+        }
+        $variableName = $this->getName($instanceof->expr);
+        if ($variableName === null) {
+            return null;
+        }
+
+        $className = $this->getName($instanceof->class);
+        if ($className === null) {
+            return null;
+        }
+
+        return $variableName . '_' . $className;
+    }
+
+    private function traverseBinaryOpAndRemoveDuplicatedInstanceOfs(BinaryOp $binaryOp): Node
+    {
+        $this->traverseNodesWithCallable([&$binaryOp], function (Node &$node): ?Node {
+            if (! $node instanceof BinaryOp) {
+                return null;
+            }
+
+            if ($node->left instanceof Instanceof_) {
+                return $this->processBinaryWithFirstInstaneOf($node->left, $node->right);
+            }
+
+            if ($node->right instanceof Instanceof_) {
+                return $this->processBinaryWithFirstInstaneOf($node->right, $node->left);
+            }
+
+            return null;
+        });
+
+        return $binaryOp;
+    }
+
+    private function removeClassFromDuplicatedInstanceOfs(string $variableClassKey): void
+    {
+        // remove just once
+        unset($this->duplicatedInstanceOfs[array_search($variableClassKey, $this->duplicatedInstanceOfs, true)]);
+    }
+
+    private function processBinaryWithFirstInstaneOf(Instanceof_ $instanceof, Node\Expr $otherExpr): ?Node\Expr
+    {
+        $variableClassKey = $this->createUniqueKeyForInstanceOf($instanceof);
+
+        if (! in_array($variableClassKey, $this->duplicatedInstanceOfs, true)) {
+            return null;
+        }
+
+        // remove just once
+        $this->removeClassFromDuplicatedInstanceOfs($variableClassKey);
+
+        // remove left instanceof
+        return $otherExpr;
+    }
+}

packages/DeadCode/tests/Rector/Instanceof_/RemoveDuplicatedInstanceOfRector/Fixture/fixture.php.inc

@@ -0,0 +1,29 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector\Fixture;
+
+class SomeClass
+{
+    public function run($value)
+    {
+        $isIt = $value instanceof A || $value instanceof A;
+        $isIt = $value instanceof A && $value instanceof A;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector\Fixture;
+
+class SomeClass
+{
+    public function run($value)
+    {
+        $isIt = $value instanceof A;
+        $isIt = $value instanceof A;
+    }
+}
+
+?>

packages/DeadCode/tests/Rector/Instanceof_/RemoveDuplicatedInstanceOfRector/Fixture/jump_around_one.php.inc

@@ -0,0 +1,27 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector\Fixture;
+
+class JumpAroundOne
+{
+    public function run($value)
+    {
+        $isIt = $value instanceof A && $value instanceof B && $value instanceof A;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector\Fixture;
+
+class JumpAroundOne
+{
+    public function run($value)
+    {
+        $isIt = $value instanceof A && $value instanceof B;
+    }
+}
+
+?>

packages/DeadCode/tests/Rector/Instanceof_/RemoveDuplicatedInstanceOfRector/Fixture/nested.php.inc

@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector\Fixture;
+
+class Nested
+{
+    public function run($value)
+    {
+        if ($value instanceof A && $value instanceof B) {
+            if ($value instanceof B && $value instanceof A && $value instanceof A) {
+                if ($value = 5) {
+                    return $value instanceof B && $value instanceof A;
+                }
+            }
+        }
+
+        return false;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector\Fixture;
+
+class Nested
+{
+    public function run($value)
+    {
+        if ($value instanceof A && $value instanceof B) {
+            if ($value instanceof B && $value instanceof A) {
+                if ($value = 5) {
+                    return $value instanceof B && $value instanceof A;
+                }
+            }
+        }
+
+        return false;
+    }
+}
+
+?>

packages/DeadCode/tests/Rector/Instanceof_/RemoveDuplicatedInstanceOfRector/Fixture/skip_multiple_variables.php.inc

@@ -0,0 +1,11 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector\Fixture;
+
+class SkipMultipleVariables
+{
+    public function run($value, $value2)
+    {
+        return $value instanceof A || $value2 instanceof A;
+    }
+}

packages/DeadCode/tests/Rector/Instanceof_/RemoveDuplicatedInstanceOfRector/RemoveDuplicatedInstanceOfRectorTest.php

@@ -0,0 +1,24 @@
+<?php declare(strict_types=1);
+
+namespace Rector\DeadCode\Tests\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector;
+
+use Rector\DeadCode\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class RemoveDuplicatedInstanceOfRectorTest extends AbstractRectorTestCase
+{
+    public function test(): void
+    {
+        $this->doTestFiles([
+            __DIR__ . '/Fixture/fixture.php.inc',
+            __DIR__ . '/Fixture/jump_around_one.php.inc',
+            __DIR__ . '/Fixture/nested.php.inc',
+            __DIR__ . '/Fixture/skip_multiple_variables.php.inc',
+        ]);
+    }
+
+    protected function getRectorClass(): string
+    {
+        return RemoveDuplicatedInstanceOfRector::class;
+    }
+}