-
Notifications
You must be signed in to change notification settings - Fork 510
Add support for @phpstan-throws #291
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please fix the build. PHP 8 failures come from master and are fine, but your code contains errors.
- I think we should choose a different strategy - if there are @phpstan-throws tags, @throws tags should be skipped. That's for example what resolveVarTags does.
- There are no tests. I'd appreciate test for the change in InvalidPHPStanDocTagRule.php , and also something to demonstrate that the change in PhpDocNodeResolver does something at all. This is where it's currently tested: https://github.com/phpstan/phpstan-src/blob/02f4d4a0e09ddf9b6f30b022fc5357327cc0843d/tests/PHPStan/Reflection/Annotations/ThrowsAnnotationsTest.php
c8513e7
to
a0c6650
Compare
I made some updates, but I need phpstan/phpdoc-parser#51 first @ondrejmirtes :) |
a0c6650
to
c701b22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also bump phpdoc-parser in composer.json to 0.4.9 :)
src/PhpDoc/PhpDocNodeResolver.php
Outdated
}, $phpDocNode->getThrowsTagValues()); | ||
$resolved = null; | ||
|
||
foreach (['@throws', '@phpstan-throws'] as $tagName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be reversed - in case of both @throws
and @phpstan-throws
being present, @phpstan-throws
should win. Also please add a test for this situation.
c701b22
to
01cbe49
Compare
Should be ok now @ondrejmirtes |
Could you forcepush again? The build seems broken. |
Related to phpstan/phpstan#3695 (comment)
@phpstan-throws void
can avoid conflict with psalm/phan.