tree-sitter / tree-sitter-javascript

Javascript grammar for tree-sitter
MIT License
318 stars 107 forks source link

Unable to identify IIFE. #132

Closed vikrant-sahu closed 3 years ago

vikrant-sahu commented 3 years ago

I have the following snippet which the tree-sitter is unable to recognise:

(function () {
var userName = "Steve";

function display(name)
{
alert("MyScript2.js: " + name);
}

display(userName);
})();

It just skips this part of the code.

issue-label-bot[bot] commented 3 years ago

Issue-Label Bot is automatically applying the label bug to this issue, with a confidence of 0.71. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

patrickt commented 3 years ago

Can you be more specific about what you’ve tried, and in what circumstances your code is skipped? I pasted the above code into the tree-sitter playground and it parsed correctly.

vikrant-sahu commented 3 years ago

Okay. Sorry I have to put the code above it to reproduce it.

function add(x,y){
    return x+y
}

var subtract = function(x,y){
    return x-y
}

(function () {
var userName = "Steve";

function display(name)
{
alert("MyScript2.js: " + name);
}

display(userName);
})();

The above code should has 3 blocks i.e a function declaration, a variable declaration(actually a function) and a IIFE. It's not considering the IIFE as separate block and embedding it inside the previous block i.e variable declaration.

patrickt commented 3 years ago

Aha, yeah, that looks problematic—without a semicolon there, it tries to parse that (function ()) as arguments to a hypothetical function call that would be returned from the above function() call. Inserting a semicolon after the subtract function body appears to avoid this, but (assuming that the above passes correctly in a JS engine) we should fix this. Unfortunately, I know very little about the JS/TS grammar, so I’ll defer to @maxbrunsfeld.

maxbrunsfeld commented 3 years ago

I think that’s the correct parse, and you’d need to put a semicolon at the end of your var declaration, but I might be wrong.

I’m on mobile. Could someone double check the behavior of that code using node or a browser?

patrickt commented 3 years ago

Looks like node accepts it without the semicolon. After a function alert(str) { console.log(str) } in the node console, it prints MyScript2.js: Steve.

vikrant-sahu commented 3 years ago

yeah, that's common in JS. The thing is JS is very dynamic.

Also when a function or a class starts with export or const it calls it export_statement and lexical_declaration irrespective of if it's function or a class.

maxbrunsfeld commented 3 years ago

Tree-sitter-javascript's parse tree is correct.

@vikrant-sahu JavaScript's automatic semicolon insertion rules can be a bit tricky. The behavior can be especially confusing when you begin a line with ( or [, because if you don't use semicolons, that [ or ( will almost always be treated as a continuation of the previous statement.

The most common practice is to either:

  1. use semicolons at the end of statements, or
  2. when beginning a line with ( or [, add a semicolon at the beginning of that line.

In the future though, before opening issues like this, first double check that your understanding of the code is correct by running it. In this case, notice how the behavior of the code changes if you add a semicolon at the end of the var statement.