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.

Sign up for GitHub

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

Jump to bottom

Parentheses in string literals can cause errors #270

Closed
alangpierce opened this issue Jul 1, 2016 · 2 comments
Closed

Parentheses in string literals can cause errors #270

alangpierce opened this issue Jul 1, 2016 · 2 comments
Labels
crash

Comments

@alangpierce
Copy link
Member

Here's a small test case demonstrating this:

a(')').b

http://decaffeinate.github.io/decaffeinate/repl/#?evaluate=true&code=a(')').b

It gives the error "cannot find '.' in member access".

That was the problem I specifically ran into, but an even simpler case that fails is just a(')'), which produces this incorrect intermediate JS (using some improved error logging I have set up):

an intermediate stage failed to parse: Unexpected token (1:5)
> 1 | a(')';)
    |      ^
  2 |
alangpierce added a commit to alangpierce/decaffeinate that referenced this issue Jul 1, 2016
@alangpierce
Improve error reporting when there is a parser error
bcec7f0 
This is originally based on decaffeinate#222 ,
but I ended up changing the approach pretty significantly.

Since the `PatchError` class already contains logic to pretty-print an error in
a particular place in a file, with necessary context, I decided to reuse that.
Unfortunately, it relied on having a `ParseContext` from `decaffeinate-parser`,
even though none of the code is CoffeeScript-specific. The main purpose of the
`ParseContext` over just the source code is for the `lineMap`, so for now I
refactored `PatchError` to just take the source code and create its own
`lineMap` by copying the function from `decaffeinate-parser`. But maybe a better
solution is to just break the abstraction a little and expose the
`lineColumnMapper` function from `decaffeinate-parser`? Or maybe there's some
other clever way to reuse this code that I haven't thought of. Since it's just
20 lines, it seems like it's not worth worrying about that much, I think.

This change has been really useful for me to track down the underlying cause of
errors in files I'm trying to decaffeinate, and makes it much easier to change
files to be decaffeinate-friendly. See decaffeinate#269, decaffeinate#270, and decaffeinate#271 for examples of the
output produced after this change.
@alangpierce alangpierce mentioned this issue Jul 1, 2016
Merged
@alangpierce
Copy link
Member Author

Looks like this was fixed by the change to trimNonMatchingParentheses.js in
decaffeinate/decaffeinate-parser@2684933 , so just waiting on a release of decaffeinate-parser and then decaffeinate.

@alangpierce alangpierce mentioned this issue Jul 14, 2016
Closed
@eventualbuddha
Copy link
Collaborator

Fixed in v2.17.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
crash
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@eventualbuddha @alangpierce