From 0227d24e3636af762e8f800997f0e86fcddce07e Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 10 Apr 2022 21:42:40 +0200 Subject: [PATCH] Merge InArrayAndArrayKeysToArrayKeyExistsRector to ArrayKeysAndInArrayToArrayKeyExistsRector with almost identical behavior (#2047) --- .../docs/rector_rules_overview.md | 50 ++------- config/set/code-quality.php | 2 - .../Fixture/another_in_array.php.inc | 33 ++++++ .../Fixture/just_called_array_keys.php.inc | 33 ++++++ ...kip_array_keys_with_extra_argument.php.inc | 14 +++ .../Fixture/fixture.php.inc | 65 ------------ ...AndArrayKeysToArrayKeyExistsRectorTest.php | 33 ------ .../config/configured_rule.php | 11 -- ...ayKeysAndInArrayToArrayKeyExistsRector.php | 100 ++++++++++-------- ...rrayAndArrayKeysToArrayKeyExistsRector.php | 83 --------------- 10 files changed, 140 insertions(+), 284 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/another_in_array.php.inc create mode 100644 rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/just_called_array_keys.php.inc create mode 100644 rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/skip_array_keys_with_extra_argument.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/Fixture/fixture.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/InArrayAndArrayKeysToArrayKeyExistsRectorTest.php delete mode 100644 rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/config/configured_rule.php delete mode 100644 rules/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 53b0c4039df..ece5df02200 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -1,4 +1,4 @@ -# 510 Rules Overview +# 508 Rules Overview
@@ -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
-### 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); -``` - -
- ### InlineConstructorDefaultToPropertyRector Move property default from constructor to property default @@ -1243,26 +1227,6 @@ Simplify negated conditions with de Morgan theorem
-### 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; - } - } -``` - -
- ### 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) diff --git a/config/set/code-quality.php b/config/set/code-quality.php index c6d2c2d7676..d0083dc86e8 100644 --- a/config/set/code-quality.php +++ b/config/set/code-quality.php @@ -30,7 +30,6 @@ 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 @@ $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); diff --git a/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/another_in_array.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/another_in_array.php.inc new file mode 100644 index 00000000000..ae4fc489b15 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/another_in_array.php.inc @@ -0,0 +1,33 @@ + +----- + diff --git a/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/just_called_array_keys.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/just_called_array_keys.php.inc new file mode 100644 index 00000000000..08ae68bafd6 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/just_called_array_keys.php.inc @@ -0,0 +1,33 @@ + ',', + 1 => ';', + ])); + } +} + +?> +----- + ',', + 1 => ';', + ]); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/skip_array_keys_with_extra_argument.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/skip_array_keys_with_extra_argument.php.inc new file mode 100644 index 00000000000..07bc1602149 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector/Fixture/skip_array_keys_with_extra_argument.php.inc @@ -0,0 +1,14 @@ + ',', + 1 => ';', + ], 'key')); + } +} diff --git a/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/Fixture/fixture.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/Fixture/fixture.php.inc deleted file mode 100644 index 412986fde3d..00000000000 --- a/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,65 +0,0 @@ - ',', - 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); - } -} - -?> ------ - ',', - 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); - } -} - -?> diff --git a/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/InArrayAndArrayKeysToArrayKeyExistsRectorTest.php b/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/InArrayAndArrayKeysToArrayKeyExistsRectorTest.php deleted file mode 100644 index d19f6511640..00000000000 --- a/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/InArrayAndArrayKeysToArrayKeyExistsRectorTest.php +++ /dev/null @@ -1,33 +0,0 @@ -doTestFileInfo($fileInfo); - } - - /** - * @return Iterator - */ - public function provideData(): Iterator - { - return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/config/configured_rule.php deleted file mode 100644 index cfaa0a16535..00000000000 --- a/rules-tests/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector/config/configured_rule.php +++ /dev/null @@ -1,11 +0,0 @@ -services(); - $services->set(InArrayAndArrayKeysToArrayKeyExistsRector::class); -}; diff --git a/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php b/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php index 45a5032fcd8..7a73864db0b 100644 --- a/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php +++ b/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php @@ -6,6 +6,7 @@ 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 @@ public function getRuleDefinition(): RuleDefinition [ 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 @@ public function refactor(Node $node): ?Node return null; } - if (! isset($node->args[1])) { - return null; - } + $args = $node->getArgs(); + $secondArg = $args[1]; - if (! $node->args[1] instanceof Arg) { - return null; - } + $arrayVariable = $secondArg->value; - $arrayVariable = $node->args[1]->value; + $previousAssignArraysKeysFuncCall = $this->findPreviousAssignToArrayKeys($node, $arrayVariable); + if ($previousAssignArraysKeysFuncCall instanceof Assign) { + /** @var FuncCall $arrayKeysFuncCall */ + $arrayKeysFuncCall = $previousAssignArraysKeysFuncCall->expr; - /** @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; - } + $this->removeNode($previousAssignArraysKeysFuncCall); - if (! $node instanceof Assign) { - return ! (bool) $this->betterNodeFinder->find( - $node, - fn (Node $n): bool => $this->nodeComparator->areNodesEqual($arrayVariable, $n) - ); - } + return $this->createArrayKeyExists($node, $arrayKeysFuncCall); + } - if (! $this->nodeComparator->areNodesEqual($arrayVariable, $node->var)) { - return false; + if ($arrayVariable instanceof FuncCall && $this->isName($arrayVariable, 'array_keys')) { + $arrayKeysFuncCallArgs = $arrayVariable->getArgs(); + if (count($arrayKeysFuncCallArgs) > 1) { + return null; } - if (! $node->expr instanceof FuncCall) { - return false; - } + // unwrap array func call + $secondArg->value = $arrayKeysFuncCallArgs[0]->value; + $node->name = new Name('array_key_exists'); - return $this->nodeNameResolver->isName($node->expr, 'array_keys'); - }); + unset($node->args[2]); - if (! $previousAssignArraysKeysFuncCall instanceof Assign) { - return null; + return $node; } - /** @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 @@ private function createArrayKeyExists(FuncCall $inArrayFuncCall, FuncCall $array 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'); + }); + } } diff --git a/rules/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php b/rules/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php deleted file mode 100644 index 195b0b97f36..00000000000 --- a/rules/CodeQuality/Rector/FuncCall/InArrayAndArrayKeysToArrayKeyExistsRector.php +++ /dev/null @@ -1,83 +0,0 @@ -> - */ - 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; - } -}