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

[Php74] Do not remove array of type[] doc type on TypedPropertyRector when php 8.0 feature enabled #1811

Merged
merged 16 commits into from
Feb 13, 2022

Conversation

samsonasik
Copy link
Member

ref deprecated-packages/symplify#3928 (comment)

It currently var doc with string[] type, which should not be removed.

@samsonasik
Copy link
Member Author

Fixed 🎉 by check SpacingAwareArrayTypeNode inside BracketsAwareUnionTypeNode is not mixed

@samsonasik
Copy link
Member Author

Actually, even mixed[] need to be kept so when checked with phpstan, it will be ok, ref

  1. https://phpstan.org/r/17e60422-665a-4cda-9e56-802d8459061f without @var , error notice
  2. https://phpstan.org/r/a3db29e8-a9ff-41fe-ba56-495ccb1bca3b with @var, no error notice

@samsonasik
Copy link
Member Author

Fixed, keep mixed[] as well.

@samsonasik samsonasik force-pushed the do-not-reomev-array-of-string-type branch from f8bc4eb to 0fe6ca2 Compare February 13, 2022 02:44
@samsonasik samsonasik changed the title [Php74] Do not remove array of string doc type on TypedPropertyRector when php 8.0 feature enabled [Php74] Do not remove array of type[] doc type on TypedPropertyRector when php 8.0 feature enabled Feb 13, 2022
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@samsonasik samsonasik force-pushed the do-not-reomev-array-of-string-type branch from 5385992 to c7c5779 Compare February 13, 2022 04:57
@samsonasik
Copy link
Member Author

I am merging it ;)

@samsonasik samsonasik merged commit 9077ef4 into main Feb 13, 2022
@samsonasik samsonasik deleted the do-not-reomev-array-of-string-type branch February 13, 2022 12:37
@TomasVotruba
Copy link
Member

Thanks 👍

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