Sort with defaults

Fixes #18 by allowing sort when properties have a default, as long as none of the identifiers in the default match one of the properties being destructured.

lib/rules/sort-destructure-keys.js

@@ -23,32 +23,6 @@ function getNodeName(node) {
   }
 }
 
-/**
- * Determines if the node is a RestElement, accounting for different
- * parsers.
- */
-function isRestProperty({ type }) {
-  return (
-    type === "RestElement" ||
-    type === "RestProperty" ||
-    type === "ExperimentalRestProperty"
-  );
-}
-
-/**
- * Returns whether or not a node is safe to be sorted.
- */
-function shouldCheck(node) {
-  const { value } = node;
-
-  return (
-    isRestProperty(node) ||
-    value.type === "Identifier" ||
-    (value.type === "ObjectPattern" && value.properties.every(shouldCheck)) ||
-    (value.type === "AssignmentPattern" && value.right.type === "Literal")
-  );
-}
-
 /**
  * Sort priority for node types.
  */
@@ -170,19 +144,76 @@ module.exports = {
     const sorter = createSorter(caseSensitive);
 
     return {
-      ObjectPattern(node) {
+      ObjectPattern(objectPatternNode) {
+        const scope = context.getScope();
+
+        function isReferencedByOtherProperties(id) {
+          for (const variable of scope.variables) {
+            if (variable.name !== id.name) {
+              continue;
+            }
+
+            for (const reference of variable.references) {
+              if (reference.identifier === id) {
+                continue;
+              }
+
+              let current = reference.identifier;
+              while (current) {
+                if (current === objectPatternNode) {
+                  return true;
+                }
+                current = current.parent;
+              }
+            }
+            return false;
+          }
+        }
+
+        /**
+         * Returns whether or not a node is safe to be sorted.
+         * @param {import('estree').ObjectPattern['properties'][number]} node
+         */
+        function shouldCheck(node) {
+          if (node.type !== "Property") {
+            return true;
+          }
+
+          switch (node.value.type) {
+            case "ObjectPattern":
+              return node.value.properties.every(shouldCheck);
+            case "ArrayPattern":
+              // Fake the element as a property for simplicity
+              return node.value.elements.every((e) =>
+                shouldCheck({ type: "Property", value: e })
+              );
+            case "AssignmentPattern":
+              if (node.value.left.type === "Identifier") {
+                return !isReferencedByOtherProperties(node.value.left);
+              }
+              break;
+            case "Identifier": {
+              return !isReferencedByOtherProperties(node.value);
+            }
+            default:
+              return true;
+          }
+
+          return true;
+        }
+
         /*
          * If the node is more complex than just basic destructuring
          * with literal defaults, we just skip it. If some values use
          * previous values as defaults, then we cannot simply sort them.
          */
-        if (!node.properties.every(shouldCheck)) {
+        if (!objectPatternNode.properties.every(shouldCheck)) {
           return;
         }
 
         let prevNode = null;
 
-        for (const nextNode of node.properties) {
+        for (const nextNode of objectPatternNode.properties) {
           if (prevNode && sorter(prevNode, nextNode) > 0) {
             context.report({
               node: nextNode,
@@ -196,7 +227,7 @@ module.exports = {
                   context,
                   caseSensitive,
                   fixer,
-                  node,
+                  node: objectPatternNode,
                   sorter,
                 }),
             });

tests/lib/rules/sort-destructure-keys.js

@@ -142,20 +142,30 @@ function testsWithParser(parser, parserOptions = {}) {
     testsFor("default identifiers", {
       valid: [
         "const {b, a = b} = someObj;",
+        "const {b, a = foo(b)} = someObj;",
         "const {b, ['a']: {x = b}} = someObj;",
         "const {a, c: {e, d = e}, b} = someObj;",
+        `const {c, b, d = 'foo', a = d} = someObj;`,
+        `const {c, b, d: { e }, a = e} = someObj;`,
+        `const {c, b, d: [e], a = e} = someObj;`,
       ],
       invalid: [
         {
-          code: "const {a, c: {e, d}, b = c} = someObj;",
+          code: "const {a, c: {e, d}, b = a} = someObj;",
           errors: just("e", "d"),
-          output: "const {a, c: {d, e}, b = c} = someObj;",
+          output: "const {a, c: {d, e}, b = a} = someObj;",
         },
         {
           code: "const {a, c, b = 3} = someObj;",
           errors: just("c", "b"),
           output: "const {a, b = 3, c} = someObj;",
         },
+        // Allow identifiers or expressions, as long as they aren't the other properties
+        {
+          code: "const {a, c, b = OutOfScope} = someObj;",
+          errors: just("c", "b"),
+          output: "const {a, b = OutOfScope, c} = someObj;",
+        },
       ],
     });