Merge InArrayAndArrayKeysToArrayKeyExistsRector to ArrayKeysAndInArrayToArrayKeyExistsRector with almost identical behavior (#2047)

build/target-repository/docs/rector_rules_overview.md

@@ -1,4 +1,4 @@
-# 510 Rules Overview
+# 508 Rules Overview
 
 <br>
 
@@ -6,7 +6,7 @@
 
 - [Arguments](#arguments) (5)
 
-- [CodeQuality](#codequality) (72)
+- [CodeQuality](#codequality) (70)
 
 - [CodingStyle](#codingstyle) (35)
 
@@ -354,14 +354,11 @@ Replace `array_keys()` and `in_array()` to `array_key_exists()`
 - class: [`Rector\CodeQuality\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector`](../rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php)
 
 ```diff
- class SomeClass
+ function run($packageName, $values)
  {
-     public function run($packageName, $values)
-     {
--        $keys = array_keys($values);
--        return in_array($packageName, $keys, true);
-+        return array_key_exists($packageName, $values);
-     }
+-    $keys = array_keys($values);
+-    return in_array($packageName, $keys, true);
++    return array_key_exists($packageName, $values);
  }
 ```
 
@@ -855,19 +852,6 @@ Changes comparison with get_class to instanceof
 
 <br>
 
-### InArrayAndArrayKeysToArrayKeyExistsRector
-
-Simplify `in_array` and `array_keys` functions combination into `array_key_exists` when `array_keys` has one argument only
-
-- class: [`Rector\CodeQuality\Rector\FuncCall\InArrayAndArrayKeysToArrayKeyExistsRector`](../rules/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php)
-
-```diff
--in_array("key", array_keys($array), true);
-+array_key_exists("key", $array);
-```
-
-<br>
-
 ### InlineConstructorDefaultToPropertyRector
 
 Move property default from constructor to property default
@@ -1243,26 +1227,6 @@ Simplify negated conditions with de Morgan theorem
 
 <br>
 
-### SimplifyDuplicatedTernaryRector
-
-Remove ternary that duplicated return value of true : false
-
-- class: [`Rector\CodeQuality\Rector\Ternary\SimplifyDuplicatedTernaryRector`](../rules/CodeQuality/Rector/Ternary/SimplifyDuplicatedTernaryRector.php)
-
-```diff
- class SomeClass
- {
-     public function run(bool $value, string $name)
-     {
--         $isTrue = $value ? true : false;
-+         $isTrue = $value;
-          $isName = $name ? true : false;
-     }
- }
-```
-
-<br>
-
 ### SimplifyEmptyArrayCheckRector
 
 Simplify `is_array` and `empty` functions combination into a simple identical check for an empty array
@@ -1641,7 +1605,7 @@ When throwing into a catch block, checks that the previous exception is passed t
 
 ### UnnecessaryTernaryExpressionRector
 
-Remove unnecessary ternary expressions.
+Remove unnecessary ternary expressions
 
 - class: [`Rector\CodeQuality\Rector\Ternary\UnnecessaryTernaryExpressionRector`](../rules/CodeQuality/Rector/Ternary/UnnecessaryTernaryExpressionRector.php)
 

config/set/code-quality.php

@@ -30,7 +30,6 @@ use Rector\CodeQuality\Rector\FuncCall\ArrayMergeOfNonArraysToSimpleArrayRector;
 use Rector\CodeQuality\Rector\FuncCall\CallUserFuncWithArrowFunctionToInlineRector;
 use Rector\CodeQuality\Rector\FuncCall\ChangeArrayPushToArrayAssignRector;
 use Rector\CodeQuality\Rector\FuncCall\CompactToVariablesRector;
-use Rector\CodeQuality\Rector\FuncCall\InArrayAndArrayKeysToArrayKeyExistsRector;
 use Rector\CodeQuality\Rector\FuncCall\IntvalToTypeCastRector;
 use Rector\CodeQuality\Rector\FuncCall\IsAWithStringWithThirdArgumentRector;
 use Rector\CodeQuality\Rector\FuncCall\RemoveSoleValueSprintfRector;
@@ -86,7 +85,6 @@ return static function (ContainerConfigurator $containerConfigurator): void {
     $services->set(ReplaceMultipleBooleanNotRector::class);
     $services->set(ForeachToInArrayRector::class);
     $services->set(SimplifyForeachToCoalescingRector::class);
-    $services->set(InArrayAndArrayKeysToArrayKeyExistsRector::class);
     $services->set(SimplifyFuncGetArgsCountRector::class);
     $services->set(SimplifyInArrayValuesRector::class);
     $services->set(SimplifyStrposLowerRector::class);

rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/another_in_array.php.inc

@@ -0,0 +1,33 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector\Fixture;
+
+final class AnotherInArray
+{
+    public function resultIntoAVariable(): void
+    {
+        $array = ['foo', 'bar'];
+        $key = 'key';
+
+        $result = in_array($key, array_keys($array), true);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector\Fixture;
+
+final class AnotherInArray
+{
+    public function resultIntoAVariable(): void
+    {
+        $array = ['foo', 'bar'];
+        $key = 'key';
+
+        $result = array_key_exists($key, $array);
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/just_called_array_keys.php.inc

@@ -0,0 +1,33 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector\Fixture;
+
+final class JustCalledArrayKeys
+{
+    public function hasKey(): bool
+    {
+        return !in_array('key', array_keys([
+           'key' => ',',
+           1 => ';',
+        ]));
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector\Fixture;
+
+final class JustCalledArrayKeys
+{
+    public function hasKey(): bool
+    {
+        return !array_key_exists('key', [
+           'key' => ',',
+           1 => ';',
+        ]);
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/skip_array_keys_with_extra_argument.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FuncCall\ArrayKeysAndInArrayToArrayKeyExistsRector\Fixture;
+
+final class SkipArrayKeysWithExtraArgument
+{
+    public function hasMoreThanOneArgument(): bool
+    {
+        return in_array('key', array_keys([
+            'key' => ',',
+            1 => ';',
+        ], 'key'));
+    }
+}

rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/Fixture/fixture.php.inc

@@ -1,65 +0,0 @@
-<?php
-
-namespace Rector\Tests\CodeQuality\Rector\FuncCall\InArrayAndArrayKeysToArrayKeyExistsRector\Fixture;
-
-final class Fixture
-{
-    public function hasKey(): bool
-    {
-        return !in_array('key', array_keys([
-          'key' => ',',
-          1 => ';',
-        ]));
-    }
-
-    public function hasMoreThanOneArgument(): bool
-    {
-        return in_array('key', array_keys([
-          'key' => ',',
-          1 => ';',
-      ], 'key'));
-    }
-
-    public function resultIntoAVariable(): void
-    {
-        $array = ['foo', 'bar'];
-        $key = 'key';
-
-        $result = in_array($key, array_keys($array), true);
-    }
-}
-
-?>
------
-<?php
-
-namespace Rector\Tests\CodeQuality\Rector\FuncCall\InArrayAndArrayKeysToArrayKeyExistsRector\Fixture;
-
-final class Fixture
-{
-    public function hasKey(): bool
-    {
-        return !array_key_exists('key', [
-          'key' => ',',
-          1 => ';',
-        ]);
-    }
-
-    public function hasMoreThanOneArgument(): bool
-    {
-        return in_array('key', array_keys([
-          'key' => ',',
-          1 => ';',
-      ], 'key'));
-    }
-
-    public function resultIntoAVariable(): void
-    {
-        $array = ['foo', 'bar'];
-        $key = 'key';
-
-        $result = array_key_exists($key, $array);
-    }
-}
-
-?>

rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/InArrayAndArrayKeysToArrayKeyExistsRectorTest.php

@@ -1,33 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Rector\Tests\CodeQuality\Rector\FuncCall\InArrayAndArrayKeysToArrayKeyExistsRector;
-
-use Iterator;
-use Rector\Testing\PHPUnit\AbstractRectorTestCase;
-use Symplify\SmartFileSystem\SmartFileInfo;
-
-final class InArrayAndArrayKeysToArrayKeyExistsRectorTest extends AbstractRectorTestCase
-{
-    /**
-     * @dataProvider provideData()
-     */
-    public function test(SmartFileInfo $fileInfo): void
-    {
-        $this->doTestFileInfo($fileInfo);
-    }
-
-    /**
-     * @return Iterator<SmartFileInfo>
-     */
-    public function provideData(): Iterator
-    {
-        return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
-    }
-
-    public function provideConfigFilePath(): string
-    {
-        return __DIR__ . '/config/configured_rule.php';
-    }
-}

rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/config/configured_rule.php

@@ -1,11 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-use Rector\CodeQuality\Rector\FuncCall\InArrayAndArrayKeysToArrayKeyExistsRector;
-use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
-
-return static function (ContainerConfigurator $containerConfigurator): void {
-    $services = $containerConfigurator->services();
-    $services->set(InArrayAndArrayKeysToArrayKeyExistsRector::class);
-};

rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php

@@ -6,6 +6,7 @@ namespace Rector\CodeQuality\Rector\FuncCall;
 
 use PhpParser\Node;
 use PhpParser\Node\Arg;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\FunctionLike;
@@ -26,23 +27,17 @@ final class ArrayKeysAndInArrayToArrayKeyExistsRector extends AbstractRector
             [
                 new CodeSample(
                     <<<'CODE_SAMPLE'
-class SomeClass
+function run($packageName, $values)
 {
-    public function run($packageName, $values)
-    {
-        $keys = array_keys($values);
-        return in_array($packageName, $keys, true);
-    }
+    $keys = array_keys($values);
+    return in_array($packageName, $keys, true);
 }
 CODE_SAMPLE
 ,
                     <<<'CODE_SAMPLE'
-class SomeClass
+function run($packageName, $values)
 {
-    public function run($packageName, $values)
-    {
-        return array_key_exists($packageName, $values);
-    }
+    return array_key_exists($packageName, $values);
 }
 CODE_SAMPLE
                 ),
@@ -67,53 +62,37 @@ CODE_SAMPLE
             return null;
         }
 
-        if (! isset($node->args[1])) {
-            return null;
+        $args = $node->getArgs();
+        $secondArg = $args[1];
+
+        $arrayVariable = $secondArg->value;
+
+        $previousAssignArraysKeysFuncCall = $this->findPreviousAssignToArrayKeys($node, $arrayVariable);
+        if ($previousAssignArraysKeysFuncCall instanceof Assign) {
+            /** @var FuncCall $arrayKeysFuncCall */
+            $arrayKeysFuncCall = $previousAssignArraysKeysFuncCall->expr;
+
+            $this->removeNode($previousAssignArraysKeysFuncCall);
+
+            return $this->createArrayKeyExists($node, $arrayKeysFuncCall);
         }
 
-        if (! $node->args[1] instanceof Arg) {
-            return null;
+        if ($arrayVariable instanceof FuncCall && $this->isName($arrayVariable, 'array_keys')) {
+            $arrayKeysFuncCallArgs = $arrayVariable->getArgs();
+            if (count($arrayKeysFuncCallArgs) > 1) {
+                return null;
+            }
+
+            // unwrap array func call
+            $secondArg->value = $arrayKeysFuncCallArgs[0]->value;
+            $node->name = new Name('array_key_exists');
+
+            unset($node->args[2]);
+
+            return $node;
         }
 
-        $arrayVariable = $node->args[1]->value;
-
-        /** @var Assign|Node|null $previousAssignArraysKeysFuncCall */
-        $previousAssignArraysKeysFuncCall = $this->betterNodeFinder->findFirstPreviousOfNode($node, function (
-            Node $node
-        ) use ($arrayVariable): bool {
-            // breaking out of scope
-            if ($node instanceof FunctionLike) {
-                return true;
-            }
-
-            if (! $node instanceof Assign) {
-                return ! (bool) $this->betterNodeFinder->find(
-                    $node,
-                    fn (Node $n): bool => $this->nodeComparator->areNodesEqual($arrayVariable, $n)
-                );
-            }
-
-            if (! $this->nodeComparator->areNodesEqual($arrayVariable, $node->var)) {
-                return false;
-            }
-
-            if (! $node->expr instanceof FuncCall) {
-                return false;
-            }
-
-            return $this->nodeNameResolver->isName($node->expr, 'array_keys');
-        });
-
-        if (! $previousAssignArraysKeysFuncCall instanceof Assign) {
-            return null;
-        }
-
-        /** @var FuncCall $arrayKeysFuncCall */
-        $arrayKeysFuncCall = $previousAssignArraysKeysFuncCall->expr;
-
-        $this->removeNode($previousAssignArraysKeysFuncCall);
-
-        return $this->createArrayKeyExists($node, $arrayKeysFuncCall);
+        return null;
     }
 
     private function createArrayKeyExists(FuncCall $inArrayFuncCall, FuncCall $arrayKeysFuncCall): ?FuncCall
@@ -138,4 +117,31 @@ CODE_SAMPLE
 
         return new FuncCall(new Name('array_key_exists'), $arguments);
     }
+
+    private function findPreviousAssignToArrayKeys(FuncCall $funcCall, Expr $expr): null|Node|FunctionLike
+    {
+        return $this->betterNodeFinder->findFirstPreviousOfNode($funcCall, function (Node $node) use ($expr): bool {
+            // breaking out of scope
+            if ($node instanceof FunctionLike) {
+                return true;
+            }
+
+            if (! $node instanceof Assign) {
+                return ! (bool) $this->betterNodeFinder->find(
+                    $node,
+                    fn (Node $n): bool => $this->nodeComparator->areNodesEqual($expr, $n)
+                );
+            }
+
+            if (! $this->nodeComparator->areNodesEqual($expr, $node->var)) {
+                return false;
+            }
+
+            if (! $node->expr instanceof FuncCall) {
+                return false;
+            }
+
+            return $this->nodeNameResolver->isName($node->expr, 'array_keys');
+        });
+    }
 }

rules/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php

@@ -1,83 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Rector\CodeQuality\Rector\FuncCall;
-
-use PhpParser\Node;
-use PhpParser\Node\Arg;
-use PhpParser\Node\Expr\FuncCall;
-use PhpParser\Node\Name;
-use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
-use Rector\Core\Rector\AbstractRector;
-use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
-use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
-
-/**
- * @see \Rector\Tests\CodeQuality\Rector\FuncCall\InArrayAndArrayKeysToArrayKeyExistsRector\InArrayAndArrayKeysToArrayKeyExistsRectorTest
- */
-final class InArrayAndArrayKeysToArrayKeyExistsRector extends AbstractRector
-{
-    public function __construct(
-        private readonly ArgsAnalyzer $argsAnalyzer
-    ) {
-    }
-
-    public function getRuleDefinition(): RuleDefinition
-    {
-        return new RuleDefinition(
-            'Simplify `in_array` and `array_keys` functions combination into `array_key_exists` when `array_keys` has one argument only',
-            [new CodeSample('in_array("key", array_keys($array), true);', 'array_key_exists("key", $array);')]
-        );
-    }
-
-    /**
-     * @return array<class-string<Node>>
-     */
-    public function getNodeTypes(): array
-    {
-        return [FuncCall::class];
-    }
-
-    /**
-     * @param FuncCall $node
-     */
-    public function refactor(Node $node): ?Node
-    {
-        if (! $this->isName($node, 'in_array')) {
-            return null;
-        }
-
-        if (! $this->argsAnalyzer->isArgsInstanceInArgsPositions($node->args, [0, 1])) {
-            return null;
-        }
-
-        /** @var Arg $secondArgument */
-        $secondArgument = $node->args[1];
-        if (! $secondArgument->value instanceof FuncCall) {
-            return null;
-        }
-
-        if (! $this->isName($secondArgument->value, 'array_keys')) {
-            return null;
-        }
-
-        if (count($secondArgument->value->args) > 1) {
-            return null;
-        }
-
-        /** @var Arg $keyArg */
-        $keyArg = $node->args[0];
-        /** @var Arg $arrayArg */
-        $arrayArg = $node->args[1];
-
-        /** @var FuncCall $innerFuncCallNode */
-        $innerFuncCallNode = $arrayArg->value;
-        $arrayArg = $innerFuncCallNode->args[0];
-
-        $node->name = new Name('array_key_exists');
-        $node->args = [$keyArg, $arrayArg];
-
-        return $node;
-    }
-}