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.

Sign up for GitHub

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

Jump to bottom

SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingAnyTypeHint does not like linebreaks since 8.13.0 #1586

Closed
greg0ire opened this issue Jun 23, 2023 · 3 comments
Closed

SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingAnyTypeHint does not like linebreaks since 8.13.0 #1586

greg0ire opened this issue Jun 23, 2023 · 3 comments
Labels
PHPStan docparser Wontfix

Comments

@greg0ire
Copy link 

   /**
     * @return AST\BetweenExpression|
     *         AST\CollectionMemberExpression|
     *         AST\ComparisonExpression|
     *         AST\EmptyCollectionComparisonExpression|
     *         AST\ExistsExpression|
     *         AST\InExpression|
     *         AST\InstanceOfExpression|
     *         AST\LikeExpression|
     *         AST\NullComparisonExpression
     */

causes the rule in the title to kick in. The whole code can be seen at https://github.com/doctrine/orm/blob/4c3bd208018c26498e5f682aaad45fa00ea307d5/lib/Doctrine/ORM/Query/Parser.php#L2569-L2577
greg0ire added a commit to greg0ire/doctrine-orm that referenced this issue Jun 23, 2023
@greg0ire
Work around slevomat/coding-standard issues
5114dce 
I tweaked the code so that it would not fall victim to
slevomat/coding-standard#1585 or
slevomat/coding-standard#1586, thus fixing the
phpcs job without losing information or breaking other jobs.
@greg0ire greg0ire mentioned this issue Jun 23, 2023
Merged
@kukulich
Copy link
Contributor

Thanks, @ondrejmirtes will try to fix it in https://github.com/phpstan/phpdoc-parser

@kukulich kukulich added the PHPCS label Jun 24, 2023
@kukulich kukulich mentioned this issue Jun 25, 2023
Closed
@ondrejmirtes
Copy link
Contributor

Psalm maintainers recommend to parenthesize the multiline union type: vimeo/psalm#9969 (comment) Which would make it work with phpstan/phpdoc-parser and Slevomat CS too.

greg0ire added a commit to greg0ire/doctrine-orm that referenced this issue Jun 27, 2023
@greg0ire
Follow recommendation about multiline type
c8991db 
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.
@greg0ire greg0ire mentioned this issue Jun 27, 2023
Merged
greg0ire added a commit to greg0ire/doctrine-orm that referenced this issue Jun 28, 2023
@greg0ire
Follow recommendation about multiline type
c9c5157 
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.
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
PHPStan docparser Wontfix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ondrejmirtes @kukulich @greg0ire