Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi line string fails with function call #457

Closed
vinnymac opened this issue Oct 18, 2016 · 3 comments
Closed

Multi line string fails with function call #457

vinnymac opened this issue Oct 18, 2016 · 3 comments
Assignees

Comments

@vinnymac
Copy link

decaffeinate is crashing on my CoffeeScript input:

String """
  Test Test Test
"""

repl

I get this error:

AddVariableDeclarationsStage failed to parse: Unexpected token, expected , (1:9)
> 1 | String(""" Test Test Test """
    |          ^
  2 | );

Note, changing to the below syntax appears to work just fine:

"""
  Test Test Test
"""

repl

@alangpierce alangpierce self-assigned this Oct 18, 2016
alangpierce added a commit to alangpierce/coffeescript that referenced this issue Oct 18, 2016
Fixes decaffeinate/decaffeinate#457

The existing logic for computing the end location of a string was to take the
end of the string contents, then add the delimiter length to last_column. For
example, `"""abc"""` would have an end position three characters after the `c`.
However, if a string ended in a newline, then the end location for the string
contents would be one line above the end location for the string, so the proper
fix is to move the end location to the next line, not just to shift it to the
right.

This avoids a bug where the location data would sometimes reference a
non-existent location (one past the end of its line), which would confuse
decaffeinate-parser.
alangpierce added a commit to alangpierce/coffeescript that referenced this issue Oct 18, 2016
Fixes decaffeinate/decaffeinate#457

The existing logic for computing the end location of a string was to take the
end of the string contents, then add the delimiter length to last_column. For
example, `"""abc"""` would have an end position three characters after the `c`.
However, if a string ended in a newline, then the end location for the string
contents would be one line above the end location for the string, so the proper
fix is to move the end location to the next line, not just to shift it to the
right.

This avoids a bug where the location data would sometimes reference a
non-existent location (one past the end of its line), which would confuse
decaffeinate-parser.
@alangpierce
Copy link
Member

Thanks for reporting. I tracked this down and it turns out it's a bug in the CoffeeScript lexer/parser. The CoffeeScript lexer was giving the wrong location for the end of the string, so decaffeinate-parser thought the string included the implicitly-added close-paren,, so the string wasn't properly recognized as a triple-quoted string, so decaffeinate never ran the code to change triple-quoted strings to regular strings.

I submitted this fix, which should fix the issue: decaffeinate/coffeescript#9

@vinnymac
Copy link
Author

vinnymac commented Oct 22, 2016

@alangpierce I find that very odd, since if I put the same problematic code into the repl from coffeescript.org it appears to transpile just fine to ES5. Is it the forked version only that contains the bug?

@alangpierce
Copy link
Member

@vinnymac The bug only causes the intermediate syntax tree representation for CoffeeScript to be incorrect, not the compiled JS. It's supposed to provide the start and end position of every node, but it was giving wrong information in this case. The normal CoffeeScript compiler doesn't rely on that information (or at least, not as much), so it never caused any problems in the CoffeeScript output. But decaffeinate does an "in-place" transformation so that formatting is preserved as much as possible, so it relies heavily on the CoffeeScript parser providing correct location data.

alangpierce added a commit to decaffeinate/coffeescript that referenced this issue Oct 31, 2016
Fixes decaffeinate/decaffeinate#457

The existing logic for computing the end location of a string was to take the
end of the string contents, then add the delimiter length to last_column. For
example, `"""abc"""` would have an end position three characters after the `c`.
However, if a string ended in a newline, then the end location for the string
contents would be one line above the end location for the string, so the proper
fix is to move the end location to the next line, not just to shift it to the
right.

This avoids a bug where the location data would sometimes reference a
non-existent location (one past the end of its line), which would confuse
decaffeinate-parser.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants