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:
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'})