Repository: jlongster/prettier (js)

Issue:
Missing semi-colon in strange for-loop → invalid code
Input:

```js
for (a in b) var c = {}; [];
```

Output:

```js
for (a in b)
  var c = {}
[];
```

Trying to format the output again fails with:

```
SyntaxError: Unexpected token (3:1)
  1 | for (a in b)
  2 |   var c = {}
> 3 | [];
    |  ^
  4 | 
  5 | 
```

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22for%20(a%20in%20b)%20var%20c%20%3D%20%7B%7D%3B%20%5B%5D%3B%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D

Found using the fuzzer in #371.

Pull request:
Fix missing semi-colon in for loop and var body
We avoid adding a `;` for a variable declaration in for loop but this is only meant if the var declaration is inside of the `()` part of the for loop. Not if the body part.

Fixes #385

Patch:
diff --git a/src/printer.js b/src/printer.js
index 0b4a59a25..43aa47b3c 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -414,10 +414,8 @@ function genericPrintNoParens(path, options, print) {
           "specifiers"
         );
 
-        assert.ok(standalones.length <= 1);
-
         if (standalones.length > 0) {
-          parts.push(standalones[0]);
+          parts.push(join(", ", standalones));
         }
 
         if (standalones.length > 0 && grouped.length > 0) {
@@ -748,17 +746,17 @@ function genericPrintNoParens(path, options, print) {
       ];
 
       // We generally want to terminate all variable declarations with a
-      // semicolon, except when they are children of for loops.
+      // semicolon, except when they in the () part of for loops.
       var parentNode = path.getParentNode();
 
-      if (
-        !namedTypes.ForStatement.check(parentNode) &&
-          !namedTypes.ForInStatement.check(parentNode) &&
-          !(namedTypes.ForOfStatement &&
-            namedTypes.ForOfStatement.check(parentNode)) &&
-          !(namedTypes.ForAwaitStatement &&
-            namedTypes.ForAwaitStatement.check(parentNode))
-      ) {
+      var isParentForLoop = namedTypes.ForStatement.check(parentNode) ||
+        namedTypes.ForInStatement.check(parentNode) |
+        (namedTypes.ForOfStatement &&
+          namedTypes.ForOfStatement.check(parentNode)) ||
+        (namedTypes.ForAwaitStatement &&
+          namedTypes.ForAwaitStatement.check(parentNode));
+
+      if (!(isParentForLoop && parentNode.body !== n)) {
         parts.push(";");
       }
 
@@ -977,7 +975,7 @@ function genericPrintNoParens(path, options, print) {
       if (n.test) parts.push("case ", path.call(print, "test"), ":");
       else parts.push("default:");
 
-      if (n.consequent.length > 0) {
+      if (n.consequent.find(node => node.type !== "EmptyStatement")) {
         const cons = path.call(
           function(consequentPath) {
             return printStatementSequence(consequentPath, options, print);
@@ -2330,59 +2328,68 @@ function nodeStr(node, options) {
     return node.raw;
   }
 
-  const containsSingleQuote = str.indexOf("'") !== -1;
-  const containsDoubleQuote = str.indexOf('"') !== -1;
+  const raw = node.extra ? node.extra.raw : node.raw;
+  // `rawContent` is the string exactly like it appeared in the input source
+  // code, with its enclosing quote.
+  const rawContent = raw.slice(1, -1);
 
-  let shouldUseSingleQuote = options.singleQuote;
-  if (options.singleQuote && containsSingleQuote && !containsDoubleQuote) {
-    shouldUseSingleQuote = false;
-  }
-  if (!options.singleQuote && !containsSingleQuote && containsDoubleQuote) {
-    shouldUseSingleQuote = true;
-  }
+  const double = { quote: '"', regex: /"/g };
+  const single = { quote: "'", regex: /'/g };
 
-  const raw = node.extra ? node.extra.raw : node.raw;
-  const isSingleQuote = raw.charAt(0) === "'";
+  const preferred = options.singleQuote ? single : double;
+  const alternate = preferred === single ? double : single;
+
+  let shouldUseAlternateQuote = false;
 
-  if (isSingleQuote && shouldUseSingleQuote ||
-      !isSingleQuote && !shouldUseSingleQuote) {
-    return raw;
+  // If `rawContent` contains at least one of the quote preferred for enclosing
+  // the string, we might want to enclose with the alternate quote instead, to
+  // minimize the number of escaped quotes.
+  if (rawContent.includes(preferred.quote)) {
+    const numPreferredQuotes = (rawContent.match(preferred.regex) || []).length;
+    const numAlternateQuotes = (rawContent.match(alternate.regex) || []).length;
+
+    shouldUseAlternateQuote = numPreferredQuotes > numAlternateQuotes;
   }
 
-  const nextQuote = shouldUseSingleQuote ? "'" : '"';
-
-  // When we change the outer quotes, we need to change the number of
-  // backslash before. In order to know how many backslash to put,
-  // I manually did the first few examples and extracted a general rule
-  // out of it.
-  //
-  // shouldSwitch === true
-  //
-  //   3 -> 3 "\\\'" -> '\\\''
-  //   2 -> 3 "\\'"  -> '\\\''
-  //   1 -> 1 "\'"   -> '\''
-  //   0 -> 1 "\'"   -> '\''
-  //
-  //   Rule: n -> Math.floor(n / 2) + 1
-  //
-  // shouldSwitch === false
-  //
-  //   3 -> 2 '\\\'' -> "\\'"
-  //   1 -> 0 '\''   -> "'"
-  //
-  //   Rule: n -> n - 1
-  return nextQuote + raw.slice(1, -1)
-    .replace(/\\*['"]/g, (match) => {
-      const quote = match[match.length - 1];
-      const shouldSwitch = quote === "'" && shouldUseSingleQuote ||
-       quote === '"' && !shouldUseSingleQuote;
-      const numBackslash = match.length - 1;
-      const nextNumBackslash = shouldSwitch ?
-        Math.floor(numBackslash / 2) + 1 :
-        numBackslash - 1;
-
-      return '\\'.repeat(nextNumBackslash) + quote;
-    }) + nextQuote;
+  const enclosingQuote = shouldUseAlternateQuote
+    ? alternate.quote
+    : preferred.quote;
+
+  // It might sound unnecessary to use `makeString` even if `node.raw` already
+  // is enclosed with `enclosingQuote`, but it isn't. `node.raw` could contain
+  // unnecessary escapes (such as in `"\'"`). Always using `makeString` makes
+  // sure that we consistently output the minimum amount of escaped quotes.
+  return makeString(rawContent, enclosingQuote);
+}
+
+function makeString(rawContent, enclosingQuote) {
+  const otherQuote = enclosingQuote === '"' ? "'" : '"';
+
+  // Matches _any_ escape and unescaped quotes (both single and double).
+  const regex = /\\([\s\S])|(['"])/g;
+
+  // Escape and unescape single and double quotes as needed to be able to
+  // enclose `rawContent` with `enclosingQuote`.
+  const newContent = rawContent.replace(regex, (match, escaped, quote) => {
+    // If we matched an escape, and the escaped character is a quote of the
+    // other type than we intend to enclose the string with, there's no need for
+    // it to be escaped, so return it _without_ the backslash.
+    if (escaped === otherQuote) {
+      return escaped;
+    }
+
+    // If we matched an unescaped quote and it is of the _same_ type as we
+    // intend to enclose the string with, it must be escaped, so return it with
+    // a backslash.
+    if (quote === enclosingQuote) {
+      return "\\" + quote;
+    }
+
+    // Otherwise return the escape or unescaped quote as-is.
+    return match;
+  });
+
+  return enclosingQuote + newContent + enclosingQuote;
 }
 
 function isFirstStatement(path) {


Test patch:
diff --git a/tests/for/__snapshots__/jsfmt.spec.js.snap b/tests/for/__snapshots__/jsfmt.spec.js.snap
index e9bcb4c28..274a6614a 100644
--- a/tests/for/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/for/__snapshots__/jsfmt.spec.js.snap
@@ -291,3 +291,12 @@ for (var i = 0; i < 10; ++i) {
 }
 "
 `;
+
+exports[`test var.js 1`] = `
+"for (a in b) var c = {}; [];
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+for (a in b)
+  var c = {};
+[];
+"
+`;
diff --git a/tests/for/var.js b/tests/for/var.js
new file mode 100644
index 000000000..09f1808b9
--- /dev/null
+++ b/tests/for/var.js
@@ -0,0 +1,1 @@
+for (a in b) var c = {}; [];
diff --git a/tests/import/__snapshots__/jsfmt.spec.js.snap b/tests/import/__snapshots__/jsfmt.spec.js.snap
index 25cd1a55a..6ccad1ddc 100644
--- a/tests/import/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/import/__snapshots__/jsfmt.spec.js.snap
@@ -51,3 +51,17 @@ import {
 import {fitsIn, oneLine} from \".\";
 "
 `;
+
+exports[`test multiple_standalones.js 1`] = `
+"import a, * as b from \'a\';
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+import a, * as b from \"a\";
+"
+`;
+
+exports[`test multiple_standalones.js 2`] = `
+"import a, * as b from \'a\';
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+import a, * as b from \"a\";
+"
+`;
diff --git a/tests/import/multiple_standalones.js b/tests/import/multiple_standalones.js
new file mode 100644
index 000000000..91311bec5
--- /dev/null
+++ b/tests/import/multiple_standalones.js
@@ -0,0 +1,1 @@
+import a, * as b from 'a';
diff --git a/tests/quotes/__snapshots__/jsfmt.spec.js.snap b/tests/quotes/__snapshots__/jsfmt.spec.js.snap
index 0fd9a45e6..fee003ce7 100644
--- a/tests/quotes/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/quotes/__snapshots__/jsfmt.spec.js.snap
@@ -29,95 +29,259 @@ function b(object, key) {
 `;
 
 exports[`test strings.js 1`] = `
-"\"abc\"
+"// Every string will be changed to double quotes, unless we end up with fewer
+// escaped quotes by using single quotes. (Vice versa if the \"singleQuote\"
+// option is true).
+//
+// Note that even if a string already has the correct enclosing quotes, it is
+// still processed in order to remove unnecessarily escaped quotes within it,
+// for consistency.
+
+// Simple strings.
+\"abc\"
 \'abc\'
 
-\'\\\'\'
+// Escape.
+\'\\0\'
+
+// Emoji.
+\'🐶\'
+
+// Empty string.
+\"\"
+\'\'
 
+// Single double quote.
+\"\\\"\"
 \'\"\'
-\'\\\"\'
-\'\\\\\"\'
 
+// Single single quote.
 \"\'\"
+\'\\\'\'
+
+// Unnecessary escapes.
 \"\\\'\"
-\"\\\\\'\"
+\'\\\"\'
+
+// One of each.
+\"\\\"\'\"
+\'\"\\\'\'
+
+// One of each with unnecessary escapes.
+\"\\\"\\\'\"
+\'\\\"\\\'\'
 
-\"\'\\\"\"
-\'\\\'\"\'
+// More double quotes than single quotes.
+\"\\\"\'\\\"\"
+\'\"\\\'\"\'
 
+// More single quotes than double quotes.
+\"\\\"\'\'\"
+\'\"\\\'\\\'\'
+
+// Two of each.
+\"\\\"\\\"\'\'\"
+\'\"\"\\\'\\\'\'
+
+// Single backslash.
 \'\\\\\'
 \"\\\\\"
 
-\'\\0\'
-\'🐶\'
+// Backslases.
+\"\\\"\\\\\\\"\\\\\\\\\\\" \'\\\'\\\\\'\\\\\\\'\\\\\\\\\'\"
+\'\\\'\\\\\\\'\\\\\\\\\\\' \"\\\"\\\\\"\\\\\\\"\\\\\\\\\"\'
+
+// Somewhat more real-word example.
+\"He\'s sayin\': \\\"How\'s it goin\'?\\\" Don\'t ask me why.\"
+\'He\\\'s sayin\\\': \"How\\\'s it goin\\\'?\" Don\\\'t ask me why.\'
+
+// Somewhat more real-word example 2.
+\"var backslash = \\\"\\\\\\\", doubleQuote = \'\\\"\';\"
+\'var backslash = \"\\\\\", doubleQuote = \\\'\"\\\';\'
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// Every string will be changed to double quotes, unless we end up with fewer
+// escaped quotes by using single quotes. (Vice versa if the \"singleQuote\"
+// option is true).
+//
+// Note that even if a string already has the correct enclosing quotes, it is
+// still processed in order to remove unnecessarily escaped quotes within it,
+// for consistency.
+
+// Simple strings.
 \"abc\";
 \"abc\";
 
-\"\'\";
+// Escape.
+\"\\0\";
+
+// Emoji.
+\"🐶\"; // Empty string.
+\"\";
 
+\"\"; // Single double quote.
 \'\"\';
-\'\\\"\';
-\'\\\\\"\';
 
+\'\"\'; // Single single quote.
 \"\'\";
-\"\\\'\";
-\"\\\\\'\";
 
-\"\'\\\"\";
-\"\'\\\"\";
+\"\'\"; // Unnecessary escapes.
+\"\'\";
 
-\"\\\\\";
+\'\"\'; // One of each.
+\"\\\"\'\";
+
+\"\\\"\'\"; // One of each with unnecessary escapes.
+\"\\\"\'\";
+
+\"\\\"\'\"; // More double quotes than single quotes.
+\'\"\\\'\"\';
+
+\'\"\\\'\"\'; // More single quotes than double quotes.
+\"\\\"\'\'\";
+
+\"\\\"\'\'\"; // Two of each.
+\"\\\"\\\"\'\'\";
+
+\"\\\"\\\"\'\'\"; // Single backslash.
 \"\\\\\";
 
-\"\\0\";
-\"🐶\";
+\"\\\\\"; // Backslases.
+\"\\\"\\\\\\\"\\\\\\\\\\\" \'\'\\\\\'\\\\\'\\\\\\\\\'\";
+
+\'\\\'\\\\\\\'\\\\\\\\\\\' \"\"\\\\\"\\\\\"\\\\\\\\\"\'; // Somewhat more real-word example.
+\"He\'s sayin\': \\\"How\'s it goin\'?\\\" Don\'t ask me why.\";
+
+\"He\'s sayin\': \\\"How\'s it goin\'?\\\" Don\'t ask me why.\"; // Somewhat more real-word example 2.
+\'var backslash = \"\\\\\", doubleQuote = \\\'\"\\\';\';
+\'var backslash = \"\\\\\", doubleQuote = \\\'\"\\\';\';
 "
 `;
 
 exports[`test strings.js 2`] = `
-"\"abc\"
+"// Every string will be changed to double quotes, unless we end up with fewer
+// escaped quotes by using single quotes. (Vice versa if the \"singleQuote\"
+// option is true).
+//
+// Note that even if a string already has the correct enclosing quotes, it is
+// still processed in order to remove unnecessarily escaped quotes within it,
+// for consistency.
+
+// Simple strings.
+\"abc\"
 \'abc\'
 
-\'\\\'\'
+// Escape.
+\'\\0\'
+
+// Emoji.
+\'🐶\'
+
+// Empty string.
+\"\"
+\'\'
 
+// Single double quote.
+\"\\\"\"
 \'\"\'
-\'\\\"\'
-\'\\\\\"\'
 
+// Single single quote.
 \"\'\"
+\'\\\'\'
+
+// Unnecessary escapes.
 \"\\\'\"
-\"\\\\\'\"
+\'\\\"\'
+
+// One of each.
+\"\\\"\'\"
+\'\"\\\'\'
+
+// One of each with unnecessary escapes.
+\"\\\"\\\'\"
+\'\\\"\\\'\'
 
-\"\'\\\"\"
-\'\\\'\"\'
+// More double quotes than single quotes.
+\"\\\"\'\\\"\"
+\'\"\\\'\"\'
 
+// More single quotes than double quotes.
+\"\\\"\'\'\"
+\'\"\\\'\\\'\'
+
+// Two of each.
+\"\\\"\\\"\'\'\"
+\'\"\"\\\'\\\'\'
+
+// Single backslash.
 \'\\\\\'
 \"\\\\\"
 
-\'\\0\'
-\'🐶\'
+// Backslases.
+\"\\\"\\\\\\\"\\\\\\\\\\\" \'\\\'\\\\\'\\\\\\\'\\\\\\\\\'\"
+\'\\\'\\\\\\\'\\\\\\\\\\\' \"\\\"\\\\\"\\\\\\\"\\\\\\\\\"\'
+
+// Somewhat more real-word example.
+\"He\'s sayin\': \\\"How\'s it goin\'?\\\" Don\'t ask me why.\"
+\'He\\\'s sayin\\\': \"How\\\'s it goin\\\'?\" Don\\\'t ask me why.\'
+
+// Somewhat more real-word example 2.
+\"var backslash = \\\"\\\\\\\", doubleQuote = \'\\\"\';\"
+\'var backslash = \"\\\\\", doubleQuote = \\\'\"\\\';\'
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// Every string will be changed to double quotes, unless we end up with fewer
+// escaped quotes by using single quotes. (Vice versa if the \"singleQuote\"
+// option is true).
+//
+// Note that even if a string already has the correct enclosing quotes, it is
+// still processed in order to remove unnecessarily escaped quotes within it,
+// for consistency.
+
+// Simple strings.
 \'abc\';
 \'abc\';
 
-\"\'\";
+// Escape.
+\'\\0\';
+
+// Emoji.
+\'🐶\'; // Empty string.
+\'\';
 
+\'\'; // Single double quote.
 \'\"\';
-\'\\\"\';
-\'\\\\\"\';
 
+\'\"\'; // Single single quote.
 \"\'\";
-\"\\\'\";
-\"\\\\\'\";
 
-\'\\\'\"\';
-\'\\\'\"\';
+\"\'\"; // Unnecessary escapes.
+\"\'\";
 
-\'\\\\\';
+\'\"\'; // One of each.
+\'\"\\\'\';
+
+\'\"\\\'\'; // One of each with unnecessary escapes.
+\'\"\\\'\';
+
+\'\"\\\'\'; // More double quotes than single quotes.
+\'\"\\\'\"\';
+
+\'\"\\\'\"\'; // More single quotes than double quotes.
+\"\\\"\'\'\";
+
+\"\\\"\'\'\"; // Two of each.
+\'\"\"\\\'\\\'\';
+
+\'\"\"\\\'\\\'\'; // Single backslash.
 \'\\\\\';
 
-\'\\0\';
-\'🐶\';
+\'\\\\\'; // Backslases.
+\"\\\"\\\\\\\"\\\\\\\\\\\" \'\'\\\\\'\\\\\'\\\\\\\\\'\";
+
+\'\\\'\\\\\\\'\\\\\\\\\\\' \"\"\\\\\"\\\\\"\\\\\\\\\"\'; // Somewhat more real-word example.
+\"He\'s sayin\': \\\"How\'s it goin\'?\\\" Don\'t ask me why.\";
+
+\"He\'s sayin\': \\\"How\'s it goin\'?\\\" Don\'t ask me why.\"; // Somewhat more real-word example 2.
+\'var backslash = \"\\\\\", doubleQuote = \\\'\"\\\';\';
+\'var backslash = \"\\\\\", doubleQuote = \\\'\"\\\';\';
 "
 `;
diff --git a/tests/quotes/strings.js b/tests/quotes/strings.js
index b296b144d..11ff656f1 100644
--- a/tests/quotes/strings.js
+++ b/tests/quotes/strings.js
@@ -1,21 +1,69 @@
+// Every string will be changed to double quotes, unless we end up with fewer
+// escaped quotes by using single quotes. (Vice versa if the "singleQuote"
+// option is true).
+//
+// Note that even if a string already has the correct enclosing quotes, it is
+// still processed in order to remove unnecessarily escaped quotes within it,
+// for consistency.
+
+// Simple strings.
 "abc"
 'abc'
 
-'\''
+// Escape.
+'\0'
+
+// Emoji.
+'🐶'
 
+// Empty string.
+""
+''
+
+// Single double quote.
+"\""
 '"'
-'\"'
-'\\"'
 
+// Single single quote.
 "'"
+'\''
+
+// Unnecessary escapes.
 "\'"
-"\\'"
+'\"'
+
+// One of each.
+"\"'"
+'"\''
+
+// One of each with unnecessary escapes.
+"\"\'"
+'\"\''
+
+// More double quotes than single quotes.
+"\"'\""
+'"\'"'
+
+// More single quotes than double quotes.
+"\"''"
+'"\'\''
 
-"'\""
-'\'"'
+// Two of each.
+"\"\"''"
+'""\'\''
 
+// Single backslash.
 '\\'
 "\\"
 
-'\0'
-'🐶'
+// Backslases.
+"\"\\\"\\\\\" '\'\\'\\\'\\\\'"
+'\'\\\'\\\\\' "\"\\"\\\"\\\\"'
+
+// Somewhat more real-word example.
+"He's sayin': \"How's it goin'?\" Don't ask me why."
+'He\'s sayin\': "How\'s it goin\'?" Don\'t ask me why.'
+
+// Somewhat more real-word example 2.
+"var backslash = \"\\\", doubleQuote = '\"';"
+'var backslash = "\\", doubleQuote = \'"\';'
diff --git a/tests/strings/__snapshots__/jsfmt.spec.js.snap b/tests/strings/__snapshots__/jsfmt.spec.js.snap
index a648fe85a..b75363d4e 100644
--- a/tests/strings/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/strings/__snapshots__/jsfmt.spec.js.snap
@@ -30,10 +30,10 @@ exports[`test strings.js 1`] = `
   \"abc\",
   \"\'\",
   \'\"\',
-  \'\\\"\',
+  \'\"\',
   \'\\\\\"\',
   \"\'\",
-  \"\\\'\",
+  \"\'\",
   \"\\\\\'\",
   \"\'\\\"\",
   \"\'\\\"\",
diff --git a/tests/switch/__snapshots__/jsfmt.spec.js.snap b/tests/switch/__snapshots__/jsfmt.spec.js.snap
index 60b9dc0f3..0005e88a0 100644
--- a/tests/switch/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/switch/__snapshots__/jsfmt.spec.js.snap
@@ -1,3 +1,12 @@
+exports[`test empty_switch.js 1`] = `
+"switch (1) { default:; }
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+switch (1) {
+  default:
+}
+"
+`;
+
 exports[`test more_switch.js 1`] = `
 "/* @flow */
 
diff --git a/tests/switch/empty_switch.js b/tests/switch/empty_switch.js
new file mode 100644
index 000000000..b91bfe9d4
--- /dev/null
+++ b/tests/switch/empty_switch.js
@@ -0,0 +1,1 @@
+switch (1) { default:; }