Method closing curly brace sometimes placed too late #301

alangpierce · 2016-07-11T00:38:23Z

Here's an example that's as reduced as I can get it:

A = {
  b: ->
    return c d,
      e,
        f
}
G

It gives this error:

AddVariableDeclarationsStage failed to parse: Unexpected token (7:0)
  5 |         f)
  6 | }
> 7 | G;
    | ^
  8 |   };;

The text was updated successfully, but these errors were encountered:

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.

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: * 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 #301. Closes #269.

alangpierce mentioned this issue Jul 11, 2016

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

Merged

eventualbuddha closed this as completed in #302 Jul 11, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Method closing curly brace sometimes placed too late #301

Method closing curly brace sometimes placed too late #301

alangpierce commented Jul 11, 2016

Method closing curly brace sometimes placed too late #301

Method closing curly brace sometimes placed too late #301

Comments

alangpierce commented Jul 11, 2016