Adds support for arrow functions from PHP 7.4 #65
Conversation
cfroystad
force-pushed
the
arrow_functions_v2
branch
from
7a8b91e
to
84e3e94
Compare
cfroystad
force-pushed
the
arrow_functions_v2
branch
from
84e3e94
to
0be9300
Compare
|
Reworked the node structure to be more consistent with the rest of the grammar. I think this is ready for review when you find the time :) |
There was a problem hiding this comment.
Code looks good, but I am a little confused as to why the parser size increased by 13k lines. Maybe @maxbrunsfeld can weigh in? Or is it the alias there?
|
By the way, I don’t think the parser increase is a show-stopper, given that PHP is one of the faster tree-sitter parsers around. Would just like to figure it out. :) |
grammar.js
Outdated
| _anonymous_arrow_function: $ => seq( | ||
| $._arrow_function_header, | ||
| field('body', alias($._expression, $.expression_statement)) | ||
| ), |
There was a problem hiding this comment.
Can you explain what this _anonymous_arrow_function is for? It seems like the same thing as arrow_function, except that there's no trailing semicolon in the body, because the body is only an expression, not an expression_statement. It makes sense that this creates a fair amount of ambiguity e.g. in cases where an arrow function is followed by a semicolon.
Just for my understanding - why not just have the body of arrow_function be an expression (like _anonymous_arrow_function is now), and remove _anonymous_arrow_function entirely? In other words, why would an arrow function ever need to have a trailing semicolon inside of it (as opposed to after it)? Could you give an example of code that wouldn't parse correctly with that structure?
There was a problem hiding this comment.
Looking at the RFC, I think arrow function bodies should always be expressions (not expression statements). I think we just need one arrow_function rule. Let me know if there's something I'm missing.
There was a problem hiding this comment.
Good questions!
Adding both of them was the only way I could find to make the parser handle ; correctly in the different cases in the beginning. However I should have revisited that before thinking the PR done, cause you're perfectly correct - they should always just be expression.
Thank you for the guidance and sorry for not solving that before bugging you :)
Code updated
|
Parser size before: 1664 states, 85034 lines |
cfroystad
force-pushed
the
arrow_functions_v2
branch
from
c087099
to
cf1e04d
Compare
rfc
I'm not sure if the we should have different nodes for arrow_function and anonymous_arrow_function or the anonymous_arrow_function should alias to the array_function name?
What do you think?
Edit 2021.04.17:
Changed the implementation to always return the arrow_function node