Skip to content

feature: Parse parenthesized & conditional phpdoc type#6796

Merged
Wirone merged 1 commit intoPHP-CS-Fixer:masterfrom
mvorisek:parse_conditional_phpdoc
Apr 20, 2023
Merged

feature: Parse parenthesized & conditional phpdoc type#6796
Wirone merged 1 commit intoPHP-CS-Fixer:masterfrom
mvorisek:parse_conditional_phpdoc

Conversation

@mvorisek
Copy link
Copy Markdown
Contributor

@mvorisek mvorisek commented Feb 24, 2023

@mvorisek mvorisek changed the title Parse parenthesized & conditional phpdoc type bug: Parse parenthesized & conditional phpdoc type Feb 24, 2023
@mvorisek mvorisek changed the title bug: Parse parenthesized & conditional phpdoc type feature: Parse parenthesized & conditional phpdoc type Feb 25, 2023
@mvorisek mvorisek force-pushed the parse_conditional_phpdoc branch from 31aca7e to 8e6d624 Compare February 25, 2023 00:04
@mvorisek mvorisek marked this pull request as ready for review February 25, 2023 00:06
@mvorisek mvorisek force-pushed the parse_conditional_phpdoc branch 3 times, most recently from 21d1ee5 to f308e95 Compare February 25, 2023 00:46
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.

TypeExpressionTest provides cases only for union types. Shouldn't it provide cases also for intersection and DNF types?

@mvorisek
Copy link
Copy Markdown
Contributor Author

TypeExpressionTest provides cases only for union types. Shouldn't it provide cases also for intersection and DNF types?

As the PR name says, and the impl. as well, union/intersect is untouched. But feel free to send a PR against my PR branch.

@Wirone
Copy link
Copy Markdown
Member

Wirone commented Mar 22, 2023

As the PR name says, and the impl. as well, union/intersect is untouched.

I know PR is about parsing parenthesized & conditional phpdoc type, but since DNF types use parenthesis, I believe your test cases should cover it to prove it works as it should 😉.

@mvorisek
Copy link
Copy Markdown
Contributor Author

@Wirone I added a test for intersect

@Wirone
Copy link
Copy Markdown
Member

Wirone commented Mar 28, 2023

@mvorisek that's 1/2 of what I asked 😉. I really would like to see test case with DNF types, like ($x is ((CFoo&CBar)|CBaz) ? (TFoo|(TBar&TBaz)) : (FFoo|FBar|(FBaz&FBuzz))). Since we've added 8.2 support we really should include features from this version in the tests.

@mvorisek
Copy link
Copy Markdown
Contributor Author

@Wirone please submit a PR into https://github.com/mvorisek/PHP-CS-Fixer/tree/parse_conditional_phpdoc

@mvorisek mvorisek force-pushed the parse_conditional_phpdoc branch from d94237a to 04e4237 Compare April 9, 2023 09:32
@mvorisek
Copy link
Copy Markdown
Contributor Author

mvorisek commented Apr 9, 2023

DFN test added

@mvorisek mvorisek requested a review from Wirone April 14, 2023 22:07
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.

I have only one thing to consider, but it's a minor change (not a blocker).

@mvorisek mvorisek force-pushed the parse_conditional_phpdoc branch from 04e4237 to c5cbfd6 Compare April 14, 2023 23:01
@Wirone Wirone merged commit 10084fd into PHP-CS-Fixer:master Apr 20, 2023
@Wirone
Copy link
Copy Markdown
Member

Wirone commented Apr 20, 2023

Thank you, @mvorisek 🍻

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Complex/conditional phpdoc is not parsed/indented correctly

3 participants