Skip to content

Commit

Permalink
fix: properly set location data in OUTDENT nodes (jashkenas#2)
Browse files Browse the repository at this point in the history
Backport of jashkenas#4291 .

In f609036, a line was changed from
`if length > 0 then (length - 1) else 0` to `Math.max 0, length - 1`. However,
in some cases, the `length` variable can be `undefined`. The previous code would
correctly compute `lastCharacter` as 0, but the new code would compute it as
`NaN`. This would cause trouble later on: the end location would just be the end
of the current chunk, which would be incorrect.

Here's a specific case where the parser was behaving incorrectly:
```
a {
  b: ->
    return c d,
      if e
        f
}
g
```

The OUTDENT tokens after the `f` had an undefined length, so the `NaN` made it
so the end location was at the end of the file. That meant that various nodes in
the AST, like the `return` node, would incorrectly have an end location at the
end of the file.

To fix, I just reverted the change to that particular line.

Also add a test that tokens have locations that are in order
  • Loading branch information
alangpierce committed Aug 6, 2016
1 parent c558757 commit 55ee19b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/coffee-script/lexer.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/lexer.coffee
Expand Up @@ -649,7 +649,7 @@ exports.Lexer = class Lexer

# Use length - 1 for the final offset - we're supplying the last_line and the last_column,
# so if last_column == first_column, then we're looking at a character of length 1.
lastCharacter = Math.max 0, length - 1
lastCharacter = if length > 0 then (length - 1) else 0
[locationData.last_line, locationData.last_column] =
@getLineAndColumnFromChunk offsetInChunk + lastCharacter

Expand Down
19 changes: 19 additions & 0 deletions test/location.coffee
Expand Up @@ -450,6 +450,25 @@ test "#3621: Multiline regex and manual `Regex` call with interpolation should
eq tokenA.origin?[1], tokenB.origin?[1]
eq tokenA.stringEnd, tokenB.stringEnd

test "Verify tokens have locations that are in order", ->
source = '''
a {
b: ->
return c d,
if e
f
}
g
'''
tokens = CoffeeScript.tokens source
lastToken = null
for token in tokens
if lastToken
ok token[2].first_line >= lastToken[2].last_line
if token[2].first_line == lastToken[2].last_line
ok token[2].first_column >= lastToken[2].last_column
lastToken = token

test "Verify all tokens get a location", ->
doesNotThrow ->
tokens = CoffeeScript.tokens testScript
Expand Down

0 comments on commit 55ee19b

Please sign in to comment.