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

Can't get AnnotationToAttribute to work. #8384

Closed
plandolt opened this issue Jan 3, 2024 · 10 comments · Fixed by rectorphp/rector-symfony#557
Closed

Can't get AnnotationToAttribute to work. #8384

plandolt opened this issue Jan 3, 2024 · 10 comments · Fixed by rectorphp/rector-symfony#557
Assignees

Comments

@plandolt
Copy link

plandolt commented Jan 3, 2024

I have a symfony application and want to update all the Annotation-Route use statements with the new Attribute-Route use statement.

I installed rector in a subdirectory (tools/rector) but have my rector.php configuration in the top project level directory with that content:

$rectorConfig->paths([
    __DIR__.'/src',
]);

$rectorConfig->symfonyContainerXml(__DIR__.'/var/cache/dev/App_KernelDevDebugContainer.xml');

$rectorConfig->sets([
    SymfonySetList::ANNOTATIONS_TO_ATTRIBUTES,
]);

When I run:
tools/rector/vendor/bin/rector process --dry-run

I just get nothing:
[OK] Rector is done!

Do you have any hints to point me in the right direction?

@TomasVotruba
Copy link
Member

Hi,

I'm thinking of possible requirements:

  • composer must have 8.0+
  • PHP version must have 8.0+
  • Rector has to be installed as normal dependency, so autoload works
  • the Symfony attributes must exist in /vendor, they're attribute only since specific Symfony version, not since PHP 8.0 only

If nothing helps, please share the rector.php and minimal reproducer repository on Github. We'll check it 👍

@plandolt
Copy link
Author

plandolt commented Jan 4, 2024

I had PHP 8.2 on the webapp and the rector subdirectory. I then tried to install rector as a direct dev dependency to the webapp but still no luck. The attribute Class exists as I use symfony 6.4.

I created a small demo with a freshly installed symfony (--version=lts --webapp) here to test: https://github.com/scuben/rector-demo-issue-8384

vendor/bin/rector process --dry-run
[OK] Rector is done!

Despite https://github.com/scuben/rector-demo-issue-8384/blob/main/src/Controller/DummyController.php#L7 should be changed to the Attribute Version.

@TomasVotruba
Copy link
Member

Thanks for sharing! We'll be able to figure this out soon 👍

It seems the https://github.com/scuben/rector-demo-issue-8384/blob/main@%7B2024-01-04T08:46:43Z%7D/src/Controller/DummyController.php#L11 is already an #[Attribute].

Could you send the link to the @annotation form?

@plandolt
Copy link
Author

plandolt commented Jan 4, 2024

Ah, I might completely misunderstand the rule.

My goal is to change the use statement from use Symfony\Component\Routing\Annotation\Route; to use Symfony\Component\Routing\Attribute\Route;

Is there a rule for that kind of migration?

@samsonasik
Copy link
Member

You can enable $rectorConfig->importNames() and the use statement will be replaced :)

@plandolt
Copy link
Author

plandolt commented Jan 4, 2024

I updated the rector.php file but I still have no proposed changes. Do you have any idea why?

@samsonasik samsonasik reopened this Jan 4, 2024
@samsonasik
Copy link
Member

Are you sure you verify Symfony\Component\Routing\Annotation\Route to Symfony\Component\Routing\Attribute\Route registered in config and changed in the method?

@samsonasik
Copy link
Member

I see, it seems because the method already use attribute, so it can't change to attribute again.

I suggest to use RenameClassRector to rename existing attribute.

@plandolt
Copy link
Author

plandolt commented Jan 4, 2024

Great, that was the hint I needed. With the RenameClassRector I could achieve what I planned to do. Thank you for your support!

@TomasVotruba
Copy link
Member

@scuben I see now, thanks for feedback.
I'll see how can we integrate it to attribute set itself to avoid this extra work.

Reopening for myself to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants