Add more modifier checks to MethodDeclaration node #306
Conversation
|
|
|
Thanks for the PR! Can you tell me about why you want this change or what you are using this parser for? |
|
I'm working on a PHP linter that uses this parser, and I need to be able to query the nodes more easily. I originally was decorating the nodes, but decided to try a PR to see how receptive you'd be to this kind of improvement. Sorry about the really sparse PR! |
roblourens
left a comment
roblourens
left a comment
There was a problem hiding this comment.
I think this makes sense, thanks!
| } | ||
|
|
||
| public function isPublic() : bool { | ||
| return $this->hasModifier(TokenKind::PublicKeyword); |
There was a problem hiding this comment.
class X { function test() {}} still has an implicitly public method X::test(), but this returns false - I can see this being misunderstood in applications using this helper, for that edge case.
This is low priority for me, though
There was a problem hiding this comment.
Yeah that's a good point. Could change isPublic to just check !isPrivate but it might be better to just expose hasModifier, I think the goal is to expose the AST but not necessarily "interpret" it...
There was a problem hiding this comment.
It might be useful to have a link to projects using tolerant-php-parser in the documentation, as examples for people writing their own applications/libraries.
E.g. there's https://github.com/tysonandre/tolerant-php-parser-to-php-ast (part of Phan), https://github.com/felixfbecker/php-language-server , etc.
The first one is notable in trying to convert all of the information provided by various node types.
There was a problem hiding this comment.
I made that change and just left hasModifier, FYI @inxilpro
There was a problem hiding this comment.
I could also see changing these to hasPublicModifier()/etc to be more clear about what question it's answering.
4 participants
No description provided.