Currently, there is a minor bug with XHPAST when parsing class methods with no modifiers. See D10687 for some more information.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rPHU00fe30dd9ebf: Fix a minor XHPAST bug
Specifically, consider the following:
class Foo { public function bar() {} function baz() {} }
Previously when parsing this code with xhpast we get a node of type n_METHOD_DECLARATION: "public function bar() {}" and a node of type n_METHOD_DECLARATION: "baz() {}". After this change, xhpast correctly parses this as a node of type n_METHOD_DECLARATION: "public function bar() {}" and a node of type n_METHOD_DECLARATION: "function baz() {}".
Diff Detail
Diff Detail
- Repository
- rPHU libphutil
- Branch
- master
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 3579 Build 3587: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
I think there's a cleaner fix inline?
| support/xhpast/parser.y | ||
|---|---|---|
| 1176 | I think you want: NLMORE($$, $2); ...here, instead. As written, this change will expand the range of $1 (which is n_METHOD_MODIFIER_LIST) to include the keyword "function", but I think this is not desirable: the word "function" does not seem like it should be part of the "method modifier list" to me. If you use NLMORE($$, $2) instead, you'll expand the range of $$ (the n_METHOD_DECLARATION) leftward, if $2 is defined and has a leftmost extent that is left-more than the left side of $$. Basically, you'll move the beginning of the n_METHOD_DECLARATION backward to include function if it does not already. | |
Comment Actions
Overall, you'll get the same n_METHOD_DECLARATION as this patch, but without expanding the n_METHOD_MODIFIER_LIST as a side effect.
| support/xhpast/parser.y | ||
|---|---|---|
| 1176 | Sure, that seems reasonable. I think I based this on the fact that n_CLASS_ATTRIBUTES includes class. | |
Comment Actions
Since this landed, calling getDocblockToken() on a n_METHOD_DECLARATION does not return docblock token anymore.
Comment Actions
Ah -- the order of the NLMORE() is important here, because appendChild() does not extend the tokens to the left. By calling NMLORE() first, appendChild() does not change the token range if $1 exists. I'll fix appendChild().