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

Follow recommendation about multiline type #10804

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

greg0ire
Copy link
Member

Apparently, there is consensus about multiline types between:

  • PHPStan
  • Psalm
  • Slevomat Coding Standard

See slevomat/coding-standard#1586 (comment)

Using parenthesis is supposed to be less ambiguous, although I cannot explain how it is ambiguous before… maybe if we had the pipes at the beginning of lines instead of the end it would be ambiguous because you could not clearly tell if the return is multiline or not?

Anyway, the change has a positive impact on the Psalm baseline, showing that psalm-return annotation was not really understood previously. Thanks @ondrejmirtes for the heads up!

@ondrejmirtes
Copy link
Contributor

although I cannot explain how it is ambiguous before

It's a parser. The text is always open to multiple interpretations.

When a line ends with | the parser doesn't know if it's part of the type, or of it's just a description, and whether we're supposed to parse the next line.

If you start a type with (, you can parse until you find a ).

@greg0ire
Copy link
Member Author

Makes sense! I'll amend my commit message 👍

Apparently, there is consensus about multiline types between:

- PHPStan
- Psalm
- Slevomat Coding Standard

See slevomat/coding-standard#1586 (comment)

Using parenthesis is less ambiguous, it makes it clear to the parser
where the type begins and where it ends.

The change has a positive impact on the Psalm baseline, showing
that psalm-return annotation was not really understood previously.
@derrabus derrabus added this to the 2.15.4 milestone Jun 28, 2023
@greg0ire greg0ire merged commit 584c4ae into doctrine:2.15.x Jun 28, 2023
@greg0ire greg0ire deleted the parenthesized branch June 28, 2023 12:11
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.

3 participants