From 9e8ed6c33f39aa9d7d859fb39b6f2d0344cdea0d Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 28 Aug 2023 14:43:08 +0200 Subject: [PATCH] Remove SwapFuncCallArgumentsRector as keeps swaping to infinity, use custom rule instead (#4874) --- .../docs/rector_rules_overview.md | 25 +--- .../Fixture/fixture.php.inc | 27 ---- .../SwapFuncCallArgumentsRectorTest.php | 28 ---- .../config/configured_rule.php | 15 -- .../FuncCall/SwapFuncCallArgumentsRector.php | 134 ------------------ .../ValueObject/SwapFuncCallArguments.php | 30 ---- .../Class_/AnnotationToAttributeRector.php | 5 + .../config/configured_rule.php | 6 +- utils/Command/OutsideAnySetCommand.php | 21 ++- 9 files changed, 30 insertions(+), 261 deletions(-) delete mode 100644 rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/Fixture/fixture.php.inc delete mode 100644 rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/SwapFuncCallArgumentsRectorTest.php delete mode 100644 rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/config/configured_rule.php delete mode 100644 rules/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector.php delete mode 100644 rules/Arguments/ValueObject/SwapFuncCallArguments.php diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 15e77d6f634..8b95129f29a 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -1,10 +1,10 @@ -# 354 Rules Overview +# 353 Rules Overview
## Categories -- [Arguments](#arguments) (5) +- [Arguments](#arguments) (4) - [CodeQuality](#codequality) (70) @@ -138,27 +138,6 @@ Replaces defined map of arguments in defined methods and their calls.
-### SwapFuncCallArgumentsRector - -Reorder arguments in function calls - -:wrench: **configure it!** - -- class: [`Rector\Arguments\Rector\FuncCall\SwapFuncCallArgumentsRector`](../rules/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector.php) - -```diff - final class SomeClass - { - public function run() - { -- return some_function('one', 'two', 'three'); -+ return some_function('three', 'two', 'one'); - } - } -``` - -
- ## CodeQuality ### AbsolutizeRequireAndIncludePathRector diff --git a/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/Fixture/fixture.php.inc b/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/Fixture/fixture.php.inc deleted file mode 100644 index 239222c4cee..00000000000 --- a/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,27 +0,0 @@ - ------ - diff --git a/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/SwapFuncCallArgumentsRectorTest.php b/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/SwapFuncCallArgumentsRectorTest.php deleted file mode 100644 index 09f00ff4870..00000000000 --- a/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/SwapFuncCallArgumentsRectorTest.php +++ /dev/null @@ -1,28 +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/FuncCall/SwapFuncCallArgumentsRector/config/configured_rule.php b/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/config/configured_rule.php deleted file mode 100644 index 726c137fd25..00000000000 --- a/rules-tests/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector/config/configured_rule.php +++ /dev/null @@ -1,15 +0,0 @@ -ruleWithConfiguration( - SwapFuncCallArgumentsRector::class, - [new SwapFuncCallArguments('some_function', [1, 0])] - ); -}; diff --git a/rules/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector.php b/rules/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector.php deleted file mode 100644 index e22cb5569fe..00000000000 --- a/rules/Arguments/Rector/FuncCall/SwapFuncCallArgumentsRector.php +++ /dev/null @@ -1,134 +0,0 @@ -> - */ - public function getNodeTypes(): array - { - return [FuncCall::class]; - } - - /** - * @param FuncCall $node - */ - public function refactor(Node $node): ?FuncCall - { - $isJustSwapped = (bool) $node->getAttribute(self::JUST_SWAPPED, false); - if ($isJustSwapped) { - return null; - } - - foreach ($this->functionArgumentSwaps as $functionArgumentSwap) { - if (! $this->isName($node, $functionArgumentSwap->getFunction())) { - continue; - } - - $newArguments = $this->resolveNewArguments($functionArgumentSwap, $node); - if ($newArguments === []) { - return null; - } - - foreach ($newArguments as $newPosition => $argument) { - $node->args[$newPosition] = $argument; - } - - $node->setAttribute(self::JUST_SWAPPED, true); - - return $node; - } - - return null; - } - - /** - * @param mixed[] $configuration - */ - public function configure(array $configuration): void - { - Assert::allIsAOf($configuration, SwapFuncCallArguments::class); - $this->functionArgumentSwaps = $configuration; - } - - /** - * @return array - */ - private function resolveNewArguments(SwapFuncCallArguments $swapFuncCallArguments, FuncCall $funcCall): array - { - $newArguments = []; - foreach ($swapFuncCallArguments->getOrder() as $oldPosition => $newPosition) { - if (! isset($funcCall->args[$oldPosition])) { - continue; - } - - if (! isset($funcCall->args[$newPosition])) { - continue; - } - - if (! $funcCall->args[$oldPosition] instanceof Arg) { - continue; - } - - $newArguments[$newPosition] = $funcCall->args[$oldPosition]; - } - - return $newArguments; - } -} diff --git a/rules/Arguments/ValueObject/SwapFuncCallArguments.php b/rules/Arguments/ValueObject/SwapFuncCallArguments.php deleted file mode 100644 index 5d798248b3a..00000000000 --- a/rules/Arguments/ValueObject/SwapFuncCallArguments.php +++ /dev/null @@ -1,30 +0,0 @@ - $order - */ - public function __construct( - private readonly string $function, - private readonly array $order - ) { - } - - public function getFunction(): string - { - return $this->function; - } - - /** - * @return array - */ - public function getOrder(): array - { - return $this->order; - } -} diff --git a/rules/Php80/Rector/Class_/AnnotationToAttributeRector.php b/rules/Php80/Rector/Class_/AnnotationToAttributeRector.php index 9658a7569af..027125c4028 100644 --- a/rules/Php80/Rector/Class_/AnnotationToAttributeRector.php +++ b/rules/Php80/Rector/Class_/AnnotationToAttributeRector.php @@ -32,6 +32,7 @@ use Rector\Php80\ValueObject\DoctrineTagAndAnnotationToAttribute; use Rector\PhpAttribute\NodeFactory\PhpAttributeGroupFactory; use Rector\PhpDocParser\PhpDocParser\PhpDocNodeTraverser; +use Rector\RectorGenerator\Exception\ConfigurationException; use Rector\VersionBonding\Contract\MinPhpVersionInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -118,6 +119,10 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { + if ($this->annotationsToAttributes === []) { + throw new ConfigurationException(sprintf('The "%s" rule requires configuration.', self::class)); + } + $phpDocInfo = $this->phpDocInfoFactory->createFromNode($node); if (! $phpDocInfo instanceof PhpDocInfo) { return null; diff --git a/tests/Issues/ConstructorPromoAnnotationToAttribute/config/configured_rule.php b/tests/Issues/ConstructorPromoAnnotationToAttribute/config/configured_rule.php index b0e5f58de89..6315a9d83e8 100644 --- a/tests/Issues/ConstructorPromoAnnotationToAttribute/config/configured_rule.php +++ b/tests/Issues/ConstructorPromoAnnotationToAttribute/config/configured_rule.php @@ -3,10 +3,14 @@ declare(strict_types=1); use Rector\Config\RectorConfig; + use Rector\Php80\Rector\Class_\AnnotationToAttributeRector; use Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector; +use Rector\Php80\ValueObject\AnnotationToAttribute; return static function (RectorConfig $rectorConfig): void { $rectorConfig->rule(ClassPropertyAssignToConstructorPromotionRector::class); - $rectorConfig->rule(AnnotationToAttributeRector::class); + $rectorConfig->ruleWithConfiguration(AnnotationToAttributeRector::class, [ + new AnnotationToAttribute('OldTag', 'NewAttribute'), + ]); }; diff --git a/utils/Command/OutsideAnySetCommand.php b/utils/Command/OutsideAnySetCommand.php index d505fd1eb2e..a5f0913e905 100644 --- a/utils/Command/OutsideAnySetCommand.php +++ b/utils/Command/OutsideAnySetCommand.php @@ -35,10 +35,25 @@ protected function execute(InputInterface $input, OutputInterface $output): int $rectorClassesOutsideAnySet = array_diff($existingRectorClasses, $setDefinedRectorClasses); - sort($rectorClassesOutsideAnySet); + // skip transform rules, as designed for custom use + $filteredRectorClassesOutsideAnySet = array_filter( + $rectorClassesOutsideAnySet, + static function (string $rectorClass): bool { + if (str_contains($rectorClass, '\\Transform\\')) { + return false; + } - $this->symfonyStyle->listing($rectorClassesOutsideAnySet); - $this->symfonyStyle->warning(sprintf('%d rules is outside any set', count($rectorClassesOutsideAnySet))); + // skip renaming rules too, as designed for custom use + return ! str_contains($rectorClass, '\\Renaming\\'); + } + ); + + sort($filteredRectorClassesOutsideAnySet); + + $this->symfonyStyle->listing($filteredRectorClassesOutsideAnySet); + $this->symfonyStyle->warning( + sprintf('%d rules is outside any set', count($filteredRectorClassesOutsideAnySet)) + ); return self::SUCCESS; }