Skip to content

Commit

Permalink
Fix: prefer-const when using destructuring assign (fixes #8308)
Browse files Browse the repository at this point in the history
  • Loading branch information
nzakas committed Oct 5, 2018
1 parent 066f7e0 commit 3cc9bb3
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
43 changes: 39 additions & 4 deletions lib/rules/prefer-const.js
Expand Up @@ -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;
}
Expand Down Expand Up @@ -148,7 +149,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;
Expand All @@ -157,16 +159,47 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
.filter(prop => prop.value)
.map(prop => prop.value.name)
.some(name => isOuterVariableInDestructing(name, variable.scope));

hasNonIdentifiers = properties.some(prop => {
if (prop) {
let value = prop.value;

if (prop.value.type === "AssignmentPattern") {
value = prop.value.left;
}

return value.type !== "Identifier";
}

return false;
});
} else if (leftNode.type === "ArrayPattern") {
const elements = leftNode.elements;

hasOuterVariables = elements
.map(element => element && element.name)
.some(name => isOuterVariableInDestructing(name, variable.scope));

hasNonIdentifiers = elements.some(element => {
if (element) {
let value = element;

if (value.type === "AssignmentPattern") {
value = value.left;
}

return value.type !== "Identifier";
}

return false;

});
}
if (hasOuterVariables) {

if (hasOuterVariables || hasNonIdentifiers) {
return null;
}

}

writer = reference;
Expand All @@ -192,9 +225,11 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
if (!shouldBeConst) {
return null;
}

if (isReadBeforeInit) {
return variable.defs[0].name;
}

return writer.identifier;
}

Expand Down Expand Up @@ -295,7 +330,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 = [];

Expand All @@ -316,7 +351,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 &&

Expand Down
10 changes: 10 additions & 0 deletions tests/lib/rules/prefer-const.js
Expand Up @@ -117,6 +117,16 @@ 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 a; const b = {}; ({ a, c: b.c } = func());",
parserOptions: { ecmaVersion: 2018 }
},

// ignoreReadBeforeAssign
{
code: "let x; function foo() { bar(x); } x = 0;",
Expand Down

0 comments on commit 3cc9bb3

Please sign in to comment.