Ref T4334. Allows XHPAST to parse the access-on-instanciation grammar added in 5.4 Refactors some of the other grammar to be closer to Zend's parser. Adds positive and negative tests for allowed syntax.
Details
Details
- Reviewers
epriestley btrahan - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T4334: Support PHP5.4+ syntax in XHPAST
- Commits
- rPHUe719f331aa1a: Add support in XHPAST for "(new X)->y" syntax (added in 5.4)
arc unit
Ran generate_php_symbols.php script on files that previously errored out with "unexpected T_OBJECT_OPERATOR", now there are no errors
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
One small thing inline -- does that make sense? The rest of this looks great to me. Thanks for digging in!
| support/xhpast/parser.y | ||
|---|---|---|
| 2311–2312 | I think this should either look like the parenthesis_expr rule does, and generate a n_PARENTHETICAL_EXPRESSION node, or look like the yield_expr rule does and do NEXPAND($1, $2, $3). Since the parens are required, maybe the NEXPAND is more consistent. | |
Comment Actions
Specifically, what NEXPAND does is "expand" the node to include more tokens. So the current parse will look like this, with these tokens as part of the node:
"(" "new" "a" ")"
^^^^^^^^^If you NEXPAND it, you'll get this:
"(" "new" "a" ")"
^^^^^^^^^^^^^^^^^This generally doesn't matter when working with the tree, but one practical effect is that getConcreteString() on the n_NEW node will print out "new a" in the former case, and "(new a)" in the latter case. Since the parentheses are required for the node to be valid, this seems like a sensible interpretation.
The alternative would add an n_PARENTHETICAL_EXPRESSION node, so we'd get this:
"(" "new" "a" ")"
^^^^^^^^^ n_NEW
^^^^^^^^^^^^^^^^^ n_PARENTHETICAL_EXPRESSIONThis seems like a less expected result than calling the whole thing an n_NEW to me, though.