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
Add support for @throws annotation #994
Conversation
public function throwRuntimeException(); | ||
|
||
/** | ||
* @throws RuntimeException|LogicException |
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 syntax is not valid.
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.
Thanks, you are right. I saw this syntax in some library and phpstorm supports it too. But better to be strict...
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.
@Majkl578 I don't understand. Why is it not valid? What am I missing?
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.
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.
@Majkl578 I've read that and I don't think it disallows RuntimeException|LogicException
, because it is a type that „represent an object of the class Exception or any subclass thereof.“
I really like the idea of tracking exceptions. |
@Majkl578 Exception tracking is task for some rules. Look at my PoC https://github.com/pepakriz/phpstan-exception-rules . Over time it could be part of phpstan-strict-rules or something... But at first, I need support for |
Yes I understand. :) Just thinking out loud. I.e. how to mark the exception as checked/unchecked, that would probably need some tracking too. |
I mentioned in some related issue that there will have to be some
configuration options or an interface to determine which exceptions are
checked and unchecked.
I believe that implementation of the checkers requires custom node visitors
so it will be a subject of some future version.
I will review this PR soon.
On Tue, 8 May 2018 at 21:53, Michael Moravec ***@***.***> wrote:
Yes I understand. :) Just thinking out loud. I.e. how to mark the
exception as checked/unchecked, that would probably need some tracking too.
Would be nice in strict-rules one day.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#994 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGZuHDjR28AhOopXJBNCKEvkVdbytWkks5twfe1gaJpZM4T3Ksu>
.
--
Ondřej Mirtes
|
src/PhpDoc/PhpDocNodeResolver.php
Outdated
return $tag->value instanceof ThrowsTagValueNode; | ||
}), | ||
'value' | ||
); |
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 use currently unavailable $phpDocNode->getThrowsTagValues()
. Send PR to https://github.com/phpstan/phpdoc-parser
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.
Done: phpstan/phpdoc-parser#12
I will rebase after merge
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.
Ok, it's ready
Thank you! I'd merge this, but there needs to be cache version bump in FileTypeMapper.php 😊 Then it will be good to go. |
@ondrejmirtes it's ready now, including cache bump. Thanks! |
Thanks! |
No description provided.