Skip to content

Convert Doctrine annotations with arguments on the next line - #8907

Open
ondrejmirtes wants to merge 1 commit into
rectorphp:mainfrom
ondrejmirtes:doctrine-parentheses-next-line
Open

ondrejmirtes wants to merge 1 commit into
rectorphp:mainfrom
ondrejmirtes:doctrine-parentheses-next-line

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Contributor

Merge this if you think it’s worth it, I’m not sure personally.

Follow-up to #8881, reported by @bbrala in #8881 (comment).

Doctrine skips the whitespace and * between the tag name and (. So these two annotations have the same arguments:

/**
 * @Action
 * (id = "next_line")
 */

/**
 * @Action(id = "next_line")
 */

phpdoc-parser reads the arguments only when ( comes directly after the tag name. AnnotationToAttributeRector turned the first annotation into #[Action] and dropped the arguments. This happened before #8881 too: Rector 2.8.3 does the same.

For a tag of an annotation class, DoctrineAnnotationDecorator now parses the arguments on the next line with phpdoc-parser. When they do not parse, Rector keeps the tag as it is and does not create an attribute without arguments. An example is a nested annotation on its own line. Before, this made two unrelated attributes.

The change is in Rector and not in phpdoc-parser. phpdoc-parser does not know which tags are annotations: a tag like @todo with (maybe) fix this on the next line would become a Doctrine node.

Doctrine skips the whitespace and "*" between the tag name and "(", so
@action with (id="value") on the next line has the same arguments as
@action(id="value"). phpdoc-parser reads the arguments only when "("
follows the tag name directly, and AnnotationToAttributeRector created an
attribute without arguments and dropped them.

For a tag of an annotation class, the decorator now parses the arguments on
the next line. When they do not parse, e.g. a nested annotation on its own
line, the tag stays as it is.

Amp-Thread-ID: https://ampcode.com/threads/T-01a197b0-0078-7305-a757-b15baab1dfdf
@samsonasik

Copy link
Copy Markdown
Member

@bbrala could you verify this? Thank you

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants