Repository: jlongster/prettier
Pull request #224 (2017-01-16 01:10:12)

Make sure empty for loops generate valid code
```js
for (;;);
function f() {}
```

The `;` was dropped meaning that the line right after was executed within the for loop which is not correct.

I tried to return `;` but it looks like

```js
for (;;)
  ;
```

which looks super weird so I ended up printing `{}` which looks like

```js
for (;;) {}
```

This fixes and issue found by #223

jlongster: > I tried to return ; but it looks like
> 
>```
> for (;;)
>   ;
>```

I wonder why that is emitted. As far as I know on master branch block-less if for statements should go on the same line like:

```js
for(var i=0; i<10; i++) console.log(i)
```

Since the block-less expression case goes on the same line I'd expect the semicolon to go there too. There'd probably be a space, but I think that's OK since this is such a rare construct.

jlongster: I can't remember the PR but recently there was one that changed

```
if(cond)
  val
```

to

```
if(cond) val
```

and it should apply to all if/for/while/etc constructs.
demoneaux: Yep, I opened #194 which did that. Maybe an alternative to this way would be to adjust it in the `adjustClause` method instead.
jlongster: > Yep, I opened #194 which did that. Maybe an alternative to this way would be to adjust it in the adjustClause method instead.

It looks like #194 does make the tweak in `adjustClause`?
vjeux: Nice, good catch!

I'll emit `;` which is better as it doesn't change the ast.
vjeux: Now inserting it inside of adjustClause. I had to add adjustClause to WithStatement as well.

Patch:
diff --git a/src/fast-path.js b/src/fast-path.js
index dd4b2b55e..cc195dc92 100644
--- a/src/fast-path.js
+++ b/src/fast-path.js
@@ -366,6 +366,7 @@ FPp.needsParens = function(assumeExpressionContext) {
           return n.ObjectPattern.check(node.left) && this.firstInStatement();
       }
 
+    case "FunctionExpression":
     case "ArrowFunctionExpression":
       if (parent.type === "CallExpression" && name === "callee") {
         return true;
diff --git a/src/printer.js b/src/printer.js
index 8d1392974..0bdd19e99 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -641,6 +641,18 @@ function genericPrintNoParens(path, options, print) {
       if (n.elements.length === 0) {
         parts.push("[]");
       } else {
+        // JavaScript allows you to have empty elements in an array which
+        // changes its length based on the number of commas. The algorithm
+        // is that if the last argument is null, we need to force insert
+        // a comma to ensure JavaScript recognizes it.
+        //   [,].length === 1
+        //   [1,].length === 1
+        //   [1,,].length === 2
+        //
+        // Note that util.getLast returns null if the array is empty, but
+        // we already check for an empty array just above so we are safe
+        const needsForcedTrailingComma = util.getLast(n.elements) === null;
+
         parts.push(
           multilineGroup(
             concat([
@@ -652,6 +664,7 @@ function genericPrintNoParens(path, options, print) {
                   join(concat([ ",", line ]), path.map(print, "elements"))
                 ])
               ),
+              needsForcedTrailingComma ? "," : "",
               ifBreak(options.trailingComma ? "," : ""),
               options.bracketSpacing ? line : softline,
               "]"
@@ -786,8 +799,8 @@ function genericPrintNoParens(path, options, print) {
       return concat([
         "with (",
         path.call(print, "object"),
-        ") ",
-        path.call(print, "body")
+        ")",
+        adjustClause(path.call(print, "body"), options)
       ]);
     case "IfStatement":
       const con = adjustClause(path.call(print, "consequent"), options);
@@ -2098,6 +2111,10 @@ function printJSXElement(path, options, print) {
 }
 
 function adjustClause(clause, options, forceSpace) {
+  if (clause === "") {
+    return ";";
+  }
+
   if (isCurlyBracket(clause) || forceSpace) {
     return concat([ " ", clause ]);
   }


Test patch:
diff --git a/tests/arrays/__snapshots__/jsfmt.spec.js.snap b/tests/arrays/__snapshots__/jsfmt.spec.js.snap
index 8b9f6b5ec..0dd5cc49b 100644
--- a/tests/arrays/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/arrays/__snapshots__/jsfmt.spec.js.snap
@@ -81,6 +81,19 @@ module.exports = \"arrays\";
 "
 `;
 
+exports[`test last.js 1`] = `
+"[,];
+[,,];
+[,,1,];
+[,,1,1];
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+[ , ];
+[ , , ];
+[ , , 1 ];
+[ , , 1, 1 ];
+"
+`;
+
 exports[`test numeric_elem.js 1`] = `
 "var arr = [];
 var day = new Date;
diff --git a/tests/arrays/last.js b/tests/arrays/last.js
new file mode 100644
index 000000000..12af7b6a6
--- /dev/null
+++ b/tests/arrays/last.js
@@ -0,0 +1,4 @@
+[,];
+[,,];
+[,,1,];
+[,,1,1];
diff --git a/tests/arrows/__snapshots__/jsfmt.spec.js.snap b/tests/arrows/__snapshots__/jsfmt.spec.js.snap
index bf224ddd7..3e6449a4b 100644
--- a/tests/arrows/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/arrows/__snapshots__/jsfmt.spec.js.snap
@@ -28,11 +28,17 @@ var ident = <T>(x: T): T => x;
 exports[`test arrow_function_expression.js 1`] = `
 "(a => {}).length
 typeof (() => {});
+export default (() => {})();
+(() => {})()\`\`;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 (a => {
 }).length;
 typeof (() => {
 });
+export default (() => {
+})()
+(() => {
+})()\`\`;
 "
 `;
 
diff --git a/tests/arrows/arrow_function_expression.js b/tests/arrows/arrow_function_expression.js
index 57a6e9209..f0309346f 100644
--- a/tests/arrows/arrow_function_expression.js
+++ b/tests/arrows/arrow_function_expression.js
@@ -1,2 +1,4 @@
 (a => {}).length
 typeof (() => {});
+export default (() => {})();
+(() => {})()``;
diff --git a/tests/empty_statement/__snapshots__/jsfmt.spec.js.snap b/tests/empty_statement/__snapshots__/jsfmt.spec.js.snap
new file mode 100644
index 000000000..db47c9da6
--- /dev/null
+++ b/tests/empty_statement/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,21 @@
+exports[`test body.js 1`] = `
+"with (a);
+if (1); else if (2); else;
+for (;;);
+while (1);
+for (var i in o);
+for (var i of o);
+do; while(1);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+with (a);
+if (1);
+else if (2);
+else;
+for (;;);
+while (1);
+for (var i in o);
+for (var i of o);
+do;
+while (1);
+"
+`;
diff --git a/tests/empty_statement/body.js b/tests/empty_statement/body.js
new file mode 100644
index 000000000..85c9f5b74
--- /dev/null
+++ b/tests/empty_statement/body.js
@@ -0,0 +1,7 @@
+with (a);
+if (1); else if (2); else;
+for (;;);
+while (1);
+for (var i in o);
+for (var i of o);
+do; while(1);
diff --git a/tests/empty_statement/jsfmt.spec.js b/tests/empty_statement/jsfmt.spec.js
new file mode 100644
index 000000000..989047bcc
--- /dev/null
+++ b/tests/empty_statement/jsfmt.spec.js
@@ -0,0 +1,1 @@
+run_spec(__dirname);
diff --git a/tests/function/__snapshots__/jsfmt.spec.js.snap b/tests/function/__snapshots__/jsfmt.spec.js.snap
index e33bd50d5..a776342cc 100644
--- a/tests/function/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/function/__snapshots__/jsfmt.spec.js.snap
@@ -351,6 +351,23 @@ var e = (d.bind(1): Function)();
 "
 `;
 
+exports[`test function_expression.js 1`] = `
+"(function() {}).length
+typeof (function() {});
+export default (function() {})();
+(function() {})()\`\`;
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+(function() {
+}).length;
+typeof (function() {
+});
+export default (function() {
+})()
+(function() {
+})()\`\`;
+"
+`;
+
 exports[`test rest.js 1`] = `
 "/* regression tests */
 
diff --git a/tests/function/function_expression.js b/tests/function/function_expression.js
new file mode 100644
index 000000000..786e8bdb5
--- /dev/null
+++ b/tests/function/function_expression.js
@@ -0,0 +1,4 @@
+(function() {}).length
+typeof (function() {});
+export default (function() {})();
+(function() {})()``;