diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 5e1492b416e..8aab0d0f9f7 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -24,6 +24,7 @@ jobs: matrix: php_version: ['8.2'] directory: + - 'e2e/applied-auto-import' - 'e2e/applied-polyfill-php80' - 'e2e/applied-rule-change-docblock' - 'e2e/applied-rule-removed-node' diff --git a/e2e/applied-auto-import/.gitignore b/e2e/applied-auto-import/.gitignore new file mode 100644 index 00000000000..61ead86667c --- /dev/null +++ b/e2e/applied-auto-import/.gitignore @@ -0,0 +1 @@ +/vendor diff --git a/e2e/applied-auto-import/composer.json b/e2e/applied-auto-import/composer.json new file mode 100644 index 00000000000..5468cd74606 --- /dev/null +++ b/e2e/applied-auto-import/composer.json @@ -0,0 +1,7 @@ +{ + "require": { + "php": "^8.1" + }, + "minimum-stability": "dev", + "prefer-stable": true +} diff --git a/e2e/applied-auto-import/expected-output.diff b/e2e/applied-auto-import/expected-output.diff new file mode 100644 index 00000000000..5ab0dad5769 --- /dev/null +++ b/e2e/applied-auto-import/expected-output.diff @@ -0,0 +1,32 @@ +1 file with changes +=================== + +1) src/RenameDocblock.php:2 + + ---------- begin diff ---------- +@@ @@ + + namespace App; + +-use SomeUnusedClass; ++use DateTimeInterface; + + /** +- * @param \DateTime $someOldClass ++ * @param DateTimeInterface $someOldClass + */ +-function someFunction(\DateTime $someOldClass) ++function someFunction(DateTimeInterface $someOldClass) + { + } + ----------- end diff ----------- + +Applied rules: + * RenameClassRector + * DocblockNameImportingPostRector + * NameImportingPostRector + * UnusedImportRemovingPostRector + * UseAddingPostRector + + + [OK] 1 file would have been changed (dry-run) by Rector diff --git a/e2e/applied-auto-import/rector.php b/e2e/applied-auto-import/rector.php new file mode 100644 index 00000000000..e6de67b6c3e --- /dev/null +++ b/e2e/applied-auto-import/rector.php @@ -0,0 +1,19 @@ +paths([ + __DIR__ . '/src', + ]); + + $rectorConfig->ruleWithConfiguration(RenameClassRector::class, [ + 'DateTime' => 'DateTimeInterface' + ]); + + $rectorConfig->importNames(); + $rectorConfig->removeUnusedImports(); +}; diff --git a/e2e/applied-auto-import/src/RenameDocblock.php b/e2e/applied-auto-import/src/RenameDocblock.php new file mode 100644 index 00000000000..25c59f18bca --- /dev/null +++ b/e2e/applied-auto-import/src/RenameDocblock.php @@ -0,0 +1,12 @@ + $useImportTypes * @param array $constantUseImportTypes * @param array $functionUseImportTypes - * @return Stmt[] */ public function addImportsToStmts( FileWithoutNamespace $fileWithoutNamespace, @@ -39,7 +38,7 @@ public function addImportsToStmts( array $useImportTypes, array $constantUseImportTypes, array $functionUseImportTypes - ): array { + ): bool { $usedImports = $this->usedImportsResolver->resolveForStmts($stmts); $existingUseImportTypes = $usedImports->getUseImports(); $existingConstantUseImports = $usedImports->getConstantImports(); @@ -57,7 +56,7 @@ public function addImportsToStmts( $newUses = $this->createUses($useImportTypes, $constantUseImportTypes, $functionUseImportTypes, null); if ($newUses === []) { - return [$fileWithoutNamespace]; + return false; } $stmts = array_values(array_filter($stmts, static function (Stmt $stmt): bool { @@ -94,7 +93,7 @@ public function addImportsToStmts( $fileWithoutNamespace->stmts = $stmts; $fileWithoutNamespace->stmts = array_values($fileWithoutNamespace->stmts); - return [$fileWithoutNamespace]; + return true; } $this->mirrorUseComments($stmts, $newUses); @@ -103,7 +102,7 @@ public function addImportsToStmts( $fileWithoutNamespace->stmts = array_merge($newUses, $this->resolveInsertNop($fileWithoutNamespace), $stmts); $fileWithoutNamespace->stmts = array_values($fileWithoutNamespace->stmts); - return [$fileWithoutNamespace]; + return true; } /** @@ -116,7 +115,7 @@ public function addImportsToNamespace( array $useImportTypes, array $constantUseImportTypes, array $functionUseImportTypes - ): void { + ): bool { $namespaceName = $this->getNamespaceName($namespace); $existingUsedImports = $this->usedImportsResolver->resolveForStmts($namespace->stmts); @@ -141,13 +140,15 @@ public function addImportsToNamespace( $newUses = $this->createUses($useImportTypes, $constantUseImportTypes, $functionUseImportTypes, $namespaceName); if ($newUses === []) { - return; + return false; } $this->mirrorUseComments($namespace->stmts, $newUses); $namespace->stmts = array_merge($newUses, $this->resolveInsertNop($namespace), $namespace->stmts); $namespace->stmts = array_values($namespace->stmts); + + return true; } /** diff --git a/rules/CodingStyle/Application/UseImportsRemover.php b/rules/CodingStyle/Application/UseImportsRemover.php index 78446eb71e6..d20c6bf1eb4 100644 --- a/rules/CodingStyle/Application/UseImportsRemover.php +++ b/rules/CodingStyle/Application/UseImportsRemover.php @@ -4,8 +4,9 @@ namespace Rector\CodingStyle\Application; -use PhpParser\Node\Stmt; +use PhpParser\Node\Stmt\Namespace_; use PhpParser\Node\Stmt\Use_; +use Rector\PhpParser\Node\CustomNode\FileWithoutNamespace; use Rector\Renaming\Collector\RenamedNameCollector; final readonly class UseImportsRemover @@ -16,35 +17,40 @@ public function __construct( } /** - * @param Stmt[] $stmts * @param string[] $removedUses - * @return Stmt[] */ - public function removeImportsFromStmts(array $stmts, array $removedUses): array + public function removeImportsFromStmts(FileWithoutNamespace|Namespace_ $node, array $removedUses): bool { $hasRemoved = false; - foreach ($stmts as $key => $stmt) { + foreach ($node->stmts as $key => $stmt) { if (! $stmt instanceof Use_) { continue; } - $stmt = $this->removeUseFromUse($removedUses, $stmt); + if ($this->removeUseFromUse($removedUses, $stmt)) { + $node->stmts[$key] = $stmt; + $hasRemoved = true; + } // remove empty uses if ($stmt->uses === []) { - unset($stmts[$key]); - $hasRemoved = true; + unset($node->stmts[$key]); } } - return $hasRemoved ? array_values($stmts) : $stmts; + if ($hasRemoved) { + $node->stmts = array_values($node->stmts); + } + + return $hasRemoved; } /** * @param string[] $removedUses */ - private function removeUseFromUse(array $removedUses, Use_ $use): Use_ + private function removeUseFromUse(array $removedUses, Use_ $use): bool { + $hasChanged = false; foreach ($use->uses as $usesKey => $useUse) { $useName = $useUse->name->toString(); if (! in_array($useName, $removedUses, true)) { @@ -56,8 +62,13 @@ private function removeUseFromUse(array $removedUses, Use_ $use): Use_ } unset($use->uses[$usesKey]); + $hasChanged = true; + } + + if ($hasChanged) { + $use->uses = array_values($use->uses); } - return $use; + return $hasChanged; } } diff --git a/src/Bridge/SetRectorsResolver.php b/src/Bridge/SetRectorsResolver.php index f50457965bc..c54c7240302 100644 --- a/src/Bridge/SetRectorsResolver.php +++ b/src/Bridge/SetRectorsResolver.php @@ -46,7 +46,7 @@ public function resolveFromFilePathIncludingConfiguration(string $configFilePath { $rectorConfig = $this->loadRectorConfigFromFilePath($configFilePath); - $rectorClassesWithOptionalConfiguration = $rectorConfig->getRectorClasses(); + $rectorClassesWithOptionalConfiguration = $rectorConfig->getMainRectorClasses(); foreach ($rectorConfig->getRuleConfigurations() as $rectorClass => $configuration) { // remove from non-configurable, if added again with better config diff --git a/src/ChangesReporting/ValueObject/RectorWithLineChange.php b/src/ChangesReporting/ValueObject/RectorWithLineChange.php index b2edf707152..f5c0a71b6bf 100644 --- a/src/ChangesReporting/ValueObject/RectorWithLineChange.php +++ b/src/ChangesReporting/ValueObject/RectorWithLineChange.php @@ -5,6 +5,7 @@ namespace Rector\ChangesReporting\ValueObject; use Rector\Contract\Rector\RectorInterface; +use Rector\PostRector\Contract\Rector\PostRectorInterface; use Symplify\EasyParallel\Contract\SerializableInterface; use Webmozart\Assert\Assert; @@ -21,26 +22,16 @@ private const KEY_LINE = 'line'; /** - * @var class-string - */ - private string $rectorClass; - - /** - * @param class-string|RectorInterface $rectorClass + * @param class-string $rectorClass */ public function __construct( - string|RectorInterface $rectorClass, + private string $rectorClass, private int $line ) { - if ($rectorClass instanceof RectorInterface) { - $rectorClass = $rectorClass::class; - } - - $this->rectorClass = $rectorClass; } /** - * @return class-string + * @return class-string */ public function getRectorClass(): string { @@ -63,7 +54,7 @@ public static function decode(array $json): self } /** - * @return array{rector_class: class-string, line: int} + * @return array{rector_class: class-string, line: int} */ public function jsonSerialize(): array { diff --git a/src/Config/RectorConfig.php b/src/Config/RectorConfig.php index 129556ee82c..a4710622791 100644 --- a/src/Config/RectorConfig.php +++ b/src/Config/RectorConfig.php @@ -439,7 +439,7 @@ public function getRuleConfigurations(): array * @internal Used only for bridge * @return array> */ - public function getRectorClasses(): array + public function getMainRectorClasses(): array { return $this->tags[RectorInterface::class] ?? []; } diff --git a/src/Configuration/RectorConfigBuilder.php b/src/Configuration/RectorConfigBuilder.php index 64c5eb3a944..ae16e45f9bb 100644 --- a/src/Configuration/RectorConfigBuilder.php +++ b/src/Configuration/RectorConfigBuilder.php @@ -250,7 +250,7 @@ public function __invoke(RectorConfig $rectorConfig): void } // log rules from sets and compare them with explicit rules - $setRegisteredRectorClasses = $rectorConfig->getRectorClasses(); + $setRegisteredRectorClasses = $rectorConfig->getMainRectorClasses(); SimpleParameterProvider::addParameter(Option::SET_REGISTERED_RULES, $setRegisteredRectorClasses); if ($this->paths !== []) { diff --git a/src/PostRector/Rector/AbstractPostRector.php b/src/PostRector/Rector/AbstractPostRector.php index 77632fbbb03..341100666b9 100644 --- a/src/PostRector/Rector/AbstractPostRector.php +++ b/src/PostRector/Rector/AbstractPostRector.php @@ -4,8 +4,10 @@ namespace Rector\PostRector\Rector; +use PhpParser\Node; use PhpParser\Node\Stmt; use PhpParser\NodeVisitorAbstract; +use Rector\ChangesReporting\ValueObject\RectorWithLineChange; use Rector\PostRector\Contract\Rector\PostRectorInterface; use Rector\ValueObject\Application\File; use Webmozart\Assert\Assert; @@ -33,4 +35,12 @@ public function getFile(): File return $this->file; } + + protected function addRectorClassWithLine(Node $node): void + { + Assert::isInstanceOf($this->file, File::class); + + $rectorWithLineChange = new RectorWithLineChange(static::class, $node->getStartLine()); + $this->file->addRectorClassWithLine($rectorWithLineChange); + } } diff --git a/src/PostRector/Rector/ClassRenamingPostRector.php b/src/PostRector/Rector/ClassRenamingPostRector.php index 7767b6370a7..50f91212e44 100644 --- a/src/PostRector/Rector/ClassRenamingPostRector.php +++ b/src/PostRector/Rector/ClassRenamingPostRector.php @@ -44,7 +44,9 @@ public function beforeTraverse(array $nodes): array foreach ($nodes as $node) { if ($node instanceof FileWithoutNamespace || $node instanceof Namespace_) { $removedUses = $this->renamedClassesDataCollector->getOldClasses(); - $node->stmts = $this->useImportsRemover->removeImportsFromStmts($node->stmts, $removedUses); + if ($this->useImportsRemover->removeImportsFromStmts($node, $removedUses)) { + $this->addRectorClassWithLine($node); + } break; } diff --git a/src/PostRector/Rector/DocblockNameImportingPostRector.php b/src/PostRector/Rector/DocblockNameImportingPostRector.php index d60bdfea9c4..407f8de47db 100644 --- a/src/PostRector/Rector/DocblockNameImportingPostRector.php +++ b/src/PostRector/Rector/DocblockNameImportingPostRector.php @@ -39,6 +39,7 @@ public function enterNode(Node $node): Node|null return null; } + $this->addRectorClassWithLine($node); $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); return $node; } diff --git a/src/PostRector/Rector/NameImportingPostRector.php b/src/PostRector/Rector/NameImportingPostRector.php index 9c51238b31f..66ada94a3ba 100644 --- a/src/PostRector/Rector/NameImportingPostRector.php +++ b/src/PostRector/Rector/NameImportingPostRector.php @@ -5,6 +5,7 @@ namespace Rector\PostRector\Rector; use PhpParser\Node; +use PhpParser\Node\Name; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\GroupUse; @@ -36,13 +37,19 @@ public function beforeTraverse(array $nodes): array return $nodes; } - public function enterNode(Node $node): Node|null + public function enterNode(Node $node): Name|null { if (! $node instanceof FullyQualified) { return null; } - return $this->nameImporter->importName($node, $this->getFile(), $this->currentUses); + $name = $this->nameImporter->importName($node, $this->getFile(), $this->currentUses); + if (! $name instanceof Name) { + return null; + } + + $this->addRectorClassWithLine($node); + return $name; } /** diff --git a/src/PostRector/Rector/UnusedImportRemovingPostRector.php b/src/PostRector/Rector/UnusedImportRemovingPostRector.php index 35395d7aab9..b06f2ad7191 100644 --- a/src/PostRector/Rector/UnusedImportRemovingPostRector.php +++ b/src/PostRector/Rector/UnusedImportRemovingPostRector.php @@ -93,6 +93,7 @@ public function enterNode(Node $node): ?Node return null; } + $this->addRectorClassWithLine($node); $node->stmts = array_values($node->stmts); return $node; } diff --git a/src/PostRector/Rector/UseAddingPostRector.php b/src/PostRector/Rector/UseAddingPostRector.php index 35f66e57852..2927abba72b 100644 --- a/src/PostRector/Rector/UseAddingPostRector.php +++ b/src/PostRector/Rector/UseAddingPostRector.php @@ -35,6 +35,9 @@ public function beforeTraverse(array $nodes): array } $rootNode = $this->resolveRootNode($nodes); + if (! $rootNode instanceof FileWithoutNamespace && ! $rootNode instanceof Namespace_) { + return $nodes; + } $useImportTypes = $this->useNodesToAddCollector->getObjectImportsByFilePath($this->getFile()->getFilePath()); $constantUseImportTypes = $this->useNodesToAddCollector->getConstantImportsByFilePath( @@ -52,22 +55,19 @@ public function beforeTraverse(array $nodes): array /** @var FullyQualifiedObjectType[] $useImportTypes */ $useImportTypes = $this->typeFactory->uniquateTypes($useImportTypes); + $stmts = $rootNode instanceof FileWithoutNamespace ? $rootNode->stmts : $nodes; - if ($rootNode instanceof FileWithoutNamespace) { - $nodes = $rootNode->stmts; - } - - if (! $rootNode instanceof FileWithoutNamespace && ! $rootNode instanceof Namespace_) { - return $nodes; - } - - return $this->resolveNodesWithImportedUses( - $nodes, + if ($this->processStmtsWithImportedUses( + $stmts, $useImportTypes, $constantUseImportTypes, $functionUseImportTypes, $rootNode - ); + )) { + $this->addRectorClassWithLine($rootNode); + } + + return $nodes; } public function enterNode(Node $node): int @@ -84,30 +84,27 @@ public function enterNode(Node $node): int } /** - * @param Stmt[] $nodes + * @param Stmt[] $stmts * @param FullyQualifiedObjectType[] $useImportTypes * @param FullyQualifiedObjectType[] $constantUseImportTypes * @param FullyQualifiedObjectType[] $functionUseImportTypes - * @return Stmt[] */ - private function resolveNodesWithImportedUses( - array $nodes, + private function processStmtsWithImportedUses( + array $stmts, array $useImportTypes, array $constantUseImportTypes, array $functionUseImportTypes, FileWithoutNamespace|Namespace_ $namespace - ): array { + ): bool { // A. has namespace? add under it if ($namespace instanceof Namespace_) { // then add, to prevent adding + removing false positive of same short use - $this->useImportsAdder->addImportsToNamespace( + return $this->useImportsAdder->addImportsToNamespace( $namespace, $useImportTypes, $constantUseImportTypes, $functionUseImportTypes ); - - return $nodes; } // B. no namespace? add in the top @@ -116,7 +113,7 @@ private function resolveNodesWithImportedUses( // then add, to prevent adding + removing false positive of same short use return $this->useImportsAdder->addImportsToStmts( $namespace, - $nodes, + $stmts, $useImportTypes, $constantUseImportTypes, $functionUseImportTypes diff --git a/src/Testing/PHPUnit/ValueObject/RectorTestResult.php b/src/Testing/PHPUnit/ValueObject/RectorTestResult.php index c399788a1dc..d831f22be64 100644 --- a/src/Testing/PHPUnit/ValueObject/RectorTestResult.php +++ b/src/Testing/PHPUnit/ValueObject/RectorTestResult.php @@ -5,6 +5,8 @@ namespace Rector\Testing\PHPUnit\ValueObject; use Rector\Contract\Rector\RectorInterface; +use Rector\PostRector\Contract\Rector\PostRectorInterface; +use Rector\Util\RectorClassesSorter; use Rector\ValueObject\ProcessResult; /** @@ -24,7 +26,7 @@ public function getChangedContents(): string } /** - * @return array> + * @return array> */ public function getAppliedRectorClasses(): array { @@ -34,8 +36,6 @@ public function getAppliedRectorClasses(): array $rectorClasses = array_merge($rectorClasses, $fileDiff->getRectorClasses()); } - sort($rectorClasses); - - return array_unique($rectorClasses); + return RectorClassesSorter::sort($rectorClasses); } } diff --git a/src/Util/RectorClassesSorter.php b/src/Util/RectorClassesSorter.php new file mode 100644 index 00000000000..6a1777eb47e --- /dev/null +++ b/src/Util/RectorClassesSorter.php @@ -0,0 +1,37 @@ +> $rectorClasses + * @return array> + */ + public static function sort(array $rectorClasses): array + { + $rectorClasses = array_unique($rectorClasses); + + $mainRector = array_filter( + $rectorClasses, + fn (string $rectorClass): bool => is_a($rectorClass, RectorInterface::class, true) + ); + sort($mainRector); + + $postRector = array_filter( + $rectorClasses, + fn (string $rectorClass): bool => is_a($rectorClass, PostRectorInterface::class, true) + ); + sort($postRector); + + return array_merge($mainRector, $postRector); + } +} diff --git a/src/ValueObject/Reporting/FileDiff.php b/src/ValueObject/Reporting/FileDiff.php index c9c48592a11..2b6928a86d6 100644 --- a/src/ValueObject/Reporting/FileDiff.php +++ b/src/ValueObject/Reporting/FileDiff.php @@ -8,6 +8,8 @@ use Rector\ChangesReporting\ValueObject\RectorWithLineChange; use Rector\Contract\Rector\RectorInterface; use Rector\Parallel\ValueObject\BridgeItem; +use Rector\PostRector\Contract\Rector\PostRectorInterface; +use Rector\Util\RectorClassesSorter; use Symplify\EasyParallel\Contract\SerializableInterface; use Webmozart\Assert\Assert; @@ -81,7 +83,7 @@ public function getRectorShortClasses(): array } /** - * @return array> + * @return array> */ public function getRectorClasses(): array { @@ -91,7 +93,7 @@ public function getRectorClasses(): array $rectorClasses[] = $rectorWithLineChange->getRectorClass(); } - return $this->sortClasses($rectorClasses); + return RectorClassesSorter::sort($rectorClasses); } public function getFirstLineNumber(): ?int @@ -158,17 +160,4 @@ public static function decode(array $json): self $rectorWithLineChanges, ); } - - /** - * @template TType as object - * @param array> $rectorClasses - * @return array> - */ - private function sortClasses(array $rectorClasses): array - { - $rectorClasses = array_unique($rectorClasses); - sort($rectorClasses); - - return $rectorClasses; - } }