Skip to content

Commit

Permalink
[AutoImport][PostRector] Handle duplicated import on namespaced class…
Browse files Browse the repository at this point in the history 
… on UseAddingPostRector (#3461)

* [AutoImport] Handle duplicated import on namespaced class on ReplaceSensioRouteAnnotationWithSymfonyRector

* [ci-review] Rector Rectify

* moving to early clean before add to namespace or non-namespace

* [ci-review] Rector Rectify

* removing use under namespace

* Fixed 🎉

* Fix phpstan

* Final touch: clean up

* Final touch: clean up

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
@samsonasik @actions-user
samsonasik and actions-user committed Mar 7, 2023
1 parent b3da981 commit f9900dd
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 27 deletions.
48 changes: 34 additions & 14 deletions packages/PostRector/Rector/UseAddingPostRector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,39 +52,59 @@ public function beforeTraverse(array $nodes): array
$useImportTypes = $this->useNodesToAddCollector->getObjectImportsByFilePath($file->getFilePath());
$functionUseImportTypes = $this->useNodesToAddCollector->getFunctionImportsByFilePath($file->getFilePath());

$oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
$removedUses = $this->renamedClassesDataCollector->getOldClasses();

// nothing to import or remove
if ($useImportTypes === [] && $functionUseImportTypes === [] && $oldToNewClasses === []) {
if ($useImportTypes === [] && $functionUseImportTypes === [] && $removedUses === []) {
return $nodes;
}

/** @var FullyQualifiedObjectType[] $useImportTypes */
$useImportTypes = $this->typeFactory->uniquateTypes($useImportTypes);

// A. has namespace? add under it
$namespace = $this->betterNodeFinder->findFirstInstanceOf($nodes, Namespace_::class);
if ($namespace instanceof Namespace_) {
// first clean
// then add, to prevent adding + removing false positive of same short use
$this->useImportsAdder->addImportsToNamespace($namespace, $useImportTypes, $functionUseImportTypes);
$firstNode = $nodes[0];
if ($firstNode instanceof FileWithoutNamespace) {
$nodes = $firstNode->stmts;
}

$namespace = $this->betterNodeFinder->findFirstInstanceOf($nodes, Namespace_::class);
if (! $firstNode instanceof FileWithoutNamespace && ! $namespace instanceof Namespace_) {
return $nodes;
}

$firstNode = $nodes[0];
if ($namespace instanceof Namespace_) {
// clean namespace stmts, don't assign, this used to clean the stmts of Namespace_
$this->useImportsRemover->removeImportsFromStmts($namespace->stmts, $removedUses);
}

if ($firstNode instanceof FileWithoutNamespace) {
$nodes = $firstNode->stmts;
// clean no-namespace stmts, assign
$nodes = $this->useImportsRemover->removeImportsFromStmts($nodes, $removedUses);
}

$removedShortUses = $this->renamedClassesDataCollector->getOldClasses();
return $this->resolveNodesWithImportedUses($nodes, $useImportTypes, $functionUseImportTypes, $namespace);
}

/**
* @param Stmt[] $nodes
* @param FullyQualifiedObjectType[] $useImportTypes
* @param FullyQualifiedObjectType[] $functionUseImportTypes
* @return Stmt[]
*/
private function resolveNodesWithImportedUses(array $nodes, array $useImportTypes, array $functionUseImportTypes, ?Namespace_ $namespace): array
{
// 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($namespace, $useImportTypes, $functionUseImportTypes);

return $nodes;
}

// B. no namespace? add in the top
// first clean
$nodes = $this->useImportsRemover->removeImportsFromStmts($nodes, $removedShortUses);
$useImportTypes = $this->filterOutNonNamespacedNames($useImportTypes);
// then add, to prevent adding + removing false positive of same short use

// then add, to prevent adding + removing false positive of same short use
return $this->useImportsAdder->addImportsToStmts($nodes, $useImportTypes, $functionUseImportTypes);
}

Expand Down
1 change: 1 addition & 0 deletions phpstan.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ parameters:
- rules/CodingStyle/Rector/ClassMethod/FuncGetArgsToVariadicParamRector.php
- packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php
- packages/PhpDocParser/PhpDocParser/PhpDocNodeTraverser.php
- packages/PostRector/Rector/UseAddingPostRector.php

-
message: '#Method call return value that should be used, but is not#'
Expand Down
24 changes: 16 additions & 8 deletions rules/CodingStyle/Application/UseImportsRemover.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,23 @@
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Use_;
use Rector\Core\Configuration\RectorConfigProvider;
use Rector\NodeRemoval\NodeRemover;

final class UseImportsRemover
{
public function __construct(private readonly RectorConfigProvider $rectorConfigProvider)
public function __construct(
private readonly RectorConfigProvider $rectorConfigProvider,
private readonly NodeRemover $nodeRemover
)
{
}

/**
* @param Stmt[] $stmts
* @param string[] $removedShortUses
* @param string[] $removedUses
* @return Stmt[]
*/
public function removeImportsFromStmts(array $stmts, array $removedShortUses): array
public function removeImportsFromStmts(array $stmts, array $removedUses): array
{
/**
* Verify import name to cover conflict on rename+import,
Expand All @@ -34,7 +38,7 @@ public function removeImportsFromStmts(array $stmts, array $removedShortUses): a
continue;
}

$this->removeUseFromUse($removedShortUses, $stmt);
$this->removeUseFromUse($removedUses, $stmt);

// nothing left → remove
if ($stmt->uses === []) {
Expand All @@ -46,16 +50,20 @@ public function removeImportsFromStmts(array $stmts, array $removedShortUses): a
}

/**
* @param string[] $removedShortUses
* @param string[] $removedUses
*/
private function removeUseFromUse(array $removedShortUses, Use_ $use): void
private function removeUseFromUse(array $removedUses, Use_ $use): void
{
foreach ($use->uses as $usesKey => $useUse) {
foreach ($removedShortUses as $removedShortUse) {
if ($useUse->name->toString() === $removedShortUse) {
foreach ($removedUses as $removedUse) {
if ($useUse->name->toString() === $removedUse) {
unset($use->uses[$usesKey]);
}
}
}

if ($use->uses === []) {
$this->nodeRemover->removeNode($use);
}
}
}
6 changes: 1 addition & 5 deletions src/Console/Command/SetupCICommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
);

$this->symfonyStyle->newLine();

$repositoryNewSecretsLink = sprintf('https://github.com/%s/settings/secrets/actions/new', $currentRepository);
$this->symfonyStyle->writeln(
'2) Add the token to Action secrets as "ACCESS_TOKEN":' . \PHP_EOL . $repositoryNewSecretsLink
Expand Down Expand Up @@ -121,9 +122,4 @@ private function resolveCurrentRepositoryName(string $currentDirectory): ?string
$match = Strings::match($output, self::GITHUB_REPOSITORY_REGEX);
return $match['repository_name'] ?? null;
}

private function createClickableLink(string $url): string
{
return sprintf('<href=%s>%s</>', $url, $url);
}
}
31 changes: 31 additions & 0 deletions tests/Issues/Issue6496/Fixture/namespaced_import_doc_after_rename.php.inc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace App;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;

/**
* @Route("/someUrl")
*/
class SomeController extends Controller
{
}

?>
-----
<?php

namespace App;

use Symfony\Component\Routing\Annotation\Route;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;

/**
* @Route("/someUrl")
*/
class SomeController extends Controller
{
}

?>

0 comments on commit f9900dd

Please sign in to comment.