Skip to content

bug: Fix parsing of edge cases phpdoc types#6977

Merged
Wirone merged 15 commits intoPHP-CS-Fixer:masterfrom
mvorisek:fix_type_parser
May 22, 2023
Merged

bug: Fix parsing of edge cases phpdoc types#6977
Wirone merged 15 commits intoPHP-CS-Fixer:masterfrom
mvorisek:fix_type_parser

Conversation

@mvorisek
Copy link
Copy Markdown
Contributor

@mvorisek mvorisek commented May 19, 2023

heavily inspired by phpstan phpdoc grammar and its test cases

@mvorisek mvorisek changed the title Fix parsing of edge cases phpdoc types bug: Fix parsing of edge cases phpdoc types May 19, 2023
@mvorisek mvorisek marked this pull request as draft May 19, 2023 19:51
@mvorisek mvorisek force-pushed the fix_type_parser branch 2 times, most recently from 8e076cc to e134568 Compare May 19, 2023 20:27
@mvorisek mvorisek marked this pull request as ready for review May 19, 2023 21:00
@mvorisek
Copy link
Copy Markdown
Contributor Author

mvorisek commented May 19, 2023

PR is done, with #6796, #6798, #6961, #6962 it fixes parsing of almost every valid phpdoc.

I hope a new minor version can be released after this PR soon. Thank you!

@mvorisek mvorisek force-pushed the fix_type_parser branch 2 times, most recently from 8712f9f to b05ec6c Compare May 19, 2023 22:29

/**
* @return iterable<string[]|(string|null|array<string, string>)[]>
* @return iterable<(null|array<string, string>|string)[]|string[]>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't actually:

Suggested change
* @return iterable<(null|array<string, string>|string)[]|string[]>
* @return iterable<array<null|string|array<string, string>>>

??

I know it's not a part of this PR, but looking at it, seems overcomplicated. Not a something you should change, I would want to discuss it and create separate PR if needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best phpdoc here would be using array shape, ie. iterable<array{ string, null|string, null|array<string, string> }>.

Copy link
Copy Markdown
Member

@Wirone Wirone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kubawerlos please take a look too 🙂.

@Wirone Wirone requested a review from kubawerlos May 20, 2023 14:21
Copy link
Copy Markdown
Member

@kubawerlos kubawerlos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are convincing me, the only thing I'm afraid that one day @julienfalque will come and will ask "what the heck did you do to the coll regex I've created?" :D

@Wirone Wirone merged commit 60d6c21 into PHP-CS-Fixer:master May 22, 2023
@Wirone
Copy link
Copy Markdown
Member

Wirone commented May 22, 2023

Thank you @mvorisek 🍻

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants