fix: sortKeys auto-fix works with property variance (#280)

* fix: sortKeys auto-fix works with property variance * fix: sortKeys auto-fix works with nested object property types * docs: update the sort-keys readme source with the fixable line
gajus · Oct 5, 2017 · 9c69e90 · 9c69e90
1 parent 47829fa
commit 9c69e90
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 23 deletions.
diff --git a/.README/rules/sort-keys.md b/.README/rules/sort-keys.md
@@ -1,5 +1,7 @@
 ### `sort-keys`
 
+_The `--fix` option on the command line automatically fixes problems reported by this rule._
+
 Enforces sorting of Object annotations.
 
 This rule mirrors ESlint's [sort-keys](http://eslint.org/docs/rules/sort-keys) rule.

diff --git a/src/rules/sortKeys.js b/src/rules/sortKeys.js
@@ -65,17 +65,54 @@ const isValidOrders = {
   }
 };
 
+const variances = {
+  minus: '-',
+  plus: '+'
+};
+
 const generateOrderedList = (context, sort, properties) => {
   return properties.map((property) => {
     const name = getParameterName(property, context);
-    const value = context.getSourceCode().getText(property.value);
+    let value;
 
-    return [name + (property.optional ? '?' : ''), value];
+    if (property.value.type === 'ObjectTypeAnnotation') {
+      value = generateFix(property.value, context, sort); // eslint-disable-line no-use-before-define
+    } else {
+      value = context.getSourceCode().getText(property.value);
+    }
+
+    return [(variances[property.variance] || '') + name + (property.optional ? '?' : ''), value];
   })
     .sort((first, second) => { return sort(first[0], second[0]) ? -1 : 1; })
     .map((item) => { return item[0] + ': ' + item[1]; });
 };
 
+const generateFix = (node, context, sort) => {
+  // this could be done much more cleanly in ESLint >=4
+  // as we can apply multiple fixes. That also means we can
+  // maintain code style in a much nicer way
+  let nodeText;
+  const newTypes = generateOrderedList(context, sort, node.properties);
+  const source = context.getSourceCode(node);
+
+  const originalSubstring = source.getText(node);
+
+  nodeText = originalSubstring;
+
+  node.properties.forEach((property, index) => {
+    const subString = source.getText(property);
+    const addComma = subString[subString.length - 1] === ',';
+
+    nodeText = nodeText.replace(subString, '$' + index + (addComma ? ',' : ''));
+  });
+
+  newTypes.forEach((item, index) => {
+    nodeText = nodeText.replace('$' + index, item);
+  });
+
+  return nodeText;
+};
+
 const create = (context) => {
   const order = _.get(context, ['options', 0], 'asc');
   const {natural, caseSensitive} = _.get(context, ['options', 1], defaults);
@@ -108,27 +145,7 @@ const create = (context) => {
             order
           },
           fix (fixer) {
-            // this could be done much more cleanly in ESLint >=4
-            // as we can apply multiple fixes. That also means we can
-            // maintain code style in a much nicer way
-            let nodeText;
-            const newTypes = generateOrderedList(context, isValidOrder, node.properties);
-            const source = context.getSourceCode(node);
-
-            const originalSubstring = source.getText(node);
-
-            nodeText = originalSubstring;
-
-            node.properties.forEach((property, index) => {
-              const subString = source.getText(property);
-              const addComma = subString[subString.length - 1] === ',';
-
-              nodeText = nodeText.replace(subString, '$' + index + (addComma ? ',' : ''));
-            });
-
-            newTypes.forEach((item, index) => {
-              nodeText = nodeText.replace('$' + index, item);
-            });
+            const nodeText = generateFix(node, context, isValidOrder);
 
             return fixer.replaceText(node, nodeText);
           },

diff --git a/tests/rules/assertions/sortKeys.js b/tests/rules/assertions/sortKeys.js
@@ -166,6 +166,71 @@ export default {
           c: number,
         }
       `
+    },
+    {
+      code: `
+        type FooType = {
+          c: {
+            z: number,
+            x: string,
+            y: boolean,
+          },
+          a: number | string | boolean,
+          b: (param: string) => number,
+        }
+      `,
+      errors: [
+        {message: 'Expected type annotations to be in ascending order. "x" should be before "z".'},
+        {message: 'Expected type annotations to be in ascending order. "a" should be before "c".'}
+      ],
+      output: `
+        type FooType = {
+          a: number | string | boolean,
+          b: (param: string) => number,
+          c: {
+            x: string,
+            y: boolean,
+            z: number,
+          },
+        }
+      `
+    },
+    {
+      code: `
+        type FooType = {
+          c: {
+            z: {
+              j: string,
+              l: number,
+              k: boolean,
+            },
+            x: string,
+            y: boolean,
+          },
+          a: number | string | boolean,
+          b: (param: string) => number,
+        }
+      `,
+      errors: [
+        {message: 'Expected type annotations to be in ascending order. "k" should be before "l".'},
+        {message: 'Expected type annotations to be in ascending order. "x" should be before "z".'},
+        {message: 'Expected type annotations to be in ascending order. "a" should be before "c".'}
+      ],
+      output: `
+        type FooType = {
+          a: number | string | boolean,
+          b: (param: string) => number,
+          c: {
+            x: string,
+            y: boolean,
+            z: {
+              j: string,
+              k: boolean,
+              l: number,
+            },
+          },
+        }
+      `
     }
     /* eslint-enable no-restricted-syntax */
   ],