search

tree-sitter / tree-sitter-javascript

Javascript grammar for tree-sitter
MIT License
363 stars 114 forks source link

Fix: Operator precedence rules for tagged template functions #336

Closed jackschu closed 2 months ago

jackschu commented 2 months ago

Closes #334

Background understanding

(FYI i'm using ' as a backtick in in-line code blocks for convenience, whenever i use single-quote, i mean backtick)

Tagged templates have operator precedence ~16 according to MDN (src)

The tag does not have to be a plain identifier. You can use any expression with precedence greater than 16, which includes property access, function call, new expression, or even another tagged template literal.

new without its (..) args is level 16

so new foo'hi' should result in foo'hi' being evaluated first then new is applied as if the result of foo'hi' is a classname

So the correct parse tree should be something like this

(new_expression [1, 0] - [1, 9]
      constructor: (call_expression [1, 4] - [1, 9]
        function: (identifier [1, 4] - [1, 5])
        arguments: (template_string [1, 5] - [1, 9]
          (string_fragment [1, 6] - [1, 8]))))

but new() with its args list is level 17 so new foo()'hi' should result in new foo() being evaluated first then 'hi' being applied as it 'calls' the resulting class instance

So the correct parse tree should be something like this

expression_statement [0, 0] - [0, 12]
    (call_expression [0, 0] - [0, 11]
      function: (new_expression [0, 0] - [0, 7]
        constructor: (identifier [0, 4] - [0, 5])
        arguments: (arguments [0, 5] - [0, 7]))
      arguments: (template_string [0, 7] - [0, 11]
        (string_fragment [0, 8] - [0, 10])))

Current Problem and fix

new foo()'hi' is currently correct but new foo'hi' results in

(expression_statement [0, 0] - [0, 11]
    (call_expression [0, 0] - [0, 11]
      function: (new_expression [0, 0] - [0, 7]
        constructor: (identifier [0, 4] - [0, 7])) // this is wrong!
      arguments: (template_string [0, 7] - [0, 11]
        (string_fragment [0, 8] - [0, 10])))

This occurs for two reasons, call_expressions require an expression, but new_expressions only allow primary_expressions so when the identifier foo is processed into a primary_expression it gets consumed by new_expression without conflict.

This diff changes call_expressions to be constructed with choice(primary_expression, new_expression) in the case where a tagged-template is used. This is sane because of the precedence rules discussed earlier, ie primary_expression allows for call_expression member_expression (level 17), parenthesized_expression (level 18), and direct identifiers.

The only other allowed expression is new_expression (level 17) so I added it to the choice block

With this change, we still don't get the desired result because 'new' primary_expression gets resolved as new_expression before we can make a call expression. So I add a new template_call precedence for this case.

Note that regular call precedences should still come after 'new' so that new foo() continues to parse correctly

cc @amaanq

amaanq commented 2 months ago

awesome, thank you!