Fix: prefer-const when using destructuring assign (fixes #8308) (#10924)

* Fix: prefer-const when using destructuring assign (fixes #8308)

* Update prefer-const docs

* Fix documentation linting errors

* Implement recursive checking for MemberExpressions

* Fix test typos

* Fix lint issue

* Include test for object rest element

* Use null check for spread elements

docs/rules/prefer-const.md

@@ -79,6 +79,15 @@ for (let i = 0, end = 10; i < end; ++i) {
     console.log(a);
 }
 
+// `predicate` is only assigned once but cannot be separately declared as `const`
+let predicate;
+[object.type, predicate] = foo();
+
+// `a` is only assigned once but cannot be separately declared as `const`
+let a;
+const b = {};
+({ a, c: b.c } = func());
+
 // suggest to use `no-var` rule.
 var b = 3;
 console.log(b);

lib/rules/prefer-const.js

@@ -57,6 +57,7 @@ function canBecomeVariableDeclaration(identifier) {
  * @returns {boolean} Indicates if the variable is from outer scope or function parameters.
  */
 function isOuterVariableInDestructing(name, initScope) {
+
     if (initScope.through.find(ref => ref.resolved && ref.resolved.name === name)) {
         return true;
     }
@@ -96,6 +97,54 @@ function getDestructuringHost(reference) {
     return node;
 }
 
+/**
+ * Determines if a destructuring assignment node contains
+ * any MemberExpression nodes. This is used to determine if a
+ * variable that is only written once using destructuring can be
+ * safely converted into a const declaration.
+ * @param {ASTNode} node The ObjectPattern or ArrayPattern node to check.
+ * @returns {boolean} True if the destructuring pattern contains
+ *      a MemberExpression, false if not.
+ */
+function hasMemberExpressionAssignment(node) {
+    switch (node.type) {
+        case "ObjectPattern":
+            return node.properties.some(prop => {
+                if (prop) {
+
+                    /*
+                     * Spread elements have an argument property while
+                     * others have a value property. Because different
+                     * parsers use different node types for spread elements,
+                     * we just check if there is an argument property.
+                     */
+                    return hasMemberExpressionAssignment(prop.argument || prop.value);
+                }
+
+                return false;
+            });
+
+        case "ArrayPattern":
+            return node.elements.some(element => {
+                if (element) {
+                    return hasMemberExpressionAssignment(element);
+                }
+
+                return false;
+            });
+
+        case "AssignmentPattern":
+            return hasMemberExpressionAssignment(node.left);
+
+        case "MemberExpression":
+            return true;
+
+        // no default
+    }
+
+    return false;
+}
+
 /**
  * Gets an identifier node of a given variable.
  *
@@ -148,7 +197,8 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
 
             if (destructuringHost !== null && destructuringHost.left !== void 0) {
                 const leftNode = destructuringHost.left;
-                let hasOuterVariables = false;
+                let hasOuterVariables = false,
+                    hasNonIdentifiers = false;
 
                 if (leftNode.type === "ObjectPattern") {
                     const properties = leftNode.properties;
@@ -157,16 +207,23 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
                         .filter(prop => prop.value)
                         .map(prop => prop.value.name)
                         .some(name => isOuterVariableInDestructing(name, variable.scope));
+
+                    hasNonIdentifiers = hasMemberExpressionAssignment(leftNode);
+
                 } else if (leftNode.type === "ArrayPattern") {
                     const elements = leftNode.elements;
 
                     hasOuterVariables = elements
                         .map(element => element && element.name)
                         .some(name => isOuterVariableInDestructing(name, variable.scope));
+
+                    hasNonIdentifiers = hasMemberExpressionAssignment(leftNode);
                 }
-                if (hasOuterVariables) {
+
+                if (hasOuterVariables || hasNonIdentifiers) {
                     return null;
                 }
+
             }
 
             writer = reference;
@@ -192,9 +249,11 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
     if (!shouldBeConst) {
         return null;
     }
+
     if (isReadBeforeInit) {
         return variable.defs[0].name;
     }
+
     return writer.identifier;
 }
 
@@ -295,7 +354,7 @@ module.exports = {
     create(context) {
         const options = context.options[0] || {};
         const sourceCode = context.getSourceCode();
-        const checkingMixedDestructuring = options.destructuring !== "all";
+        const shouldMatchAnyDestructuredVariable = options.destructuring !== "all";
         const ignoreReadBeforeAssign = options.ignoreReadBeforeAssign === true;
         const variables = [];
 
@@ -316,7 +375,7 @@ module.exports = {
         function checkGroup(nodes) {
             const nodesToReport = nodes.filter(Boolean);
 
-            if (nodes.length && (checkingMixedDestructuring || nodesToReport.length === nodes.length)) {
+            if (nodes.length && (shouldMatchAnyDestructuredVariable || nodesToReport.length === nodes.length)) {
                 const varDeclParent = findUp(nodes[0], "VariableDeclaration", parentNode => parentNode.type.endsWith("Statement"));
                 const shouldFix = varDeclParent &&
 

tests/lib/rules/prefer-const.js

@@ -117,6 +117,54 @@ ruleTester.run("prefer-const", rule, {
             parser: fixtureParser("babel-eslint5/destructuring-object-spread")
         },
 
+        // https://github.com/eslint/eslint/issues/8308
+        {
+            code: "let predicate; [typeNode.returnType, predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [typeNode.returnType, ...predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+
+            // intentionally testing empty slot in destructuring assignment
+            code: "let predicate; [typeNode.returnType,, predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [typeNode.returnType=5, predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [[typeNode.returnType=5], predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [[typeNode.returnType, predicate]] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [typeNode.returnType, [predicate]] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [, [typeNode.returnType, predicate]] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [, {foo:typeNode.returnType, predicate}] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [, {foo:typeNode.returnType, ...predicate}] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let a; const b = {}; ({ a, c: b.c } = func());",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+
         // ignoreReadBeforeAssign
         {
             code: "let x; function foo() { bar(x); } x = 0;",
@@ -380,6 +428,33 @@ ruleTester.run("prefer-const", rule, {
                 { message: "'y' is never reassigned. Use 'const' instead.", type: "Identifier" },
                 { message: "'z' is never reassigned. Use 'const' instead.", type: "Identifier" }
             ]
+        },
+
+        // https://github.com/eslint/eslint/issues/8308
+        {
+            code: "let predicate; [, {foo:returnType, predicate}] = foo();",
+            output: null,
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                { message: "'predicate' is never reassigned. Use 'const' instead.", type: "Identifier" }
+            ]
+        },
+        {
+            code: "let predicate; [, {foo:returnType, predicate}, ...bar ] = foo();",
+            output: null,
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                { message: "'predicate' is never reassigned. Use 'const' instead.", type: "Identifier" }
+            ]
+        },
+        {
+            code: "let predicate; [, {foo:returnType, ...predicate} ] = foo();",
+            output: null,
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                { message: "'predicate' is never reassigned. Use 'const' instead.", type: "Identifier" }
+            ]
         }
+
     ]
 });