Quote and unquote number keys (#8508)

* Quote and unquote number keys

* Add changelog entry

* Add tests for escapes

* Fix idempotency

* Only run flow-repo/ tests with flow

271 jsfmt.spec.js in flow-repo use only the flow parser.
18 use flow and babel.
Now, all of them use only flow.

* Don’t unquote negative numbers

* Don’t quote/unquote numbers in TypeScript

* Quote BigInt

* Fix AST check

* Fix condition

* Fix BigInt to string

* Fix ESLint not recognizing BigInt

* Only quote/unquote “simple” numbers

* No need to clean BigIntLiteral anymore

* Add tests for 1. and .1

* Fix 1. and .1

* Remove unnecessary babel-ts from tests

* Only unquote numbers for babel

* Update changelog

.eslintrc.yml

@@ -1,12 +1,10 @@
 root: true
 env:
-  es6: true
+  es2020: true
   node: true
 extends:
   - eslint:recommended
   - plugin:prettier/recommended
-parserOptions:
-  ecmaVersion: 2018
 plugins:
   - import
   - unicorn

changelog_unreleased/javascript/pr-8508.md

@@ -0,0 +1,44 @@
+#### Quote and unquote number keys ([#8508](https://github.com/prettier/prettier/pull/8508) by [@lydell](https://github.com/lydell))
+
+Prettier removes quotes from object keys if they are identifiers. Now, Prettier also removes quotes from object keys that are numbers.
+
+If you use `quoteProps: "consistent"`, Prettier can also _add_ quotes to number keys so that _all_ properties end up with quotes.
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+x = {
+  "a": null,
+  "1": null,
+};
+
+// Prettier stable
+x = {
+  a: null,
+  "1": null,
+};
+
+// Prettier master
+x = {
+  a: null,
+  1: null,
+};
+```
+
+Prettier only touches “simple” numbers such as `1` and `123.5`. It _won’t_ make the following transformations, as they feel unexpected:
+
+```
+1e2 -> "100"
+0b10 -> "10"
+1_000 -> "1000"
+1.0 -> "1"
+0.99999999999999999 -> "1"
+999999999999999999999 -> "1e+21"
+2n -> "2"
+
+"1e+100" -> 1e100
+```
+
+(Please don’t use confusing numbers as object keys!)
+
+Note that Prettier only unquotes numbers using the `"babel"` parser. It’s not completely safe to do so in TypeScript.

src/language-js/clean.js

@@ -78,7 +78,9 @@ function clean(ast, newObj, parent) {
     delete newObj.closingElement;
   }
 
-  // We change {'key': value} into {key: value}
+  // We change {'key': value} into {key: value}.
+  // And {key: value} into {'key': value}.
+  // Also for (some) number keys.
   if (
     (ast.type === "Property" ||
       ast.type === "ObjectProperty" ||
@@ -89,6 +91,7 @@ function clean(ast, newObj, parent) {
     typeof ast.key === "object" &&
     ast.key &&
     (ast.key.type === "Literal" ||
+      ast.key.type === "NumericLiteral" ||
       ast.key.type === "StringLiteral" ||
       ast.key.type === "Identifier")
   ) {

src/language-js/printer-estree.js

@@ -68,12 +68,14 @@ const {
   isMemberExpressionChain,
   isMemberish,
   isNgForOf,
+  isNumericLiteral,
   isObjectType,
   isObjectTypePropertyAFunction,
   isSimpleFlowType,
+  isSimpleNumber,
   isSimpleTemplateLiteral,
   isStringLiteral,
-  isStringPropSafeToCoerceToIdentifier,
+  isStringPropSafeToUnquote,
   isTemplateOnItsOwnLine,
   isTestCall,
   isTheOnlyJSXElementInMarkdown,
@@ -3747,32 +3749,53 @@ function printPropertyKey(path, options, print) {
         !prop.computed &&
         prop.key &&
         isStringLiteral(prop.key) &&
-        !isStringPropSafeToCoerceToIdentifier(prop, options)
+        !isStringPropSafeToUnquote(prop, options)
     );
     needsQuoteProps.set(parent, objectHasStringProp);
   }
 
   if (
-    key.type === "Identifier" &&
+    (key.type === "Identifier" ||
+      (isNumericLiteral(key) &&
+        isSimpleNumber(printNumber(rawText(key))) &&
+        // Avoid converting 999999999999999999999 to 1e+21, 0.99999999999999999 to 1 and 1.0 to 1.
+        String(key.value) === printNumber(rawText(key)) &&
+        // Quoting number keys is safe in JS and Flow, but not in TypeScript (as
+        // mentioned in `isStringPropSafeToUnquote`).
+        !(options.parser === "typescript" || options.parser === "babel-ts"))) &&
     (options.parser === "json" ||
       (options.quoteProps === "consistent" && needsQuoteProps.get(parent)))
   ) {
     // a -> "a"
-    const prop = printString(JSON.stringify(key.name), options);
+    // 1 -> "1"
+    // 1.5 -> "1.5"
+    const prop = printString(
+      JSON.stringify(
+        key.type === "Identifier" ? key.name : key.value.toString()
+      ),
+      options
+    );
     return path.call(
       (keyPath) => comments.printComments(keyPath, () => prop, options),
       "key"
     );
   }
 
   if (
-    isStringPropSafeToCoerceToIdentifier(node, options) &&
+    isStringPropSafeToUnquote(node, options) &&
     (options.quoteProps === "as-needed" ||
       (options.quoteProps === "consistent" && !needsQuoteProps.get(parent)))
   ) {
     // 'a' -> a
+    // '1' -> 1
+    // '1.5' -> 1.5
     return path.call(
-      (keyPath) => comments.printComments(keyPath, () => key.value, options),
+      (keyPath) =>
+        comments.printComments(
+          keyPath,
+          () => (/^\d/.test(key.value) ? printNumber(key.value) : key.value),
+          options
+        ),
       "key"
     );
   }

src/language-js/utils.js

@@ -734,21 +734,55 @@ function returnArgumentHasLeadingComment(options, argument) {
   return false;
 }
 
-function isStringPropSafeToCoerceToIdentifier(node, options) {
+// Note: Quoting/unquoting numbers in TypeScript is not safe.
+//
+// let a = { 1: 1, 2: 2 }
+// let b = { '1': 1, '2': 2 }
+//
+// declare let aa: keyof typeof a;
+// declare let bb: keyof typeof b;
+//
+// aa = bb;
+// ^^
+// Type '"1" | "2"' is not assignable to type '1 | 2'.
+//   Type '"1"' is not assignable to type '1 | 2'.(2322)
+//
+// And in Flow, you get:
+//
+// const x = {
+//   0: 1
+//   ^ Non-string literal property keys not supported. [unsupported-syntax]
+// }
+//
+// Angular does not support unquoted numbers in expressions.
+//
+// So we play it safe and only unquote numbers for the "babel" parser.
+// (Vue supports unquoted numbers in expressions, but let’s keep it simple.)
+//
+// Identifiers can be unquoted in more circumstances, though.
+function isStringPropSafeToUnquote(node, options) {
   return (
+    options.parser !== "json" &&
     isStringLiteral(node.key) &&
-    isIdentifierName(node.key.value) &&
     rawText(node.key).slice(1, -1) === node.key.value &&
-    options.parser !== "json" &&
-    // With `--strictPropertyInitialization`, TS treats properties with quoted names differently than unquoted ones.
-    // See https://github.com/microsoft/TypeScript/pull/20075
-    !(
-      (options.parser === "typescript" || options.parser === "babel-ts") &&
-      node.type === "ClassProperty"
-    )
+    ((isIdentifierName(node.key.value) &&
+      // With `--strictPropertyInitialization`, TS treats properties with quoted names differently than unquoted ones.
+      // See https://github.com/microsoft/TypeScript/pull/20075
+      !(
+        (options.parser === "typescript" || options.parser === "babel-ts") &&
+        node.type === "ClassProperty"
+      )) ||
+      (isSimpleNumber(node.key.value) &&
+        String(Number(node.key.value)) === node.key.value &&
+        options.parser === "babel"))
   );
 }
 
+// Matches “simple” numbers like `123` and `2.5` but not `1_000`, `1e+100` or `0b10`.
+function isSimpleNumber(numberString) {
+  return /^(\d+|\d+\.\d+)$/.test(numberString);
+}
+
 function isJestEachTemplateLiteral(node, parentNode) {
   /**
    * describe.each`table`(name, fn)
@@ -1105,9 +1139,10 @@ module.exports = {
   isObjectType,
   isObjectTypePropertyAFunction,
   isSimpleFlowType,
+  isSimpleNumber,
   isSimpleTemplateLiteral,
   isStringLiteral,
-  isStringPropSafeToCoerceToIdentifier,
+  isStringPropSafeToUnquote,
   isTemplateOnItsOwnLine,
   isTestCall,
   isTheOnlyJSXElementInMarkdown,

tests/flow-repo/async_iteration/__snapshots__/jsfmt.spec.js.snap

@@ -2,7 +2,7 @@
 
 exports[`delegate_yield.js format 1`] = `
 ====================================options=====================================
-parsers: ["flow", "babel"]
+parsers: ["flow"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
@@ -66,7 +66,7 @@ async function* delegate_return() {
 
 exports[`generator.js format 1`] = `
 ====================================options=====================================
-parsers: ["flow", "babel"]
+parsers: ["flow"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
@@ -128,7 +128,7 @@ async function f() {
 
 exports[`return.js format 1`] = `
 ====================================options=====================================
-parsers: ["flow", "babel"]
+parsers: ["flow"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
@@ -186,7 +186,7 @@ refuse_return()
 
 exports[`throw.js format 1`] = `
 ====================================options=====================================
-parsers: ["flow", "babel"]
+parsers: ["flow"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================

tests/flow-repo/async_iteration/jsfmt.spec.js

@@ -1 +1 @@
-run_spec(__dirname, ["flow", "babel"]);
+run_spec(__dirname, ["flow"]);