Skip to content

Commit

Permalink
[Performance][PostRector] Only process FullyQualified on import Node …
Browse files Browse the repository at this point in the history 
…name on NameImportingPostRector (#5255)

* [Performance][PostRector] Only process FullyQualified on import Node name on NameImportingPostRector so no need to revisit when already a Name node instance

* clean up unused namespace name and use name

* [ci-review] Rector Rectify

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
@samsonasik @actions-user
samsonasik and actions-user committed Nov 16, 2023
1 parent 1595e34 commit 363ae1b
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 76 deletions.
6 changes: 3 additions & 3 deletions packages/PostRector/Collector/UseNodesToAddCollector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Name\FullyQualified;
use Rector\Core\Provider\CurrentFileProvider;
use Rector\Core\ValueObject\Application\File;
use Rector\Naming\Naming\UseImportsResolver;
Expand Down Expand Up @@ -85,9 +85,9 @@ public function getUseImportTypesByNode(File $file, Node $node): array
return $objectTypes;
}

public function hasImport(File $file, Name $name, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
public function hasImport(File $file, FullyQualified $fullyQualified, FullyQualifiedObjectType $fullyQualifiedObjectType): bool
{
$useImports = $this->getUseImportTypesByNode($file, $name);
$useImports = $this->getUseImportTypesByNode($file, $fullyQualified);

foreach ($useImports as $useImport) {
if ($useImport->equals($fullyQualifiedObjectType)) {
Expand Down
31 changes: 12 additions & 19 deletions packages/PostRector/Rector/NameImportingPostRector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
use Rector\Core\ValueObject\Application\File;
use Rector\Naming\Naming\AliasNameResolver;
use Rector\Naming\Naming\UseImportsResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockNameImporter;

final class NameImportingPostRector extends AbstractPostRector
Expand Down Expand Up @@ -59,7 +58,7 @@ public function enterNode(Node $node): ?Node
return null;
}

if ($node instanceof Name) {
if ($node instanceof FullyQualified) {
return $this->processNodeName($node, $file);
}

Expand All @@ -86,9 +85,9 @@ public function enterNode(Node $node): ?Node
return $node;
}

private function processNodeName(Name $name, File $file): ?Node
private function processNodeName(FullyQualified $fullyQualified, File $file): ?Node
{
if ($name->isSpecialClassName()) {
if ($fullyQualified->isSpecialClassName()) {
return null;
}

Expand All @@ -103,47 +102,41 @@ private function processNodeName(Name $name, File $file): ?Node

/** @var Use_[]|GroupUse[] $currentUses */
$currentUses = $this->useImportsResolver->resolve();
if ($this->classNameImportSkipper->shouldSkipName($name, $currentUses)) {
if ($this->classNameImportSkipper->shouldSkipName($fullyQualified, $currentUses)) {
return null;
}

$nameInUse = $this->resolveNameInUse($name, $currentUses);
$nameInUse = $this->resolveNameInUse($fullyQualified, $currentUses);
if ($nameInUse instanceof Name) {
return $nameInUse;
}

return $this->nameImporter->importName($name, $file);
return $this->nameImporter->importName($fullyQualified, $file);
}

/**
* @param Use_[]|GroupUse[] $currentUses
*/
private function resolveNameInUse(Name $name, array $currentUses): null|Name
private function resolveNameInUse(FullyQualified $fullyQualified, array $currentUses): null|Name
{
$originalName = $name->getAttribute(AttributeKey::ORIGINAL_NAME);

if (! $originalName instanceof FullyQualified) {
return null;
}

$aliasName = $this->aliasNameResolver->resolveByName($name, $currentUses);
$aliasName = $this->aliasNameResolver->resolveByName($fullyQualified, $currentUses);
if (is_string($aliasName)) {
return new Name($aliasName);
}

return $this->resolveLongNameInUseName($name, $currentUses);
return $this->resolveLongNameInUseName($fullyQualified, $currentUses);
}

/**
* @param Use_[]|GroupUse[] $currentUses
*/
private function resolveLongNameInUseName(Name $name, array $currentUses): ?Name
private function resolveLongNameInUseName(FullyQualified $fullyQualified, array $currentUses): ?Name
{
if (substr_count($name->toCodeString(), '\\') === 1) {
if (substr_count($fullyQualified->toCodeString(), '\\') === 1) {
return null;
}

$lastName = $name->getLast();
$lastName = $fullyQualified->getLast();
foreach ($currentUses as $currentUse) {
foreach ($currentUse->uses as $useUse) {
if ($useUse->name->getLast() !== $lastName) {
Expand Down
10 changes: 5 additions & 5 deletions rules/CodingStyle/ClassNameImport/ClassNameImportSkipper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Stmt\GroupUse;
use PhpParser\Node\Stmt\Use_;
use PhpParser\Node\Stmt\UseUse;
Expand Down Expand Up @@ -43,14 +43,14 @@ public function shouldSkipNameForFullyQualifiedObjectType(
/**
* @param Use_[]|GroupUse[] $uses
*/
public function shouldSkipName(Name $name, array $uses): bool
public function shouldSkipName(FullyQualified $fullyQualified, array $uses): bool
{
if (substr_count($name->toCodeString(), '\\') <= 1) {
if (substr_count($fullyQualified->toCodeString(), '\\') <= 1) {
return false;
}

$stringName = $name->toString();
$lastUseName = $name->getLast();
$stringName = $fullyQualified->toString();
$lastUseName = $fullyQualified->getLast();
$nameLastName = strtolower($lastUseName);

foreach ($uses as $use) {
Expand Down
63 changes: 22 additions & 41 deletions rules/CodingStyle/Node/NameImporter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Rector\CodingStyle\Node;

use PhpParser\Node\Name;
use PhpParser\Node\Name\FullyQualified;
use Rector\CodingStyle\ClassNameImport\ClassNameImportSkipper;
use Rector\Core\Configuration\Option;
use Rector\Core\Configuration\Parameter\SimpleParameterProvider;
Expand All @@ -23,45 +24,39 @@ public function __construct(
) {
}

public function importName(Name $name, File $file): ?Name
public function importName(FullyQualified $fullyQualified, File $file): ?Name
{
if ($this->shouldSkipName($name)) {
if ($this->shouldSkipName($fullyQualified)) {
return null;
}

$staticType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($name);
$staticType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($fullyQualified);
if (! $staticType instanceof FullyQualifiedObjectType) {
return null;
}

return $this->importNameAndCollectNewUseStatement($file, $name, $staticType);
return $this->importNameAndCollectNewUseStatement($file, $fullyQualified, $staticType);
}

private function shouldSkipName(Name $name): bool
private function shouldSkipName(FullyQualified $fullyQualified): bool
{
$virtualNode = (bool) $name->getAttribute(AttributeKey::VIRTUAL_NODE);
$virtualNode = (bool) $fullyQualified->getAttribute(AttributeKey::VIRTUAL_NODE);
if ($virtualNode) {
return true;
}

// is scalar name?
if (in_array($name->toLowerString(), ['true', 'false', 'bool'], true)) {
if (in_array($fullyQualified->toLowerString(), ['true', 'false', 'bool'], true)) {
return true;
}

// namespace <name>
// use <name>;
if ($this->isNamespaceOrUseImportName($name)) {
return true;
}

if ($this->isFunctionOrConstantImportWithSingleName($name)) {
if ($this->isFunctionOrConstantImportWithSingleName($fullyQualified)) {
return true;
}

// Importing root namespace classes (like \DateTime) is optional
if (! SimpleParameterProvider::provideBoolParameter(Option::IMPORT_SHORT_CLASSES)) {
$stringName = $name->toString();
$stringName = $fullyQualified->toString();
if (substr_count($stringName, '\\') === 0) {
return true;
}
Expand All @@ -72,13 +67,13 @@ private function shouldSkipName(Name $name): bool

private function importNameAndCollectNewUseStatement(
File $file,
Name $name,
FullyQualified $fullyQualified,
FullyQualifiedObjectType $fullyQualifiedObjectType
): ?Name {
// the same end is already imported → skip
if ($this->classNameImportSkipper->shouldSkipNameForFullyQualifiedObjectType(
$file,
$name,
$fullyQualified,
$fullyQualifiedObjectType
)) {
return null;
Expand All @@ -92,46 +87,32 @@ private function importNameAndCollectNewUseStatement(
return null;
}

$this->addUseImport($file, $name, $fullyQualifiedObjectType);
$this->addUseImport($file, $fullyQualified, $fullyQualifiedObjectType);
return $fullyQualifiedObjectType->getShortNameNode();
}

/**
* Skip:
* - namespace name
* - use import name
*/
private function isNamespaceOrUseImportName(Name $name): bool
{
if ($name->getAttribute(AttributeKey::IS_NAMESPACE_NAME) === true) {
return true;
}

return $name->getAttribute(AttributeKey::IS_USEUSE_NAME) === true;
}

private function isFunctionOrConstantImportWithSingleName(Name $name): bool
private function isFunctionOrConstantImportWithSingleName(FullyQualified $fullyQualified): bool
{
if ($name->getAttribute(AttributeKey::IS_CONSTFETCH_NAME) === true) {
return count($name->getParts()) === 1;
if ($fullyQualified->getAttribute(AttributeKey::IS_CONSTFETCH_NAME) === true) {
return count($fullyQualified->getParts()) === 1;
}

if ($name->getAttribute(AttributeKey::IS_FUNCCALL_NAME) === true) {
return count($name->getParts()) === 1;
if ($fullyQualified->getAttribute(AttributeKey::IS_FUNCCALL_NAME) === true) {
return count($fullyQualified->getParts()) === 1;
}

return false;
}

private function addUseImport(File $file, Name $name, FullyQualifiedObjectType $fullyQualifiedObjectType): void
private function addUseImport(File $file, FullyQualified $fullyQualified, FullyQualifiedObjectType $fullyQualifiedObjectType): void
{
if ($this->useNodesToAddCollector->hasImport($file, $name, $fullyQualifiedObjectType)) {
if ($this->useNodesToAddCollector->hasImport($file, $fullyQualified, $fullyQualifiedObjectType)) {
return;
}

if ($name->getAttribute(AttributeKey::IS_FUNCCALL_NAME) === true) {
if ($fullyQualified->getAttribute(AttributeKey::IS_FUNCCALL_NAME) === true) {
$this->useNodesToAddCollector->addFunctionUseImport($fullyQualifiedObjectType);
} elseif ($name->getAttribute(AttributeKey::IS_CONSTFETCH_NAME) === true) {
} elseif ($fullyQualified->getAttribute(AttributeKey::IS_CONSTFETCH_NAME) === true) {
$this->useNodesToAddCollector->addConstantUseImport($fullyQualifiedObjectType);
} else {
$this->useNodesToAddCollector->addUseImport($fullyQualifiedObjectType);
Expand Down
4 changes: 2 additions & 2 deletions rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ private function hasCallLikeInAssignExpr(Expr $expr, Scope $scope): bool

private function isVariableUsedInFollowingStmts(
ClassMethod|Function_ $functionLike,
int $assignStmtPosition,
string $variableName
int $assignStmtPosition,
string $variableName
): bool {
if ($functionLike->stmts === null) {
return false;
Expand Down
9 changes: 3 additions & 6 deletions tests/Issues/ScopeNotAvailable/config/json_throw_case_sensitive_const_fetch.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@

declare(strict_types=1);

use Rector\Php73\Rector\FuncCall\JsonThrowOnErrorRector;
use Rector\Php73\Rector\ConstFetch\SensitiveConstantNameRector;
use Rector\Config\RectorConfig;
use Rector\Php73\Rector\ConstFetch\SensitiveConstantNameRector;
use Rector\Php73\Rector\FuncCall\JsonThrowOnErrorRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rules([
JsonThrowOnErrorRector::class,
SensitiveConstantNameRector::class
]);
$rectorConfig->rules([JsonThrowOnErrorRector::class, SensitiveConstantNameRector::class]);
};

0 comments on commit 363ae1b

Please sign in to comment.