Skip to content

Add multiline generic support #44

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

Merged
merged 5 commits into from
May 6, 2020
Merged

Add multiline generic support #44

merged 5 commits into from
May 6, 2020

Conversation

icanhazstring
Copy link
Contributor

Solve #42

@@ -3046,6 +3046,30 @@ public function provideRealWorldExampleData(): \Iterator
),
]),
];

yield [
Copy link
Member

Choose a reason for hiding this comment

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

Please add these tests that check for:

  1. That trailing comma after last element is allowed
  2. That leading comma at the beginning of the line is also allowed (I know it's ugly but I've seen it in the wild).

Thank you, otherwise 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, sure thing. Will do 👍

Copy link
Contributor Author

@icanhazstring icanhazstring May 5, 2020

Choose a reason for hiding this comment

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

Just a quick question, trailing comma should not work at all am I right?
array<A, B,> is not a correct notation

Where as the leading one should be fine, as long as there are two types given

array<
   A
   , B
>

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Kotlin allows trailing comma in generic declarations since 1.4).

Copy link
Member

Choose a reason for hiding this comment

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

Trailing commas are getting popular and even supported by PHP itself. Since this works: https://phpstan.org/r/e1c07184-7f6d-48ba-99cd-29dd41fe5349, I'm also fine if it will work in generics.

Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't support multiple trailing commas: A,B,, - that should be an error, otherwise they're fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for informations. Will add support for them then :)

@icanhazstring icanhazstring requested a review from ondrejmirtes May 6, 2020 06:50
@ondrejmirtes
Copy link
Member

Awesome, love it :)

@ondrejmirtes ondrejmirtes merged commit 44500ca into phpstan:master May 6, 2020
@ondrejmirtes
Copy link
Member

Released as 0.4.7, but will probably take a few days to be baked into the next release of PHPStan :)

@icanhazstring
Copy link
Contributor Author

Thanks 🥳

@icanhazstring icanhazstring deleted the feature/multiline-generics branch May 6, 2020 06:57
rvanvelzen added a commit to rvanvelzen/phpdoc-parser that referenced this pull request Mar 28, 2022
Closes phpstan#46. Tests are basically copied and modified from phpstan#44
ondrejmirtes pushed a commit to rvanvelzen/phpdoc-parser that referenced this pull request Mar 28, 2022
Closes phpstan#46. Tests are basically copied and modified from phpstan#44
ondrejmirtes pushed a commit that referenced this pull request Mar 28, 2022
Closes #46. Tests are basically copied and modified from #44
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