From 505ef9cdc74e2bf3b7295f1789536b75bc668353 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 2 Nov 2023 01:10:19 +0700 Subject: [PATCH 1/7] [Php80] Handle RenameClassRector+AnnotationToAttributeRector with auto import and existing attribute defined --- .../Fixture/fixture.php.inc | 33 +++++++++++++++++++ ...ameAnnotationToAttributeAutoImportTest.php | 28 ++++++++++++++++ .../config/configured_rule.php | 33 +++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/fixture.php.inc create mode 100644 tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php create mode 100644 tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php diff --git a/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/fixture.php.inc b/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/fixture.php.inc new file mode 100644 index 00000000000..3b03945e4b4 --- /dev/null +++ b/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/fixture.php.inc @@ -0,0 +1,33 @@ + '\d+', 'networkId' => '\d+'])] +class IsGrantedController extends AbstractController +{ +} + +?> +----- + '\d+', 'networkId' => '\d+'])] +#[IsGranted('TEST')] +class IsGrantedController extends AbstractController +{ +} + +?> diff --git a/tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php b/tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php new file mode 100644 index 00000000000..5df450708ca --- /dev/null +++ b/tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php b/tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php new file mode 100644 index 00000000000..89343c21440 --- /dev/null +++ b/tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php @@ -0,0 +1,33 @@ +importNames(); + + // simulate loaded different config + $rectorConfig->ruleWithConfiguration( + RenameClassRector::class, + [ + 'SomeClass' => 'SomeOtherClass', + ], + ); + + // set annotation to attribute + $rectorConfig->ruleWithConfiguration(AnnotationToAttributeRector::class, [ + new AnnotationToAttribute('Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted'), + ]); + + // rename + $rectorConfig->ruleWithConfiguration( + RenameClassRector::class, + [ + 'Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted' => 'Symfony\Component\Security\Http\Attribute\IsGranted', + ], + ); +}; From 59b604be67bca9b8dde14ff2f214be20d3ccac2a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 2 Nov 2023 09:38:11 +0700 Subject: [PATCH 2/7] clean up --- .../Rector/NameImportingPostRector.php | 25 +------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/packages/PostRector/Rector/NameImportingPostRector.php b/packages/PostRector/Rector/NameImportingPostRector.php index 76e3d576d2d..c1f4c511d8b 100644 --- a/packages/PostRector/Rector/NameImportingPostRector.php +++ b/packages/PostRector/Rector/NameImportingPostRector.php @@ -21,7 +21,6 @@ use Rector\Comments\NodeDocBlock\DocBlockUpdater; use Rector\Core\Configuration\Option; use Rector\Core\Configuration\Parameter\SimpleParameterProvider; -use Rector\Core\Configuration\RenamedClassesDataCollector; use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace; use Rector\Core\Provider\CurrentFileProvider; use Rector\Core\ValueObject\Application\File; @@ -40,8 +39,7 @@ public function __construct( private readonly CurrentFileProvider $currentFileProvider, private readonly UseImportsResolver $useImportsResolver, private readonly AliasNameResolver $aliasNameResolver, - private readonly DocBlockUpdater $docBlockUpdater, - private readonly RenamedClassesDataCollector $renamedClassesDataCollector + private readonly DocBlockUpdater $docBlockUpdater ) { } @@ -62,7 +60,6 @@ public function enterNode(Node $node): ?Node } if ($node instanceof Name) { - $node = $this->resolveNameFromAttribute($node); return $this->processNodeName($node, $file); } @@ -89,26 +86,6 @@ public function enterNode(Node $node): ?Node return $node; } - private function resolveNameFromAttribute(Name $name): Name - { - if ($name instanceof FullyQualified) { - return $name; - } - - if (array_keys($name->getAttributes()) === [AttributeKey::PHP_ATTRIBUTE_NAME]) { - $oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses(); - $phpAttributeName = $name->getAttribute(AttributeKey::PHP_ATTRIBUTE_NAME); - - foreach ($oldToNewClasses as $oldName => $newName) { - if ($oldName === $phpAttributeName) { - return new FullyQualified($newName, $name->getAttributes()); - } - } - } - - return $name; - } - private function processNodeName(Name $name, File $file): ?Node { if ($name->isSpecialClassName()) { From 9bea98587d9472c74d5b13c11136c7de198909f6 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 2 Nov 2023 09:56:37 +0700 Subject: [PATCH 3/7] update check in old classes on skip --- .../PostRector/Rector/ClassRenamingPostRector.php | 13 +++++++++++-- .../PostRector/Rector/NameImportingPostRector.php | 4 ++++ .../FullyQualifiedNameClassNameImportSkipVoter.php | 7 +++++++ ...ture.php.inc => with_existing_attribute.php.inc} | 4 ++-- 4 files changed, 24 insertions(+), 4 deletions(-) rename tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/{fixture.php.inc => with_existing_attribute.php.inc} (87%) diff --git a/packages/PostRector/Rector/ClassRenamingPostRector.php b/packages/PostRector/Rector/ClassRenamingPostRector.php index 524005068b5..94475a7c156 100644 --- a/packages/PostRector/Rector/ClassRenamingPostRector.php +++ b/packages/PostRector/Rector/ClassRenamingPostRector.php @@ -7,6 +7,8 @@ use PhpParser\Node; use PhpParser\Node\Arg; use PhpParser\Node\Expr; +use PhpParser\Node\Name; +use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Namespace_; use PhpParser\Node\Stmt\PropertyProperty; @@ -51,8 +53,8 @@ public function beforeTraverse(array $nodes): array public function enterNode(Node $node): ?Node { - // cannot be renamed - if ($node instanceof Expr || $node instanceof Arg || $node instanceof PropertyProperty) { + // no longer need post rename + if (! $node instanceof Name) { return null; } @@ -65,6 +67,13 @@ public function enterNode(Node $node): ?Node $scope = $node->getAttribute(AttributeKey::SCOPE); $result = $this->classRenamer->renameNode($node, $oldToNewClasses, $scope); + if (! $result instanceof Name && ! $node instanceof FullyQualified) { + $phpAttributeName = $node->getAttribute(AttributeKey::PHP_ATTRIBUTE_NAME); + if (is_string($phpAttributeName)) { + return $this->classRenamer->renameNode(new FullyQualified($phpAttributeName, $node->getAttributes()), $oldToNewClasses, $scope); + } + } + if (! SimpleParameterProvider::provideBoolParameter(Option::AUTO_IMPORT_NAMES)) { return $result; } diff --git a/packages/PostRector/Rector/NameImportingPostRector.php b/packages/PostRector/Rector/NameImportingPostRector.php index c1f4c511d8b..724d32f484c 100644 --- a/packages/PostRector/Rector/NameImportingPostRector.php +++ b/packages/PostRector/Rector/NameImportingPostRector.php @@ -60,6 +60,10 @@ public function enterNode(Node $node): ?Node } if ($node instanceof Name) { + if ($node->tostring() === 'Symfony\Component\Security\Http\Attribute\IsGranted') { + d($this->processNodeName($node, $file)); + } + return $this->processNodeName($node, $file); } diff --git a/rules/CodingStyle/ClassNameImport/ClassNameImportSkipVoter/FullyQualifiedNameClassNameImportSkipVoter.php b/rules/CodingStyle/ClassNameImport/ClassNameImportSkipVoter/FullyQualifiedNameClassNameImportSkipVoter.php index b230f16db03..0b8e9bf8175 100644 --- a/rules/CodingStyle/ClassNameImport/ClassNameImportSkipVoter/FullyQualifiedNameClassNameImportSkipVoter.php +++ b/rules/CodingStyle/ClassNameImport/ClassNameImportSkipVoter/FullyQualifiedNameClassNameImportSkipVoter.php @@ -7,6 +7,7 @@ use PhpParser\Node; use Rector\CodingStyle\ClassNameImport\ShortNameResolver; use Rector\CodingStyle\Contract\ClassNameImport\ClassNameImportSkipVoterInterface; +use Rector\Core\Configuration\RenamedClassesDataCollector; use Rector\Core\ValueObject\Application\File; use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType; @@ -23,6 +24,7 @@ final class FullyQualifiedNameClassNameImportSkipVoter implements ClassNameImpor { public function __construct( private readonly ShortNameResolver $shortNameResolver, + private readonly RenamedClassesDataCollector $renamedClassesDataCollector ) { } @@ -31,12 +33,17 @@ public function shouldSkip(File $file, FullyQualifiedObjectType $fullyQualifiedO // "new X" or "X::static()" /** @var array $shortNamesToFullyQualifiedNames */ $shortNamesToFullyQualifiedNames = $this->shortNameResolver->resolveFromFile($file); + $removedUses = $this->renamedClassesDataCollector->getOldClasses(); foreach ($shortNamesToFullyQualifiedNames as $shortName => $fullyQualifiedName) { if ($fullyQualifiedObjectType->getShortName() !== $shortName) { continue; } + if (in_array($fullyQualifiedName, $removedUses, true)) { + continue; + } + return $fullyQualifiedObjectType->getClassName() !== $fullyQualifiedName; } diff --git a/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/fixture.php.inc b/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/with_existing_attribute.php.inc similarity index 87% rename from tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/fixture.php.inc rename to tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/with_existing_attribute.php.inc index 3b03945e4b4..1931c7a6bba 100644 --- a/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/fixture.php.inc +++ b/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/with_existing_attribute.php.inc @@ -1,6 +1,6 @@ Date: Thu, 2 Nov 2023 09:57:28 +0700 Subject: [PATCH 4/7] debug --- packages/PostRector/Rector/NameImportingPostRector.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/PostRector/Rector/NameImportingPostRector.php b/packages/PostRector/Rector/NameImportingPostRector.php index 724d32f484c..c1f4c511d8b 100644 --- a/packages/PostRector/Rector/NameImportingPostRector.php +++ b/packages/PostRector/Rector/NameImportingPostRector.php @@ -60,10 +60,6 @@ public function enterNode(Node $node): ?Node } if ($node instanceof Name) { - if ($node->tostring() === 'Symfony\Component\Security\Http\Attribute\IsGranted') { - d($this->processNodeName($node, $file)); - } - return $this->processNodeName($node, $file); } From b5c3a6ffa29465fa08cda1fdd20c0a346b4e6ecd Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Thu, 2 Nov 2023 02:58:15 +0000 Subject: [PATCH 5/7] [ci-review] Rector Rectify --- packages/PostRector/Rector/ClassRenamingPostRector.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/PostRector/Rector/ClassRenamingPostRector.php b/packages/PostRector/Rector/ClassRenamingPostRector.php index 94475a7c156..1876764912c 100644 --- a/packages/PostRector/Rector/ClassRenamingPostRector.php +++ b/packages/PostRector/Rector/ClassRenamingPostRector.php @@ -5,13 +5,10 @@ namespace Rector\PostRector\Rector; use PhpParser\Node; -use PhpParser\Node\Arg; -use PhpParser\Node\Expr; use PhpParser\Node\Name; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Namespace_; -use PhpParser\Node\Stmt\PropertyProperty; use PHPStan\Analyser\Scope; use Rector\CodingStyle\Application\UseImportsRemover; use Rector\Core\Configuration\Option; From 778c7be705a2f215fe4d3891b453ddff305629fd Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 2 Nov 2023 10:02:15 +0700 Subject: [PATCH 6/7] fix --- .../Fixture/with_existing_attribute.php.inc | 0 ...ameAnnotationToAttributeAutoImportTest.php | 28 ---------------- .../config/configured_rule.php | 33 ------------------- 3 files changed, 61 deletions(-) rename tests/Issues/{RenameAnnotationToAttributeAutoImport => AnnotationToAttributeRenameAutoImport}/Fixture/with_existing_attribute.php.inc (100%) delete mode 100644 tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php delete mode 100644 tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php diff --git a/tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/with_existing_attribute.php.inc b/tests/Issues/AnnotationToAttributeRenameAutoImport/Fixture/with_existing_attribute.php.inc similarity index 100% rename from tests/Issues/RenameAnnotationToAttributeAutoImport/Fixture/with_existing_attribute.php.inc rename to tests/Issues/AnnotationToAttributeRenameAutoImport/Fixture/with_existing_attribute.php.inc diff --git a/tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php b/tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php deleted file mode 100644 index 5df450708ca..00000000000 --- a/tests/Issues/RenameAnnotationToAttributeAutoImport/RenameAnnotationToAttributeAutoImportTest.php +++ /dev/null @@ -1,28 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php b/tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php deleted file mode 100644 index 89343c21440..00000000000 --- a/tests/Issues/RenameAnnotationToAttributeAutoImport/config/configured_rule.php +++ /dev/null @@ -1,33 +0,0 @@ -importNames(); - - // simulate loaded different config - $rectorConfig->ruleWithConfiguration( - RenameClassRector::class, - [ - 'SomeClass' => 'SomeOtherClass', - ], - ); - - // set annotation to attribute - $rectorConfig->ruleWithConfiguration(AnnotationToAttributeRector::class, [ - new AnnotationToAttribute('Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted'), - ]); - - // rename - $rectorConfig->ruleWithConfiguration( - RenameClassRector::class, - [ - 'Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted' => 'Symfony\Component\Security\Http\Attribute\IsGranted', - ], - ); -}; From 01bfee52bae1f5ec297f65aa5ad34e47e4ad98be Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 2 Nov 2023 10:02:54 +0700 Subject: [PATCH 7/7] rename fixture --- .../Fixture/with_existing_attribute.php.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Issues/AnnotationToAttributeRenameAutoImport/Fixture/with_existing_attribute.php.inc b/tests/Issues/AnnotationToAttributeRenameAutoImport/Fixture/with_existing_attribute.php.inc index 1931c7a6bba..456bb21bc6e 100644 --- a/tests/Issues/AnnotationToAttributeRenameAutoImport/Fixture/with_existing_attribute.php.inc +++ b/tests/Issues/AnnotationToAttributeRenameAutoImport/Fixture/with_existing_attribute.php.inc @@ -10,7 +10,7 @@ use Symfony\Component\Routing\Annotation\Route; * @IsGranted("TEST") */ #[Route(path: '/pro/{id}/networks/{networkId}/sectors', name: 'api_network_sectors', requirements: ['id' => '\d+', 'networkId' => '\d+'])] -class IsGrantedController extends AbstractController +class WithExistingAttribute extends AbstractController { } @@ -26,7 +26,7 @@ use Symfony\Component\Routing\Annotation\Route; #[Route(path: '/pro/{id}/networks/{networkId}/sectors', name: 'api_network_sectors', requirements: ['id' => '\d+', 'networkId' => '\d+'])] #[IsGranted('TEST')] -class IsGrantedController extends AbstractController +class WithExistingAttribute extends AbstractController { }