Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise post rectors #6240

Merged
merged 6 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions src/PostRector/Rector/ClassRenamingPostRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ final class ClassRenamingPostRector extends AbstractPostRector
{
private FileWithoutNamespace|Namespace_|null $rootNode = null;

/**
* @var array<string, string>
*/
private array $oldToNewClasses = [];

public function __construct(
private readonly ClassRenamer $classRenamer,
private readonly RenamedClassesDataCollector $renamedClassesDataCollector,
Expand Down Expand Up @@ -50,18 +55,13 @@ public function enterNode(Node $node): ?Node
return null;
}

$oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
if ($oldToNewClasses === []) {
return null;
}

/** @var Scope|null $scope */
$scope = $node->getAttribute(AttributeKey::SCOPE);

if ($node instanceof FullyQualified) {
$result = $this->classRenamer->renameNode($node, $oldToNewClasses, $scope);
$result = $this->classRenamer->renameNode($node, $this->oldToNewClasses, $scope);
} else {
$result = $this->resolveResultWithPhpAttributeName($node, $oldToNewClasses, $scope);
$result = $this->resolveResultWithPhpAttributeName($node, $scope);
}

if (! SimpleParameterProvider::provideBoolParameter(Option::AUTO_IMPORT_NAMES)) {
Expand All @@ -88,19 +88,20 @@ public function afterTraverse(array $nodes): array
return $nodes;
}

/**
* @param array<string, string> $oldToNewClasses
*/
private function resolveResultWithPhpAttributeName(
Name $name,
array $oldToNewClasses,
?Scope $scope
): ?FullyQualified {
public function shouldTraverse(array $stmts): bool
{
$this->oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
TomasVotruba marked this conversation as resolved.
Show resolved Hide resolved

return $this->oldToNewClasses !== [];
TomasVotruba marked this conversation as resolved.
Show resolved Hide resolved
}

private function resolveResultWithPhpAttributeName(Name $name, ?Scope $scope): ?FullyQualified
{
$phpAttributeName = $name->getAttribute(AttributeKey::PHP_ATTRIBUTE_NAME);
if (is_string($phpAttributeName)) {
return $this->classRenamer->renameNode(
new FullyQualified($phpAttributeName, $name->getAttributes()),
$oldToNewClasses,
$this->oldToNewClasses,
$scope
);
}
Expand Down
25 changes: 16 additions & 9 deletions src/PostRector/Rector/NameImportingPostRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@

final class NameImportingPostRector extends AbstractPostRector
{
/**
* @var array<Use_|GroupUse>
*/
private array $currentUses = [];

public function __construct(
private readonly NameImporter $nameImporter,
private readonly ClassNameImportSkipper $classNameImportSkipper,
Expand All @@ -29,6 +34,12 @@ public function __construct(
) {
}

public function beforeTraverse(array $nodes)
{
$this->currentUses = $this->useImportsResolver->resolve();
TomasVotruba marked this conversation as resolved.
Show resolved Hide resolved
return $nodes;
}

public function enterNode(Node $node): Node|int|null
{
if (! $node instanceof FullyQualified) {
Expand All @@ -39,13 +50,12 @@ public function enterNode(Node $node): Node|int|null
return null;
}

$currentUses = $this->useImportsResolver->resolve();
if ($this->classNameImportSkipper->shouldSkipName($node, $currentUses)) {
if ($this->classNameImportSkipper->shouldSkipName($node, $this->currentUses)) {
return null;
}

// make use of existing use import
$nameInUse = $this->resolveNameInUse($node, $currentUses);
$nameInUse = $this->resolveNameInUse($node);
if ($nameInUse instanceof Name) {
$nameInUse->setAttribute(AttributeKey::NAMESPACED_NAME, $node->toString());
return $nameInUse;
Expand All @@ -62,12 +72,9 @@ public function shouldTraverse(array $stmts): bool
return $this->addUseStatementGuard->shouldTraverse($stmts, $this->getFile()->getFilePath());
}

/**
* @param array<Use_|GroupUse> $currentUses
*/
private function resolveNameInUse(FullyQualified $fullyQualified, array $currentUses): null|Name
private function resolveNameInUse(FullyQualified $fullyQualified): null|Name
{
$aliasName = $this->aliasNameResolver->resolveByName($fullyQualified, $currentUses);
$aliasName = $this->aliasNameResolver->resolveByName($fullyQualified, $this->currentUses);
if (is_string($aliasName)) {
return new Name($aliasName);
}
Expand All @@ -77,7 +84,7 @@ private function resolveNameInUse(FullyQualified $fullyQualified, array $current
}

$lastName = $fullyQualified->getLast();
foreach ($currentUses as $currentUse) {
foreach ($this->currentUses as $currentUse) {
foreach ($currentUse->uses as $useUse) {
if ($useUse->name->getLast() !== $lastName) {
continue;
Expand Down
6 changes: 0 additions & 6 deletions src/PostRector/Rector/UnusedImportRemovingPostRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
use PhpParser\Node\Stmt\UseUse;
use PhpParser\NodeTraverser;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
use Rector\Configuration\Option;
use Rector\Configuration\Parameter\SimpleParameterProvider;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
use Rector\PhpParser\Node\CustomNode\FileWithoutNamespace;
Expand All @@ -36,10 +34,6 @@ public function enterNode(Node $node): ?Node
return null;
}

if (! SimpleParameterProvider::provideBoolParameter(Option::REMOVE_UNUSED_IMPORTS)) {
TomasVotruba marked this conversation as resolved.
Show resolved Hide resolved
return null;
}

$hasChanged = false;

$namespaceName = $node instanceof Namespace_ && $node->name instanceof Name
Expand Down
15 changes: 15 additions & 0 deletions src/PostRector/Rector/UseAddingPostRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

namespace Rector\PostRector\Rector;

use PhpParser\Node;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\NodeTraverser;
use Rector\CodingStyle\Application\UseImportsAdder;
use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory;
use Rector\PhpParser\Node\CustomNode\FileWithoutNamespace;
Expand Down Expand Up @@ -68,6 +70,19 @@ public function beforeTraverse(array $nodes): array
);
}

public function enterNode(Node $node): int
samsonasik marked this conversation as resolved.
Show resolved Hide resolved
{
/**
* We stop the traversal because all the work has already been done in the beforeTraverse() function
*
* Using STOP_TRAVERSAL is usually dangerous as it will stop the processing of all your nodes for all visitors
* but since the PostFileProcessor is using direct new NodeTraverser() and traverse() for only a single
* visitor per execution, using stop traversal here is safe,
* ref https://github.com/rectorphp/rector-src/blob/fc1e742fa4d9861ccdc5933f3b53613b8223438d/src/PostRector/Application/PostFileProcessor.php#L59-L61
*/
return NodeTraverser::STOP_TRAVERSAL;
}

/**
* @param Stmt[] $nodes
* @param FullyQualifiedObjectType[] $useImportTypes
Expand Down