From 502cc04dacff9c8b3ff1297878968e90ae4e55d0 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 5 Nov 2021 01:17:45 +0100 Subject: [PATCH] Add UseAnalyzer test + use returned variables (#1148) --- ecs.php | 1 + .../UseAnalyzer/Fixture/some_uses.php.inc | 15 +++ .../UseAnalyzer/Source/FirstUsage.php | 10 ++ .../UseAnalyzer/UseAnalyzerTest.php | 17 ++- .../NodeAnalyzer/UseAnalyzer.php | 119 ++++++++++++++++++ .../ValueObject/NameAndParent.php | 2 +- .../Testing/TestingParser/TestingParser.php | 22 ++-- .../ShortNameResolverTest.php | 21 +--- .../UseManipulator/Fixture/some_uses.php.inc | 15 --- .../Node/UseManipulator/Source/FirstUsage.php | 10 -- rules/CodingStyle/Naming/NameRenamer.php | 2 +- rules/CodingStyle/Node/UseManipulator.php | 100 --------------- .../Rector/Use_/RemoveUnusedAliasRector.php | 50 ++++---- 13 files changed, 198 insertions(+), 186 deletions(-) create mode 100644 packages-tests/NameImporting/NodeAnalyzer/UseAnalyzer/Fixture/some_uses.php.inc create mode 100644 packages-tests/NameImporting/NodeAnalyzer/UseAnalyzer/Source/FirstUsage.php rename rules-tests/CodingStyle/Node/UseManipulator/UseManipulatorTest.php => packages-tests/NameImporting/NodeAnalyzer/UseAnalyzer/UseAnalyzerTest.php (80%) create mode 100644 packages/NameImporting/NodeAnalyzer/UseAnalyzer.php rename {rules/CodingStyle => packages/NameImporting}/ValueObject/NameAndParent.php (91%) delete mode 100644 rules-tests/CodingStyle/Node/UseManipulator/Fixture/some_uses.php.inc delete mode 100644 rules-tests/CodingStyle/Node/UseManipulator/Source/FirstUsage.php delete mode 100644 rules/CodingStyle/Node/UseManipulator.php diff --git a/ecs.php b/ecs.php index 0701a39cfaa..3c374ee0c9c 100644 --- a/ecs.php +++ b/ecs.php @@ -71,6 +71,7 @@ __DIR__ . '/packages-tests/BetterPhpDocParser/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php', __DIR__ . '/tests/PhpParser/Node/NodeFactoryTest.php', __DIR__ . '/packages-tests/BetterPhpDocParser/PhpDocParser/StaticDoctrineAnnotationParser/StaticDoctrineAnnotationParserTest.php', + __DIR__ . '/packages-tests/NameImporting/NodeAnalyzer/UseAnalyzer/UseAnalyzerTest.php', '*TypeResolverTest.php', __DIR__ . '/rules-tests/CodingStyle/Node/UseManipulator/UseManipulatorTest.php', ], diff --git a/packages-tests/NameImporting/NodeAnalyzer/UseAnalyzer/Fixture/some_uses.php.inc b/packages-tests/NameImporting/NodeAnalyzer/UseAnalyzer/Fixture/some_uses.php.inc new file mode 100644 index 00000000000..f7483e5a476 --- /dev/null +++ b/packages-tests/NameImporting/NodeAnalyzer/UseAnalyzer/Fixture/some_uses.php.inc @@ -0,0 +1,15 @@ +boot(); - $this->useManipulator = $this->getService(UseManipulator::class); + $this->useAnalyzer = $this->getService(UseAnalyzer::class); $this->testingParser = $this->getService(TestingParser::class); } @@ -45,10 +45,9 @@ public function test( string $parentNodeType ): void { $file = $this->testingParser->parseFilePathToFile($filePath); - $firstStmt = $file->getOldStmts()[1]; - $namesAndParentsByShortName = $this->useManipulator->resolveUsedNameNodes($firstStmt); + $namesAndParentsByShortName = $this->useAnalyzer->resolveUsedNameNodes($firstStmt); $this->assertArrayHasKey($expectedShortName, $namesAndParentsByShortName); $namesAndParents = $namesAndParentsByShortName[$expectedShortName]; diff --git a/packages/NameImporting/NodeAnalyzer/UseAnalyzer.php b/packages/NameImporting/NodeAnalyzer/UseAnalyzer.php new file mode 100644 index 00000000000..eef624eaf41 --- /dev/null +++ b/packages/NameImporting/NodeAnalyzer/UseAnalyzer.php @@ -0,0 +1,119 @@ + + */ + public function resolveUsedNameNodes(Node $node): array + { + $usedNamesByShortName = $this->resolveUsedNames($node); + $usedClassNamesByShortName = $this->resolveUsedClassNames($node); + $usedTraitNamesByShortName = $this->resolveTraitUseNames($node); + + return array_merge($usedNamesByShortName, $usedClassNamesByShortName, $usedTraitNamesByShortName); + } + + /** + * @return array + */ + private function resolveUsedNames(Node $node): array + { + $namesAndParentsByShortName = []; + + /** @var Name[] $names */ + $names = $this->betterNodeFinder->findInstanceOf($node, Name::class); + + foreach ($names as $name) { + /** node name before becoming FQN - attribute from @see NameResolver */ + $originalName = $name->getAttribute(AttributeKey::ORIGINAL_NAME); + if (! $originalName instanceof Name) { + continue; + } + + $parentNode = $name->getAttribute(AttributeKey::PARENT_NODE); + if (! $parentNode instanceof Node) { + throw new ShouldNotHappenException(); + } + + $shortName = $originalName->toString(); + $namesAndParentsByShortName[$shortName][] = new NameAndParent($name, $parentNode); + } + + return $namesAndParentsByShortName; + } + + /** + * @return array + */ + private function resolveUsedClassNames(Node $node): array + { + $namesAndParentsByShortName = []; + + /** @var ClassLike[] $classLikes */ + $classLikes = $this->betterNodeFinder->findClassLikes($node); + + foreach ($classLikes as $classLike) { + $classLikeName = $classLike->name; + if (! $classLikeName instanceof Identifier) { + continue; + } + + $name = $this->nodeNameResolver->getName($classLikeName); + if ($name === null) { + continue; + } + + $namesAndParentsByShortName[$name][] = new NameAndParent($classLikeName, $classLike); + } + + return $namesAndParentsByShortName; + } + + /** + * @return array + */ + private function resolveTraitUseNames(Node $node): array + { + $namesAndParentsByShortName = []; + + /** @var Identifier[] $identifiers */ + $identifiers = $this->betterNodeFinder->findInstanceOf($node, Identifier::class); + + foreach ($identifiers as $identifier) { + $parentNode = $identifier->getAttribute(AttributeKey::PARENT_NODE); + if (! $parentNode instanceof UseUse) { + continue; + } + + $shortName = $identifier->name; + $namesAndParentsByShortName[$shortName][] = new NameAndParent($identifier, $parentNode); + } + + return $namesAndParentsByShortName; + } +} diff --git a/rules/CodingStyle/ValueObject/NameAndParent.php b/packages/NameImporting/ValueObject/NameAndParent.php similarity index 91% rename from rules/CodingStyle/ValueObject/NameAndParent.php rename to packages/NameImporting/ValueObject/NameAndParent.php index 77f49773944..737bdcfee15 100644 --- a/rules/CodingStyle/ValueObject/NameAndParent.php +++ b/packages/NameImporting/ValueObject/NameAndParent.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Rector\CodingStyle\ValueObject; +namespace Rector\NameImporting\ValueObject; use PhpParser\Node; use PhpParser\Node\Identifier; diff --git a/packages/Testing/TestingParser/TestingParser.php b/packages/Testing/TestingParser/TestingParser.php index 9af772d0cdc..70797ed9061 100644 --- a/packages/Testing/TestingParser/TestingParser.php +++ b/packages/Testing/TestingParser/TestingParser.php @@ -24,6 +24,17 @@ public function __construct( ) { } + public function parseFilePathToFile(string $filePath): File + { + $smartFileInfo = new SmartFileInfo($filePath); + $file = new File($smartFileInfo, $smartFileInfo->getContents()); + + $stmts = $this->rectorParser->parseFile($smartFileInfo); + $file->hydrateStmtsAndTokens($stmts, $stmts, []); + + return $file; + } + /** * @return Node[] */ @@ -40,15 +51,4 @@ public function parseFileToDecoratedNodes(string $file): array $file = new File($smartFileInfo, $smartFileInfo->getContents()); return $this->nodeScopeAndMetadataDecorator->decorateNodesFromFile($file, $nodes); } - - public function parseFilePathToFile(string $filePath): File - { - $smartFileInfo = new SmartFileInfo($filePath); - $file = new File($smartFileInfo, $smartFileInfo->getContents()); - - $stmts = $this->rectorParser->parseFile($smartFileInfo); - $file->hydrateStmtsAndTokens($stmts, $stmts, []); - - return $file; - } } diff --git a/rules-tests/CodingStyle/ClassNameImport/ShortNameResolver/ShortNameResolverTest.php b/rules-tests/CodingStyle/ClassNameImport/ShortNameResolver/ShortNameResolverTest.php index 501de6b33de..995f0303704 100644 --- a/rules-tests/CodingStyle/ClassNameImport/ShortNameResolver/ShortNameResolverTest.php +++ b/rules-tests/CodingStyle/ClassNameImport/ShortNameResolver/ShortNameResolverTest.php @@ -6,22 +6,20 @@ use Iterator; use Rector\CodingStyle\ClassNameImport\ShortNameResolver; -use Rector\Core\PhpParser\Parser\RectorParser; -use Rector\Core\ValueObject\Application\File; use Rector\Testing\PHPUnit\AbstractTestCase; -use Symplify\SmartFileSystem\SmartFileInfo; +use Rector\Testing\TestingParser\TestingParser; final class ShortNameResolverTest extends AbstractTestCase { private ShortNameResolver $shortNameResolver; - private RectorParser $rectorParser; + private TestingParser $testingParser; protected function setUp(): void { $this->boot(); $this->shortNameResolver = $this->getService(ShortNameResolver::class); - $this->rectorParser = $this->getService(RectorParser::class); + $this->testingParser = $this->getService(TestingParser::class); } /** @@ -30,7 +28,7 @@ protected function setUp(): void */ public function test(string $filePath, array $expectedShortNames): void { - $file = $this->createFileFromFilePath($filePath); + $file = $this->testingParser->parseFilePathToFile($filePath); $shortNames = $this->shortNameResolver->resolveFromFile($file); $this->assertSame($expectedShortNames, $shortNames); @@ -62,15 +60,4 @@ public function provideData(): Iterator 'SecondLog' => 'Rector\Tests\CodingStyle\ClassNameImport\ShortNameResolver\Source\SecondLog', ]]; } - - private function createFileFromFilePath(string $filePath): File - { - $smartFileInfo = new SmartFileInfo($filePath); - $file = new File($smartFileInfo, $smartFileInfo->getContents()); - - $stmts = $this->rectorParser->parseFile($smartFileInfo); - $file->hydrateStmtsAndTokens($stmts, $stmts, []); - - return $file; - } } diff --git a/rules-tests/CodingStyle/Node/UseManipulator/Fixture/some_uses.php.inc b/rules-tests/CodingStyle/Node/UseManipulator/Fixture/some_uses.php.inc deleted file mode 100644 index 82466f52e27..00000000000 --- a/rules-tests/CodingStyle/Node/UseManipulator/Fixture/some_uses.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -resolvedNodeNames = []; - - $this->resolveUsedNames($node); - $this->resolveUsedClassNames($node); - $this->resolveTraitUseNames($node); - - return $this->resolvedNodeNames; - } - - private function resolveUsedNames(Node $node): void - { - /** @var Name[] $namedNodes */ - $namedNodes = $this->betterNodeFinder->findInstanceOf($node, Name::class); - - foreach ($namedNodes as $namedNode) { - /** node name before becoming FQN - attribute from @see NameResolver */ - $originalName = $namedNode->getAttribute(AttributeKey::ORIGINAL_NAME); - if (! $originalName instanceof Name) { - continue; - } - - $parentNode = $namedNode->getAttribute(AttributeKey::PARENT_NODE); - if (! $parentNode instanceof Node) { - throw new ShouldNotHappenException(); - } - - $this->resolvedNodeNames[$originalName->toString()][] = new NameAndParent($namedNode, $parentNode); - } - } - - private function resolveUsedClassNames(Node $searchNode): void - { - /** @var ClassLike[] $classLikes */ - $classLikes = $this->betterNodeFinder->findClassLikes([$searchNode]); - - foreach ($classLikes as $classLike) { - $classLikeName = $classLike->name; - if (! $classLikeName instanceof Identifier) { - continue; - } - - $name = $this->nodeNameResolver->getName($classLikeName); - if ($name === null) { - continue; - } - - $this->resolvedNodeNames[$name][] = new NameAndParent($classLikeName, $classLike); - } - } - - private function resolveTraitUseNames(Node $searchNode): void - { - /** @var Identifier[] $identifiers */ - $identifiers = $this->betterNodeFinder->findInstanceOf($searchNode, Identifier::class); - - foreach ($identifiers as $identifier) { - $parentNode = $identifier->getAttribute(AttributeKey::PARENT_NODE); - if (! $parentNode instanceof UseUse) { - continue; - } - - $this->resolvedNodeNames[$identifier->name][] = new NameAndParent($identifier, $parentNode); - } - } -} diff --git a/rules/CodingStyle/Rector/Use_/RemoveUnusedAliasRector.php b/rules/CodingStyle/Rector/Use_/RemoveUnusedAliasRector.php index c195e900e22..93cffe091e5 100644 --- a/rules/CodingStyle/Rector/Use_/RemoveUnusedAliasRector.php +++ b/rules/CodingStyle/Rector/Use_/RemoveUnusedAliasRector.php @@ -12,11 +12,11 @@ use Rector\CodingStyle\ClassNameImport\ClassNameImportSkipper; use Rector\CodingStyle\Naming\NameRenamer; use Rector\CodingStyle\Node\DocAliasResolver; -use Rector\CodingStyle\Node\UseManipulator; use Rector\CodingStyle\Node\UseNameAliasToNameResolver; -use Rector\CodingStyle\ValueObject\NameAndParent; use Rector\Core\PhpParser\NodeFinder\FullyQualifiedFromUseFinder; use Rector\Core\Rector\AbstractRector; +use Rector\NameImporting\NodeAnalyzer\UseAnalyzer; +use Rector\NameImporting\ValueObject\NameAndParent; use Rector\NodeTypeResolver\Node\AttributeKey; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -27,9 +27,9 @@ final class RemoveUnusedAliasRector extends AbstractRector { /** - * @var NameAndParent[][] + * @var array */ - private array $resolvedNodeNames = []; + private array $resolvedNamesAndParentsByShortName = []; /** * @var array @@ -43,7 +43,7 @@ final class RemoveUnusedAliasRector extends AbstractRector public function __construct( private DocAliasResolver $docAliasResolver, - private UseManipulator $useManipulator, + private UseAnalyzer $useAnalyzer, private UseNameAliasToNameResolver $useNameAliasToNameResolver, private NameRenamer $nameRenamer, private ClassNameImportSkipper $classNameImportSkipper, @@ -99,15 +99,18 @@ public function refactor(Node $node): ?Node return null; } - $this->resolvedNodeNames = $this->useManipulator->resolveUsedNameNodes($searchNode); + $this->resolvedNamesAndParentsByShortName = $this->useAnalyzer->resolveUsedNameNodes($searchNode); $this->resolvedDocPossibleAliases = $this->docAliasResolver->resolve($searchNode); - $this->useNamesAliasToName = $this->useNameAliasToNameResolver->resolve($this->file); // lowercase $this->resolvedDocPossibleAliases = $this->lowercaseArray($this->resolvedDocPossibleAliases); - $this->resolvedNodeNames = array_change_key_case($this->resolvedNodeNames, CASE_LOWER); + $this->resolvedNamesAndParentsByShortName = array_change_key_case( + $this->resolvedNamesAndParentsByShortName, + CASE_LOWER + ); + $this->useNamesAliasToName = array_change_key_case($this->useNamesAliasToName, CASE_LOWER); foreach ($node->uses as $use) { @@ -125,7 +128,7 @@ public function refactor(Node $node): ?Node } // only last name is used → no need for alias - if (isset($this->resolvedNodeNames[$lowercasedLastName])) { + if (isset($this->resolvedNamesAndParentsByShortName[$lowercasedLastName])) { $use->alias = null; continue; } @@ -147,16 +150,6 @@ private function shouldSkipUse(Use_ $use): bool return ! $this->hasUseAlias($use); } - private function resolveSearchNode(Use_ $use): ?Node - { - $searchNode = $use->getAttribute(AttributeKey::PARENT_NODE); - if ($searchNode !== null) { - return $searchNode; - } - - return $use->getAttribute(AttributeKey::NEXT_NODE); - } - /** * @param string[] $values * @return string[] @@ -173,7 +166,7 @@ private function shouldSkip(Use_ $use, string $lastName, string $aliasName): boo $loweredAliasName = strtolower($aliasName); // both are used → nothing to remove - if (isset($this->resolvedNodeNames[$loweredLastName], $this->resolvedNodeNames[$loweredAliasName])) { + if (isset($this->resolvedNamesAndParentsByShortName[$loweredLastName], $this->resolvedNamesAndParentsByShortName[$loweredAliasName])) { return true; } @@ -206,11 +199,14 @@ private function refactorAliasName(Use_ $use, string $fullUseUseName, string $la } $loweredFullUseUseName = strtolower($fullUseUseName); - if (! isset($this->resolvedNodeNames[$loweredFullUseUseName])) { + if (! isset($this->resolvedNamesAndParentsByShortName[$loweredFullUseUseName])) { return; } - $this->nameRenamer->renameNameNode($this->resolvedNodeNames[$loweredFullUseUseName], $lastName); + $this->nameRenamer->renameNameNode( + $this->resolvedNamesAndParentsByShortName[$loweredFullUseUseName], + $lastName + ); $useUse->alias = null; } @@ -224,4 +220,14 @@ private function hasUseAlias(Use_ $use): bool return false; } + + private function resolveSearchNode(Use_ $use): ?Node + { + $searchNode = $use->getAttribute(AttributeKey::PARENT_NODE); + if ($searchNode !== null) { + return $searchNode; + } + + return $use->getAttribute(AttributeKey::NEXT_NODE); + } }