Fix incorrect location data in OUTDENT nodes

[alangpierce]

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. This confuses tools that rely on
the CoffeeScript parser and need the location info to be correct.

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.

[lydell]

Is it possible to write a test for this?

[vendethiel]

Why do they have a undefined length? Shouldn't it be at least 0?

[alangpierce]

@vendethiel To be honest I don't know this code that well, but my impression is that length is optional for OUTDENT tokens, since outdents are sometimes more "logical" tokens that don't correspond go a direct range of source code. If you look at the outdentToken function in lexer.coffee, it takes outdentLength as the third parameter, and two of the three usages don't pass in any length. The function also has the code outdentLength and @chunk[outdentLength] in INDENTABLE_CLOSERS, suggesting that outdentLength is intentionally optional. I could potentially try to change it to always be specified, but I don't know what other consequences that might have. The simple revert certainly seems safest.

@lydell When I initially looked I thought there weren't any tests directly for the lexer, but now I see that test/location.coffee is what I want. I just added a test there.

[lydell]

LGTM