Repository: jlongster/prettier (js)

Issue:
Do not remove quotes around unicode keys in the flow parser
```js
({'この事はつもり素晴らしいことさ': '35jL9V'})
```

outputs

```js
({この事はつもり素晴らしいことさ: '35jL9V'})
```

which doesn't parse due to a bug in the flow parser.

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22(%7B'%E3%81%93%E3%81%AE%E4%BA%8B%E3%81%AF%E3%81%A4%E3%82%82%E3%82%8A%E7%B4%A0%E6%99%B4%E3%82%89%E3%81%97%E3%81%84%E3%81%93%E3%81%A8%E3%81%95'%3A%20'35jL9V'%7D)%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%7D%7D

Would be nice to tweak the quote removal logic in the flow parser to keep the quotes if there are non-ascii characters, otherwise it throws when parsing it again. Hopefully it'll be fixed in the future but for now, we should not create JavaScript files that don't parse.

Pull request description:
Add quotes around unicode keys in flow parser
There's a bug in the flow parser with keys using literal that contain unicode characters. Let's quote them for now.

Fixes #327

Patch:
diff --git a/src/printer.js b/src/printer.js
index 8408f434c..3291a7207 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -612,7 +612,7 @@ function genericPrintNoParens(path, options, print) {
         if (n.computed) {
           parts.push("[", path.call(print, "key"), "]");
         } else {
-          parts.push(printPropertyKey(path, print));
+          parts.push(printPropertyKey(path, options, print));
         }
         parts.push(": ", path.call(print, "value"));
       }
@@ -1042,7 +1042,18 @@ function genericPrintNoParens(path, options, print) {
     case "JSXSpreadAttribute":
       return concat([ "{...", path.call(print, "argument"), "}" ]);
     case "JSXExpressionContainer":
-      return group(
+      const shouldIndent =
+        n.expression.type !== 'ArrayExpression' &&
+        n.expression.type !== 'ObjectExpression' &&
+        n.expression.type !== 'ArrowFunctionExpression' &&
+        n.expression.type !== 'CallExpression' &&
+        n.expression.type !== 'FunctionExpression';
+
+      if (!shouldIndent) {
+        return concat(["{", path.call(print, "expression"), "}"]);
+      }
+
+      return multilineGroup(
         concat([
           "{",
           indent(
@@ -1124,7 +1135,7 @@ function genericPrintNoParens(path, options, print) {
       if (n.computed) {
         key = concat([ "[", path.call(print, "key"), "]" ]);
       } else {
-        key = printPropertyKey(path, print);
+        key = printPropertyKey(path, options, print);
         if (n.variance === "plus") {
           key = concat([ "+", key ]);
         } else if (n.variance === "minus") {
@@ -1581,12 +1592,15 @@ function printStatementSequence(path, options, print) {
   return join(hardline, printed);
 }
 
-function printPropertyKey(path, print) {
+function printPropertyKey(path, options, print) {
   var node = path.getNode().key;
   if (
     (node.type === "StringLiteral" ||
       node.type === "Literal" && typeof node.value === "string") &&
-      isIdentifierName(node.value)
+      isIdentifierName(node.value) &&
+      // There's a bug in the flow parser where it throws if there are
+      // unquoted unicode literals as keys. Let's quote them for now.
+      (!options.useFlowParser || node.value.match(/[a-zA-Z0-9$_]/))
   ) {
     // 'a' -> a
     return node.value;
@@ -1619,7 +1633,7 @@ function printMethod(path, options, print) {
     parts.push(kind, " ");
   }
 
-  var key = printPropertyKey(path, print);
+  var key = printPropertyKey(path, options, print);
 
   if (node.computed) {
     key = concat([ "[", key, "]" ]);
@@ -1769,7 +1783,7 @@ function printObjectMethod(path, options, print) {
     return printMethod(path, options, print);
   }
 
-  var key = printPropertyKey(path, print);
+  var key = printPropertyKey(path, options, print);
 
   if (objMethod.computed) {
     parts.push("[", key, "]");
@@ -2056,11 +2070,20 @@ function printJSXChildren(path, options, print) {
       const isLiteral = namedTypes.Literal.check(child);
 
       if (isLiteral && typeof child.value === "string") {
-        if (/\S/.test(child.value)) {
-          const beginBreak = child.value.match(/^\s*\n/);
-          const endBreak = child.value.match(/\n\s*$/);
-          const beginSpace = child.value.match(/^\s+/);
-          const endSpace = child.value.match(/\s+$/);
+        // There's a bug in the flow parser where it doesn't unescape the
+        // value field. To workaround this, we can use rawValue which is
+        // correctly escaped (since it parsed).
+        // We really want to use value and re-escape it ourself when possible
+        // though.
+        const value = options.useFlowParser ?
+          child.raw :
+          util.htmlEscapeInsideAngleBracket(child.value);
+
+        if (/\S/.test(value)) {
+          const beginBreak = value.match(/^\s*\n/);
+          const endBreak = value.match(/\n\s*$/);
+          const beginSpace = value.match(/^\s+/);
+          const endSpace = value.match(/\s+$/);
 
           if (beginBreak) {
             children.push(hardline);
@@ -2068,7 +2091,7 @@ function printJSXChildren(path, options, print) {
             children.push(jsxWhitespace);
           }
 
-          children.push(child.value.replace(/^\s+|\s+$/g, ""));
+          children.push(value.replace(/^\s+|\s+$/g, ""));
 
           if (endBreak) {
             children.push(hardline);
@@ -2076,10 +2099,10 @@ function printJSXChildren(path, options, print) {
             if (endSpace) children.push(jsxWhitespace);
             children.push(softline);
           }
-        } else if (/\n/.test(child.value)) {
+        } else if (/\n/.test(value)) {
           // TODO: add another hardline if >1 newline appeared. (also above)
           children.push(hardline);
-        } else if (/\s/.test(child.value)) {
+        } else if (/\s/.test(value)) {
           // whitespace-only without newlines,
           // eg; a single space separating two elements
           children.push(jsxWhitespace);
diff --git a/src/util.js b/src/util.js
index fb8a29e68..3e6a8a584 100644
--- a/src/util.js
+++ b/src/util.js
@@ -222,6 +222,17 @@ function htmlEscapeInsideDoubleQuote(str) {
 }
 util.htmlEscapeInsideDoubleQuote = htmlEscapeInsideDoubleQuote;
 
+// http://stackoverflow.com/a/7124052
+function htmlEscapeInsideAngleBracket(str) {
+  return str.replace(/</g, "&lt;").replace(/>/g, "&gt;");
+  // Intentionally disable the following since it is safe inside of a
+  // angle bracket context
+  //    .replace(/&/g, '&amp;')
+  //    .replace(/"/g, '&quot;')
+  //    .replace(/'/g, '&#39;')
+}
+util.htmlEscapeInsideAngleBracket = htmlEscapeInsideAngleBracket;
+
 var PRECEDENCE = {};
 [
   [ "||" ],


Test patch:
diff --git a/tests/jsx/__snapshots__/jsfmt.spec.js.snap b/tests/jsx/__snapshots__/jsfmt.spec.js.snap
index 0603c8a21..98fd7bb7d 100644
--- a/tests/jsx/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/jsx/__snapshots__/jsfmt.spec.js.snap
@@ -1,3 +1,128 @@
+exports[`test expression.js 1`] = `
+"<View
+  style={
+    {
+      someVeryLongStyle1: \"true\",
+      someVeryLongStyle2: \"true\",
+      someVeryLongStyle3: \"true\",
+      someVeryLongStyle4: \"true\"
+    }
+  }
+/>;
+
+<View
+  style={
+    [
+      {
+        someVeryLongStyle1: \"true\",
+        someVeryLongStyle2: \"true\",
+        someVeryLongStyle3: \"true\",
+        someVeryLongStyle4: \"true\"
+      }
+    ]
+  }
+/>;
+
+<Something>
+  {() => (
+    <SomethingElse>
+      <span />
+    </SomethingElse>
+  )}
+</Something>;
+
+<Something>
+  {items.map(item => (
+    <SomethingElse>
+      <span />
+    </SomethingElse>
+  ))}
+</Something>;
+
+<Something>
+  {function() {
+    return (
+      <SomethingElse>
+        <span />
+      </SomethingElse>
+    );
+  }}
+</Something>;
+
+<RadioListItem
+  key={option}
+  imageSource={this.props.veryBigItemImageSourceFunc &&
+    this.props.veryBigItemImageSourceFunc(option)}
+  imageSize={this.props.veryBigItemImageSize}
+  imageView={this.props.veryBigItemImageViewFunc &&
+    this.props.veryBigItemImageViewFunc(option)}
+  heading={this.props.displayTextFunc(option)}
+  value={option}
+/>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<View
+  style={{
+    someVeryLongStyle1: \"true\",
+    someVeryLongStyle2: \"true\",
+    someVeryLongStyle3: \"true\",
+    someVeryLongStyle4: \"true\"
+  }}
+/>;
+
+<View
+  style={[
+    {
+      someVeryLongStyle1: \"true\",
+      someVeryLongStyle2: \"true\",
+      someVeryLongStyle3: \"true\",
+      someVeryLongStyle4: \"true\"
+    }
+  ]}
+/>;
+
+<Something>
+  {() => (
+    <SomethingElse>
+      <span />
+    </SomethingElse>
+  )}
+</Something>;
+
+<Something>
+  {items.map(item => (
+    <SomethingElse>
+      <span />
+    </SomethingElse>
+  ))}
+</Something>;
+
+<Something>
+  {function() {
+    return (
+      <SomethingElse>
+        <span />
+      </SomethingElse>
+    );
+  }}
+</Something>;
+
+<RadioListItem
+  key={option}
+  imageSource={
+    this.props.veryBigItemImageSourceFunc &&
+      this.props.veryBigItemImageSourceFunc(option)
+  }
+  imageSize={this.props.veryBigItemImageSize}
+  imageView={
+    this.props.veryBigItemImageViewFunc &&
+      this.props.veryBigItemImageViewFunc(option)
+  }
+  heading={this.props.displayTextFunc(option)}
+  value={option}
+/>;
+"
+`;
+
 exports[`test quotes.js 1`] = `
 "<div id=\"&quot;\'<>&amp;quot;\" />;
 <div id=\'\"&#39;<>&amp;quot;\' />;
diff --git a/tests/jsx/expression.js b/tests/jsx/expression.js
new file mode 100644
index 000000000..5e984414c
--- /dev/null
+++ b/tests/jsx/expression.js
@@ -0,0 +1,60 @@
+<View
+  style={
+    {
+      someVeryLongStyle1: "true",
+      someVeryLongStyle2: "true",
+      someVeryLongStyle3: "true",
+      someVeryLongStyle4: "true"
+    }
+  }
+/>;
+
+<View
+  style={
+    [
+      {
+        someVeryLongStyle1: "true",
+        someVeryLongStyle2: "true",
+        someVeryLongStyle3: "true",
+        someVeryLongStyle4: "true"
+      }
+    ]
+  }
+/>;
+
+<Something>
+  {() => (
+    <SomethingElse>
+      <span />
+    </SomethingElse>
+  )}
+</Something>;
+
+<Something>
+  {items.map(item => (
+    <SomethingElse>
+      <span />
+    </SomethingElse>
+  ))}
+</Something>;
+
+<Something>
+  {function() {
+    return (
+      <SomethingElse>
+        <span />
+      </SomethingElse>
+    );
+  }}
+</Something>;
+
+<RadioListItem
+  key={option}
+  imageSource={this.props.veryBigItemImageSourceFunc &&
+    this.props.veryBigItemImageSourceFunc(option)}
+  imageSize={this.props.veryBigItemImageSize}
+  imageView={this.props.veryBigItemImageViewFunc &&
+    this.props.veryBigItemImageViewFunc(option)}
+  heading={this.props.displayTextFunc(option)}
+  value={option}
+/>
diff --git a/tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap b/tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap
new file mode 100644
index 000000000..0a79acf98
--- /dev/null
+++ b/tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,13 @@
+exports[`test escape.js 1`] = `
+"<div>&lt;</div>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<div>&lt;</div>;
+"
+`;
+
+exports[`test escape.js 2`] = `
+"<div>&lt;</div>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<div>&lt;</div>;
+"
+`;
diff --git a/tests/jsx_escape/escape.js b/tests/jsx_escape/escape.js
new file mode 100644
index 000000000..db12ce51b
--- /dev/null
+++ b/tests/jsx_escape/escape.js
@@ -0,0 +1,1 @@
+<div>&lt;</div>
diff --git a/tests/jsx_escape/jsfmt.spec.js b/tests/jsx_escape/jsfmt.spec.js
new file mode 100644
index 000000000..fc83bcab9
--- /dev/null
+++ b/tests/jsx_escape/jsfmt.spec.js
@@ -0,0 +1,2 @@
+run_spec(__dirname);
+run_spec(__dirname, {useFlowParser: false});
diff --git a/tests/react/__snapshots__/jsfmt.spec.js.snap b/tests/react/__snapshots__/jsfmt.spec.js.snap
index 5739f880d..94f0a1138 100644
--- a/tests/react/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/react/__snapshots__/jsfmt.spec.js.snap
@@ -200,13 +200,13 @@ var Example = React.createClass({
 var ok_void = (
   <Example
     func={() => {
-      }}
+    }}
   />
 );
 var ok_args = (
   <Example
     func={x => {
-      }}
+    }}
   />
 );
 var ok_retval = <Example func={() => 1} />;
diff --git a/tests/unicode/__snapshots__/jsfmt.spec.js.snap b/tests/unicode/__snapshots__/jsfmt.spec.js.snap
index 5e9d38546..d64e9df76 100644
--- a/tests/unicode/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/unicode/__snapshots__/jsfmt.spec.js.snap
@@ -69,3 +69,12 @@ function utf16Length(str, pos) {
 }
 "
 `;
+
+exports[`test keys.js 1`] = `
+"({\'この事はつもり素晴らしいことさ\': \'35jL9V\'})
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+({
+  \"\\u3053\\u306E\\u4E8B\\u306F\\u3064\\u3082\\u308A\\u7D20\\u6674\\u3089\\u3057\\u3044\\u3053\\u3068\\u3055\": \"35jL9V\"
+});
+"
+`;
diff --git a/tests/unicode/keys.js b/tests/unicode/keys.js
new file mode 100644
index 000000000..173e6c795
--- /dev/null
+++ b/tests/unicode/keys.js
@@ -0,0 +1,1 @@
+({'この事はつもり素晴らしいことさ': '35jL9V'})