tree-sitter-javascript icon indicating copy to clipboard operation
tree-sitter-javascript copied to clipboard

"Function calls" test case isn't valid JavaScript

Open marijnh opened this issue 6 years ago • 1 comments

In JavaScript, function at the start of a statement means the thing that comes after is parsed as a function statement, never an expression. So the following test case seems weird:

Function calls
============================================

x.someMethod(arg1, "arg2");
function(x, y) {

}(a, b);

---

(program
  (expression_statement (call_expression
    (member_expression (identifier) (property_identifier))
    (arguments (identifier) (string))))
  (expression_statement (call_expression
    (function
      (formal_parameters (identifier) (identifier))
      (statement_block))
    (arguments (identifier) (identifier)))))

I mean, the code doesn't parse in a real parser, so you could argue that anything that comes out of it is okay, but this is the only test that fails when I remove the (seemingly superfluous, because already part of $._expression via $._constructable_expression) $.function from the definition of call_expression, so I suspect it points at some confusion in the grammar definition.

    call_expression: $ => prec(PREC.CALL, seq(
      choice($._expression, $.super, $.function),
      choice($.arguments, $.template_string)
    )),

marijnh avatar Apr 11 '19 13:04 marijnh

Yeah, I think that test case should probably demonstrate a call to a function expression within some larger expression, like an assignment

foo = function(x, y) {
}(a, b);

or a parenthesized expression

(function() {
}(a, b))

or something.

As for whether $.function needs to be explicitly included in the call_expression rule, I don't really know. It may not be necessary anymore. There used to be some subtleties due to conflicts between function calls and new calls, but that part of the grammar was recently improved in https://github.com/tree-sitter/tree-sitter-javascript/pull/89. If npm test passes (once we've adjusted the Function calls test), we should remove it.

maxbrunsfeld avatar Apr 11 '19 17:04 maxbrunsfeld