fix: sort-keys spreads, spreads with generics, comment handling (#439)

See added tests.
gajus · Jan 1, 2020 · dccaa76 · dccaa76
1 parent 70b4fee
commit dccaa76
Show file tree

Hide file tree

Showing 2 changed files with 273 additions and 35 deletions.
diff --git a/src/rules/sortKeys.js b/src/rules/sortKeys.js
@@ -65,47 +65,98 @@ const isValidOrders = {
   },
 };
 
-const variances = {
-  minus: '-',
-  plus: '+',
-};
-
-const getVariance = (node) => {
-  if (_.isString(node.variance)) {
-    return variances[node.variance] || '';
-  } else if (_.get(node, 'variance.type') === 'Variance') {
-    return variances[node.variance.kind] || '';
-  } else {
-    return '';
-  }
-};
-
 const generateOrderedList = (context, sort, properties) => {
-  return properties.map((property) => {
+  const source = context.getSourceCode();
+
+  const items = properties.map((property) => {
     const name = getParameterName(property, context);
+
+    const commentsBefore = source.getCommentsBefore(property);
+    const startIndex = commentsBefore.length > 0 ?
+      commentsBefore[0].start :
+      property.start;
+
+    if (property.type === 'ObjectTypeSpreadProperty' || !property.value) {
+      // NOTE: It could but currently does not fix recursive generic type arguments in GenericTypeAnnotation within ObjectTypeSpreadProperty.
+
+      // Maintain everything between the start of property including leading comments and the nextPunctuator `,` or `}`:
+      const nextPunctuator = source.getTokenAfter(property, {
+        filter: (token) => {
+          return token.type === 'Punctuator';
+        },
+      });
+      const beforePunctuator = source.getTokenBefore(nextPunctuator, {
+        includeComments: true,
+      });
+      const text = source.getText().substring(startIndex, beforePunctuator.end);
+
+      return [property, text];
+    }
+
+    const colonToken = source.getTokenBefore(property.value, {
+      filter: (token) => {
+        return token.value === ':';
+      },
+    });
+
+    // Preserve all code until the colon verbatim:
+    const key = source.getText().substring(startIndex, colonToken.start);
     let value;
 
-    if (property.type === 'ObjectTypeSpreadProperty') {
-      return ['...' + property.argument.id.name];
-    } else if (property.value.type === 'ObjectTypeAnnotation') {
+    if (property.value.type === 'ObjectTypeAnnotation') {
       // eslint-disable-next-line no-use-before-define
-      value = generateFix(property.value, context, sort);
+      value = ' ' + generateFix(property.value, context, sort);
+    } else {
+      // NOTE: It could but currently does not fix recursive generic type arguments in GenericTypeAnnotation.
+
+      // Maintain everything between the `:` and the next Punctuator `,` or `}`:
+      const nextPunctuator = source.getTokenAfter(property, {
+        filter: (token) => {
+          return token.type === 'Punctuator';
+        },
+      });
+      const beforePunctuator = source.getTokenBefore(nextPunctuator, {
+        includeComments: true,
+      });
+      const text = source.getText().substring(colonToken.end, beforePunctuator.end);
+
+      value = text;
+    }
+
+    return [property, name, key, value];
+  });
+
+  const itemGroups = [[]];
+  let itemGroupIndex = 0;
+  items.forEach((item) => {
+    if (item[0].type === 'ObjectTypeSpreadProperty') {
+      ++itemGroupIndex;
+      itemGroups[itemGroupIndex] = [item];
+      ++itemGroupIndex;
+      itemGroups[itemGroupIndex] = [];
     } else {
-      value = context.getSourceCode().getText(property.value);
+      itemGroups[itemGroupIndex].push(item);
     }
+  });
 
-    return [name, getVariance(property) + name + (property.optional ? '?' : ''), value];
-  })
-    .sort((first, second) => {
-      return sort(first[0], second[0]) ? -1 : 1;
-    })
-    .map((item) => {
-      if (item.length === 1) {
-        return item[0];
+  const orderedList = [];
+  itemGroups.forEach((itemGroup) => {
+    if (itemGroup[0] && itemGroup[0].type !== 'ObjectTypeSpreadProperty') {
+      itemGroup
+        .sort((first, second) => {
+          return sort(first[1], second[1]) ? -1 : 1;
+        });
+    }
+    orderedList.push(...itemGroup.map((item) => {
+      if (item.length === 2) {
+        return item[1];
       }
 
-      return item[1] + ': ' + item[2];
-    });
+      return item[2] + ':' + item[3];
+    }));
+  });
+
+  return orderedList;
 };
 
 const generateFix = (node, context, sort) => {
@@ -121,10 +172,24 @@ const generateFix = (node, context, sort) => {
   nodeText = originalSubstring;
 
   node.properties.forEach((property, index) => {
-    const subString = source.getText(property);
-    const addComma = subString[subString.length - 1] === ',';
-
-    nodeText = nodeText.replace(subString, '$' + index + (addComma ? ',' : ''));
+    const nextPunctuator = source.getTokenAfter(property, {
+      filter: (token) => {
+        return token.type === 'Punctuator';
+      },
+    });
+    const beforePunctuator = source.getTokenBefore(nextPunctuator, {
+      includeComments: true,
+    });
+    const commentsBefore = source.getCommentsBefore(property);
+    const startIndex = commentsBefore.length > 0 ?
+      commentsBefore[0].start :
+      property.start;
+    const subString = source.getText().substring(
+      startIndex,
+      beforePunctuator.end
+    );
+
+    nodeText = nodeText.replace(subString, '$' + index);
   });
 
   newTypes.forEach((item, index) => {

diff --git a/tests/rules/assertions/sortKeys.js b/tests/rules/assertions/sortKeys.js
@@ -65,6 +65,179 @@ export default {
         }
       `,
     },
+    {
+      code: `
+        type FooType = {
+          a: $ReadOnlyArray<number>,
+          c: $ReadOnlyMap<string, number>,
+          b: Map<string, Array<Map<string, number>>>,
+        }
+      `,
+      errors: [{message: 'Expected type annotations to be in ascending order. "b" should be before "c".'}],
+      output: `
+        type FooType = {
+          a: $ReadOnlyArray<number>,
+          b: Map<string, Array<Map<string, number>>>,
+          c: $ReadOnlyMap<string, number>,
+        }
+      `,
+    },
+    {
+      code: `
+        type FooType = {
+          ...ErrorsInRecursiveGenericTypeArgsButDoesNotFix<{
+            y: boolean,
+            x: string,
+            z: {
+              j: string,
+              l: number,
+              k: boolean,
+            },
+          }>,
+          a: number,
+          c: string,
+          b: Map<string, Array<ErrorsInRecursiveGenericTypeArgsButDoesNotFix<{
+            y: boolean,
+            x: string,
+            z: {
+              j: string,
+              l: number,
+              k: boolean,
+            },
+          }>>>,
+        }
+      `,
+      errors: [
+        {message: 'Expected type annotations to be in ascending order. "x" should be before "y".'},
+        {message: 'Expected type annotations to be in ascending order. "k" should be before "l".'},
+        {message: 'Expected type annotations to be in ascending order. "b" should be before "c".'},
+        {message: 'Expected type annotations to be in ascending order. "x" should be before "y".'},
+        {message: 'Expected type annotations to be in ascending order. "k" should be before "l".'},
+      ],
+      output: `
+        type FooType = {
+          ...ErrorsInRecursiveGenericTypeArgsButDoesNotFix<{
+            y: boolean,
+            x: string,
+            z: {
+              j: string,
+              l: number,
+              k: boolean,
+            },
+          }>,
+          a: number,
+          b: Map<string, Array<ErrorsInRecursiveGenericTypeArgsButDoesNotFix<{
+            y: boolean,
+            x: string,
+            z: {
+              j: string,
+              l: number,
+              k: boolean,
+            },
+          }>>>,
+          c: string,
+        }
+      `,
+    },
+    {
+      code: `
+        type FooType = {
+          ...BPreservesSpreadOrder,
+          ...APreservesSpreadOrder,
+          c: string,
+          b: number,
+        }
+      `,
+      errors: [{message: 'Expected type annotations to be in ascending order. "b" should be before "c".'}],
+      output: `
+        type FooType = {
+          ...BPreservesSpreadOrder,
+          ...APreservesSpreadOrder,
+          b: number,
+          c: string,
+        }
+      `,
+    },
+    {
+      code: `
+        type FooType = {
+          ...BPreservesSpreadSpans,
+          ...APreservesSpreadSpans,
+          c: string,
+          b: number,
+          ...CPreservesSpreadSpans,
+          e: string,
+          d: number,
+        }
+      `,
+      errors: [
+        {message: 'Expected type annotations to be in ascending order. "b" should be before "c".'},
+        {message: 'Expected type annotations to be in ascending order. "d" should be before "e".'},
+      ],
+      output: `
+        type FooType = {
+          ...BPreservesSpreadSpans,
+          ...APreservesSpreadSpans,
+          b: number,
+          c: string,
+          ...CPreservesSpreadSpans,
+          d: number,
+          e: string,
+        }
+      `,
+    },
+    {
+      code: `
+        type FooType = {
+          ...BPreservesSpreadOrderAndTypeArgs<string, number>,
+          ...APreservesSpreadOrderAndTypeArgs<number>,
+          c: string,
+          b: number,
+        }
+      `,
+      errors: [{message: 'Expected type annotations to be in ascending order. "b" should be before "c".'}],
+      output: `
+        type FooType = {
+          ...BPreservesSpreadOrderAndTypeArgs<string, number>,
+          ...APreservesSpreadOrderAndTypeArgs<number>,
+          b: number,
+          c: string,
+        }
+      `,
+    },
+    {
+      code: `
+        type FooType = {
+          /* preserves block comment before spread BType */
+          // preserves line comment before spread BType
+          ... /* preserves comment in spread BType */ BType<Generic> /* preserves trailing comment in spread AType */,
+          /* preserves block comment before spread AType */
+          // preserves line comment before spread AType
+          ... /* preserves comment in spread AType */ AType /* preserves trailing comment in spread AType */,
+          /* preserves block comment before reordered key "c" */
+          // preserves line comment before reordered key "c"
+          c:/* preserves comment and white space or lack of it */string/* preserves trailing comment for key "c" */,
+          b: number,
+          dWithoutComma: boolean
+        }
+      `,
+      errors: [{message: 'Expected type annotations to be in ascending order. "b" should be before "c".'}],
+      output: `
+        type FooType = {
+          /* preserves block comment before spread BType */
+          // preserves line comment before spread BType
+          ... /* preserves comment in spread BType */ BType<Generic> /* preserves trailing comment in spread AType */,
+          /* preserves block comment before spread AType */
+          // preserves line comment before spread AType
+          ... /* preserves comment in spread AType */ AType /* preserves trailing comment in spread AType */,
+          b: number,
+          /* preserves block comment before reordered key "c" */
+          // preserves line comment before reordered key "c"
+          c:/* preserves comment and white space or lack of it */string/* preserves trailing comment for key "c" */,
+          dWithoutComma: boolean
+        }
+      `,
+    },
     {
       code: `
         type FooType = {