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

fix: add implicit function call parens in the normalize step #302

Conversation

alangpierce
Copy link
Member

The code in #301 is actually parsed incorrectly by the CoffeeScript parser (even
before decaffeinate-parser does anything interesting). It says that the function
body ends at the end of the file, so decaffeinate was inserting a
close-curly-brace at the end of the file, which was incorrect. However, the
function application was correctly parsed, and explicitly putting parens for the
function application causes the CoffeeScript parser to work again, so we can
work around this issue by inserting parens for all implicit functions before the
MainStage.

I think this will also fix a bunch of other problems caused by implicit function
calls. For example, it also happens to fix #269. In the codebase that I'm trying
to decaffeinate, it fixes 49 out of the 104 files with decaffeinate failures
that I hadn't categorized.

I added the heuristic that we put the close-paren on the next line if the last
arg is a multi-line expression. This fixed the formatting in almost every test,
except the change introduced two test issues:

Closes #301.
Closes #269.

The code in decaffeinate#301 is actually parsed incorrectly by the CoffeeScript parser (even
before decaffeinate-parser does anything interesting). It says that the function
body ends at the end of the file, so decaffeinate was inserting a
close-curly-brace at the end of the file, which was incorrect. However, the
function application was correctly parsed, and explicitly putting parens for the
function application causes the CoffeeScript parser to work again, so we can
work around this issue by inserting parens for all implicit functions before the
MainStage.

I think this will also fix a bunch of other problems caused by implicit function
calls. For example, it also happens to fix decaffeinate#269. In the codebase that I'm trying
to decaffeinate, it fixes 49 out of the 104 files with decaffeinate failures
that I hadn't categorized.

I added the heuristic that we put the close-paren on the next line if the last
arg is a multi-line expression. This fixed the formatting in almost every test,
except the change introduced two test issues:
* Object literal formatting sometimes had an excessive newline in non-implicit
  function calls
  ( http://decaffeinate-project.org/repl/#?evaluate=true&code=a(b%2C%0A%20%20c%3A%20d%0A%20%20e%3A%20f%0A%29 )
  so adding parens exposed the issue. It's pretty minor, though. I think.
* In another test, the added parens interfered with some other operations in the
  normalize step in a way that will be fixed by Rich-Harris/magic-string#89

Closes decaffeinate#301.
Closes decaffeinate#269.
b() {
return c(d,
e,
f);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I think this paren would be on the next line. Pretty minor though, can be done later.

@eventualbuddha eventualbuddha merged commit 0457cea into decaffeinate:master Jul 11, 2016
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

Successfully merging this pull request may close these issues.

Method closing curly brace sometimes placed too late for loops over implicit function calls crash
2 participants