From 28af8d7499388d284ed57c903d089105c2d43fd9 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 11 Aug 2023 12:07:08 +0200 Subject: [PATCH] Remove SwapMethodCallArgumentsRector as could lead to infinite swapping, use custom rule with type/value check instead --- .../docs/rector_rules_overview.md | 47 +----- .../Fixture/dynamic_call.php.inc | 31 ---- .../dynamic_call_immediate_return.php.inc | 31 ---- .../Fixture/static_call_with_parent.php.inc | 31 ---- .../Fixture/static_call_with_self.php.inc | 31 ---- .../Fixture/static_call_with_static.php.inc | 31 ---- .../Source/MethodCaller.php | 10 -- .../SwapMethodCallArgumentsRectorTest.php | 28 ---- .../config/configured_rule.php | 14 -- .../SwapMethodCallArgumentsRector.php | 145 ------------------ .../ValueObject/SwapMethodCallArguments.php | 40 ----- 11 files changed, 2 insertions(+), 437 deletions(-) delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call.php.inc delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call_immediate_return.php.inc delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_parent.php.inc delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_self.php.inc delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_static.php.inc delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Source/MethodCaller.php delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/SwapMethodCallArgumentsRectorTest.php delete mode 100644 rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/config/configured_rule.php delete mode 100644 rules/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector.php delete mode 100644 rules/Arguments/ValueObject/SwapMethodCallArguments.php diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 4a65ebf84cc..ed961cdba91 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -1,10 +1,10 @@ -# 356 Rules Overview +# 355 Rules Overview
## Categories -- [Arguments](#arguments) (6) +- [Arguments](#arguments) (5) - [CodeQuality](#codequality) (70) @@ -254,49 +254,6 @@ return static function (RectorConfig $rectorConfig): void {
-### SwapMethodCallArgumentsRector - -Reorder arguments in method calls - -:wrench: **configure it!** - -- class: [`Rector\Arguments\Rector\MethodCall\SwapMethodCallArgumentsRector`](../rules/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector.php) - -```php -ruleWithConfiguration(SwapMethodCallArgumentsRector::class, [ - new SwapMethodCallArguments('Caller', 'call', [ - 2, - 1, - 0, - ]), - ]); -}; -``` - -↓ - -```diff - final class SomeClass - { - public function run(Caller $caller) - { -- return $caller->call('one', 'two', 'three'); -+ return $caller->call('three', 'two', 'one'); - } - } -``` - -
- ## CodeQuality ### AbsolutizeRequireAndIncludePathRector diff --git a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call.php.inc b/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call.php.inc deleted file mode 100644 index bb6a7e88461..00000000000 --- a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call.php.inc +++ /dev/null @@ -1,31 +0,0 @@ -someCall($one, $two, $three); - } -} - -?> ------ -someCall($three, $two, $one); - } -} - -?> diff --git a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call_immediate_return.php.inc b/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call_immediate_return.php.inc deleted file mode 100644 index bb57dfadb62..00000000000 --- a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/dynamic_call_immediate_return.php.inc +++ /dev/null @@ -1,31 +0,0 @@ -someCall($one, $two, $three); - } -} - -?> ------ -someCall($three, $two, $one); - } -} - -?> diff --git a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_parent.php.inc b/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_parent.php.inc deleted file mode 100644 index 932c4519232..00000000000 --- a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_parent.php.inc +++ /dev/null @@ -1,31 +0,0 @@ - ------ - diff --git a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_self.php.inc b/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_self.php.inc deleted file mode 100644 index ffedfe5c61b..00000000000 --- a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_self.php.inc +++ /dev/null @@ -1,31 +0,0 @@ - ------ - diff --git a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_static.php.inc b/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_static.php.inc deleted file mode 100644 index a919d4a0315..00000000000 --- a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Fixture/static_call_with_static.php.inc +++ /dev/null @@ -1,31 +0,0 @@ - ------ - diff --git a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Source/MethodCaller.php b/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Source/MethodCaller.php deleted file mode 100644 index e5cb877940d..00000000000 --- a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/Source/MethodCaller.php +++ /dev/null @@ -1,10 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/config/configured_rule.php b/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/config/configured_rule.php deleted file mode 100644 index 9e65e721cde..00000000000 --- a/rules-tests/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector/config/configured_rule.php +++ /dev/null @@ -1,14 +0,0 @@ -ruleWithConfiguration(SwapMethodCallArgumentsRector::class, [ - new SwapMethodCallArguments(MethodCaller::class, 'someCall', [2, 1, 0]), - ]); -}; diff --git a/rules/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector.php b/rules/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector.php deleted file mode 100644 index 2e12b85a38b..00000000000 --- a/rules/Arguments/Rector/MethodCall/SwapMethodCallArgumentsRector.php +++ /dev/null @@ -1,145 +0,0 @@ -call('one', 'two', 'three'); - } -} -CODE_SAMPLE - , <<<'CODE_SAMPLE' -final class SomeClass -{ - public function run(Caller $caller) - { - return $caller->call('three', 'two', 'one'); - } -} -CODE_SAMPLE - , [new SwapMethodCallArguments('Caller', 'call', [2, 1, 0])]), - ]); - } - - /** - * @return array> - */ - public function getNodeTypes(): array - { - return [MethodCall::class, StaticCall::class]; - } - - /** - * @param MethodCall|StaticCall $node - */ - public function refactor(Node $node): MethodCall|StaticCall|null - { - $isJustSwapped = (bool) $node->getAttribute(self::JUST_SWAPPED, false); - if ($isJustSwapped) { - return null; - } - - if ($node->isFirstClassCallable()) { - return null; - } - - $args = $node->getArgs(); - - foreach ($this->methodArgumentSwaps as $methodArgumentSwap) { - if (! $this->isName($node->name, $methodArgumentSwap->getMethod())) { - continue; - } - - if (! $this->isObjectTypeMatch($node, $methodArgumentSwap->getObjectType())) { - continue; - } - - $newArguments = $this->resolveNewArguments($methodArgumentSwap, $args); - if ($newArguments === []) { - return null; - } - - foreach ($newArguments as $newPosition => $argument) { - $node->args[$newPosition] = $argument; - } - - $node->setAttribute(self::JUST_SWAPPED, true); - - return $node; - } - - return null; - } - - public function configure(array $configuration): void - { - Assert::allIsAOf($configuration, SwapMethodCallArguments::class); - $this->methodArgumentSwaps = $configuration; - } - - private function isObjectTypeMatch(MethodCall|StaticCall $node, ObjectType $objectType): bool - { - if ($node instanceof MethodCall) { - return $this->isObjectType($node->var, $objectType); - } - - return $this->isObjectType($node->class, $objectType); - } - - /** - * @param Arg[] $args - * @return array - */ - private function resolveNewArguments(SwapMethodCallArguments $swapMethodCallArguments, array $args): array - { - $newArguments = []; - foreach ($swapMethodCallArguments->getOrder() as $oldPosition => $newPosition) { - if (! isset($args[$oldPosition])) { - continue; - } - - if (! isset($args[$newPosition])) { - continue; - } - - $newArguments[$newPosition] = $args[$oldPosition]; - } - - return $newArguments; - } -} diff --git a/rules/Arguments/ValueObject/SwapMethodCallArguments.php b/rules/Arguments/ValueObject/SwapMethodCallArguments.php deleted file mode 100644 index d49407c7504..00000000000 --- a/rules/Arguments/ValueObject/SwapMethodCallArguments.php +++ /dev/null @@ -1,40 +0,0 @@ - $order - */ - public function __construct( - private readonly string $class, - private readonly string $method, - private readonly array $order, - ) { - RectorAssert::className($class); - } - - public function getObjectType(): ObjectType - { - return new ObjectType($this->class); - } - - public function getMethod(): string - { - return $this->method; - } - - /** - * @return array - */ - public function getOrder(): array - { - return $this->order; - } -}