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

Transform parameter labels while changing Annotation to Attribute #6825

Closed
rvanlaak opened this issue Nov 23, 2021 · 6 comments
Closed

Transform parameter labels while changing Annotation to Attribute #6825

rvanlaak opened this issue Nov 23, 2021 · 6 comments
Labels

Comments

@rvanlaak
Copy link

rvanlaak commented Nov 23, 2021

Thanks for the PHPWVL talk!

Feature Request

Hook into the process to allow transforming the parameter labels during the annotation to attribute conversion.

Maybe, modifying the data values even could be useful?

Diff

- /* @Annotation(deprecatedAttributeArray={subParameter=1, otherSubParameter='test'}) */
+ #[Annotation(subParameterWithOtherLabel=1, otherSubParameterWithSuffix='test')]

Additional context

For APYBreadcrumbTrailBundle we introduced Attribute support with v1.7, and trigger deprecation warnings once the platform is PHP >= 8

With APY/APYBreadcrumbTrailBundle#76 we are thinking about including a script to help users with upgrading their implementations. The PHPWVL talk gave the additional idea of even providing it as an optional feature through a CLI question as a Composer Plugin!

@TomasVotruba
Copy link
Member

Hi, thanks for having us :)

This can be split into 2 rules, one generic and one custom:

@rvanlaak
Copy link
Author

rvanlaak commented Nov 29, 2021

Thanks, will try that. Will it not fail on the creation of the attribute, given the attribute's typehints on the parameters?

@Breadcrumb("route={absolute=true}")

to

#[Breadcrumb(routeAbsolute: true)

@TomasVotruba
Copy link
Member

TomasVotruba commented Nov 29, 2021

First generic rule will convert annotations to attributes as they are:

-@Breadcrumb("route={absolute]true}")
+#[Breadcrumb(route: '{absolute]true}')]

Then, your custom rule will modify attribute to meet your needs:

-#[Breadcrumb(route: '{absolute]true}')]
+#[Breadcrumb(routeAbsolute: true)]

@rvanlaak
Copy link
Author

The old annotation used to get all it's data via a single "data" array parameter on the constructor. That array now is gone, and each array key option now is it's own constructor parameter.

In addition, the attribute constructor does not have the route parameter (which also was an array before). The constructor instead has an routeAbsolute parameter with type boolean.

In other words, the question would be whether an exception will happen after the first generic rule gets applied, as route can exist as part of the data array but not exist as a field on the constructor.

The annotation->attribute change is BC, so the attribute's first parameter also allows the old "data array" as value. So, I will try to create a configuration to 1) first puts all attribute parameters in an array, so the attribute can get constructed, and 2) then rewrite the constructor's data array parameter to separate parameters.

1)

- @Breadcrumb("title="sample", route={absolute=true, parameters={id="{id}"}}")
+ #[Breadcrumb(title: [title: 'sample', route: '{absolute=true, parameters={id="{id}"}}')]

2)

- #[Breadcrumb(title: [title: 'sample', route: '{absolute=true, parameters={id="{id}"}}')]
+ #[Breadcrumb(title: 'sample', routeAbsolute: true, routeParameters: [id =>"{id}"])]

Will start experimenting with it in the next couple of days. Let's see what it takes to provide bundles with an major-version-upgrade-script 🚀

@TomasVotruba
Copy link
Member

TomasVotruba commented Nov 29, 2021

I'm checking your example and there is no BC break needed. Unless you need change behavior, annotation and attribute classes are identical in provided array.

I've added test fixutre to generic rule that includes nestted data:
https://github.com/rectorphp/rector-src/pull/1342/files

-@GenericAnnotation(title="sample", route={absolute=true, parameters={id="{id}"}}))
+#[GenericAnnotation(title: 'sample', route: ['absolute' => true, 'parameters' => ['id' => '{id}']])]

Here is a post about how to change the annotation class before refactoring to attributes, to keep both compatible: https://tomasvotruba.com/blog/doctrine-annotations-and-attributes-living-together-in-peace/

@TomasVotruba
Copy link
Member

Closing as answered

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

No branches or pull requests

2 participants