Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Semicolons after variable declarations #67

Closed
apatil opened this Issue · 6 comments

4 participants

Anand Patil Kyle Simpson Michael Ficarra Marijn Haverbeke
Anand Patil

Hi Marijn,

I noticed that for var x = 0;, the variable declaration's end location does not include the semicolon:

> acorn.parse("var x = 0;")
{ type: 'Program',
  start: 0,
  end: 10,
  body: 
   [ { type: 'VariableDeclaration',
       start: 0,
       end: 9,
       declarations: [Object],
       kind: 'var' } ] }

That has been an issue for me when using acorn to break JavaScript code up into top-level statements, because the statements in the output list lack semicolons after variable declarations even when they were present in the original code.

I started a pull request, but noticed that other tests in the suite treat dropping the semicolon as correct. For example, the test for var x /* comment */; wants the variable declaration to end at column 5.

Should I change the other tests to expect the semicolon?

Thanks.

Kyle Simpson

FYI: esprima does include the ; in its range based counting:

http://esprima.org/demo/parse.html?code=var%20x%20%3D%200%3B

(make sure to turn on the "Index-based range" option)

Esprima reports 0-10 for the range (the ranges are apparently "start position to end position, but not including end position"), instead of 0-9.

Michael Ficarra

IMO, the semicolon and any whitespace/comments leading up to it should be included in the range.

Marijn Haverbeke marijnh closed this issue from a commit
Marijn Haverbeke Fix wrong end position for VariableDeclaration nodes
It should include the semicolon.

Closes #67
22c3161
Marijn Haverbeke
Owner

I agree this is a bug. Fixed in attached commit.

@michaelficarra No, whitespace around a node definitely will not be included in the node's span.

Michael Ficarra

@marijnh: ... it's internal whitespace. To be clear, I meant this:

var x = 0    ;
         ^^^^
Marijn Haverbeke
Owner

@michaelficarra Ah, I see. In that case, I am in full agreement. I assumed it in "leading up to it" was the var statement.

Anand Patil

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.