Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Operator precedence rules for tagged template functions #336

Merged
merged 4 commits into from
Sep 2, 2024

Conversation

jackschu
Copy link
Contributor

@jackschu jackschu commented Aug 24, 2024

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

@jackschu jackschu force-pushed the fix-new-tagged-template branch from f8fd3a2 to 4308c48 Compare August 24, 2024 23:45
@amaanq amaanq force-pushed the fix-new-tagged-template branch from 025a590 to fa346e2 Compare September 2, 2024 03:28
@amaanq amaanq force-pushed the fix-new-tagged-template branch from fa346e2 to 7c5c639 Compare September 2, 2024 03:29
@amaanq
Copy link
Member

amaanq commented Sep 2, 2024

awesome, thank you!

@amaanq amaanq merged commit db26836 into tree-sitter:master Sep 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

apparent incorrect parse of new expression with a tagged template
2 participants