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

fix: FQCN parse phpdoc using full grammar regex #7649

Merged
merged 12 commits into from
Jan 3, 2024

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Dec 31, 2023

This PR fixes all issues with partly parsed phpdoc type.

in short, it matches the type correctly using full grammar regex and then the parsed match is tested if it is a simple type:

@mvorisek mvorisek requested a review from keradus January 1, 2024 22:36
@mvorisek mvorisek requested a review from keradus January 2, 2024 08:59
@mvorisek mvorisek force-pushed the fqcn_parse_phpdoc_using_regex branch from cd5d1fa to bbd8bc9 Compare January 2, 2024 10:34
@mvorisek mvorisek marked this pull request as draft January 2, 2024 10:36
@mvorisek mvorisek force-pushed the fqcn_parse_phpdoc_using_regex branch 2 times, most recently from 5844908 to b21f564 Compare January 2, 2024 10:44
@mvorisek mvorisek marked this pull request as ready for review January 2, 2024 10:49
Copy link
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.

I found a few minutes to look at this, here's some minor doubts 🙂.

@mvorisek mvorisek requested a review from Wirone January 2, 2024 15:14
@Wirone
Copy link
Member

Wirone commented Jan 2, 2024

@mvorisek this PR does not provide support for multiline phpDoc types, right? Is it actually possible to handle them with the regex approach, since * would be there for each line, "breaking" the actual type?

@mvorisek
Copy link
Contributor Author

mvorisek commented Jan 2, 2024

@mvorisek this PR does not provide support for multiline phpDoc types, right? Is it actually possible to handle them with the regex approach, since * would be there for each line, "breaking" the actual type?

2x right

Copy link
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.

👍 from me, but we need to wait for @keradus to verify whether his doubts were addressed properly.

@mvorisek mvorisek force-pushed the fqcn_parse_phpdoc_using_regex branch from 999bff8 to df78046 Compare January 3, 2024 16:49
@mvorisek
Copy link
Contributor Author

mvorisek commented Jan 3, 2024

Can this PR be merged? I need it to finish #7648 and #7659.

@@ -586,7 +586,7 @@ function validate(): void {}
*/
function validate(): void {}
',
['import_symbols' => true],
['import_symbols' => true, 'phpdoc_tags' => ['link']],
Copy link
Member

Choose a reason for hiding this comment

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

i would be wondering why link is not there by default, but probably other discussion :D

Comment on lines +1364 to +1372
yield 'PHPDoc with generics must not crash' => [
'<?php

/**
* @param \Iterator<mixed, \SplFileInfo> $iter
*/
function foo($iter) {}',
];

Copy link
Member

Choose a reason for hiding this comment

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

side note:
with recently introduced "test class is contract", while adding support for generics, you would need to put it behind rule option, as otherwise it would change contract behaviour.

Copy link
Member

@keradus keradus left a comment

Choose a reason for hiding this comment

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

looking good!

@Wirone Wirone merged commit 8d6b23e into PHP-CS-Fixer:master Jan 3, 2024
25 checks passed
@keradus
Copy link
Member

keradus commented Jan 3, 2024

@mvorisek if great work of yours are not getting attention for longer period, please do not hesitate to poke on us, but you may give us more time than one day before pushing 😅
unfortunately, capacity of everyone is limited and we don't have enough of it to focus on everything in no-time :(

@mvorisek mvorisek deleted the fqcn_parse_phpdoc_using_regex branch January 3, 2024 21:14
danog pushed a commit to zoonru/PHP-CS-Fixer that referenced this pull request Feb 2, 2024
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.

None yet

3 participants