Skip to content

Commit

Permalink
Merge pull request #5591 from eslint/prefer-const/ensure-same-scope
Browse files Browse the repository at this point in the history 
Fix: `prefer-const` got to not change scopes (refs #5284)
  • Loading branch information
@nzakas
nzakas committed Mar 17, 2016
2 parents 3e3b17b + 87d74b2 commit 2204693
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 184 deletions.
233 changes: 70 additions & 163 deletions lib/rules/prefer-const.js
View file
Expand Up @@ -10,215 +10,122 @@
// Helpers
//------------------------------------------------------------------------------

var LOOP_TYPES = /^(?:While|DoWhile|For|ForIn|ForOf)Statement$/;
var FOR_IN_OF_TYPES = /^For(?:In|Of)Statement$/;
var SENTINEL_TYPES = /(?:Declaration|Statement)$/;
var END_POSITION_TYPES = /^(?:Assignment|Update)/;
var PATTERN_TYPE = /^(?:.+?Pattern|RestElement|Property)$/;

/**
* Gets a write reference from a given variable if the variable is never
* reassigned.
* Adds multiple items to the tail of an array.
*
* @param {escope.Variable} variable - A variable to get.
* @returns {escope.Reference|null} A write reference or null.
* @param {any[]} array - A destination to add.
* @param {any[]} values - Items to be added.
* @returns {void}
*/
function getWriteReferenceIfOnce(variable) {
var retv = null;

var references = variable.references;
for (var i = 0; i < references.length; ++i) {
var reference = references[i];

if (reference.isWrite()) {
if (retv && !(retv.init && reference.init)) {
// This variable is reassigned.
return null;
}
retv = reference;
}
}

return retv;
}
var pushAll = Function.apply.bind(Array.prototype.push);

/**
* Checks whether or not a given reference is in a loop condition or a
* for-loop's updater.
* Checks whether a given node is located at `ForStatement.init` or not.
*
* @param {escope.Reference} reference - A reference to check.
* @returns {boolean} `true` if the reference is in a loop condition or a
* for-loop's updater.
* @param {ASTNode} node - A node to check.
* @returns {boolean} `true` if the node is located at `ForStatement.init`.
*/
function isInLoopHead(reference) {
var node = reference.identifier;
var parent = node.parent;
var assignment = false;

while (parent) {
if (LOOP_TYPES.test(parent.type)) {
return true;
}

// VariableDeclaration can be at ForInStatement.left
// This is catching the code like `for (const {b = ++a} of foo()) { ... }`
if (assignment &&
parent.type === "VariableDeclaration" &&
FOR_IN_OF_TYPES.test(parent.parent.type) &&
parent.parent.left === parent
) {
return true;
}
if (parent.type === "AssignmentPattern") {
assignment = true;
}

// If a declaration or a statement was found before a loop,
// it's not in the head of a loop.
if (SENTINEL_TYPES.test(parent.type)) {
break;
}

node = parent;
parent = parent.parent;
}

return false;
function isInitOfForStatement(node) {
return node.parent.type === "ForStatement" && node.parent.init === node;
}

/**
* Gets the end position of a given reference.
* This position is used to check every ReadReferences exist after the given
* reference.
*
* If the reference is belonging to AssignmentExpression or UpdateExpression,
* this function returns the most rear position of those nodes.
* The range of those nodes are executed before the assignment.
* Checks whether a given Identifier node becomes a VariableDeclaration or not.
*
* @param {escope.Reference} writer - A reference to get.
* @returns {number} The end position of the reference.
* @param {ASTNode} identifier - An Identifier node to check.
* @returns {boolean} `true` if the node can become a VariableDeclaration.
*/
function getEndPosition(writer) {
var node = writer.identifier;
var end = node.range[1];

// Detects the end position of the assignment expression the reference is
// belonging to.
while ((node = node.parent)) {
if (END_POSITION_TYPES.test(node.type)) {
end = node.range[1];
}
if (SENTINEL_TYPES.test(node.type)) {
break;
}
function canBecomeVariableDeclaration(identifier) {
var node = identifier.parent;
while (PATTERN_TYPE.test(node.type)) {
node = node.parent;
}

return end;
return (
node.type === "VariableDeclarator" ||
(
node.type === "AssignmentExpression" &&
node.parent.type === "ExpressionStatement"
)
);
}

/**
* Gets a function which checks a given reference with the following condition:
*
* - The reference is a ReadReference.
* - The reference exists after a specific WriteReference.
* - The reference exists inside of the scope a specific WriteReference is
* belonging to.
* Gets the WriteReference of a given variable if the variable is never
* reassigned.
*
* @param {escope.Reference} writer - A reference to check.
* @returns {function} A function which checks a given reference.
* @param {escope.Variable} variable - A variable to get.
* @returns {escope.Reference|null} The singular WriteReference or null.
*/
function isInScope(writer) {
var start = getEndPosition(writer);
var end = writer.from.block.range[1];
function getWriteReferenceIfOnce(variable) {
var retv = null;

return function(reference) {
if (!reference.isRead()) {
return true;
var references = variable.references;
for (var i = 0; i < references.length; ++i) {
var reference = references[i];

if (reference.isWrite()) {
if (retv && !(retv.init && reference.init)) {
// This variable is reassigned.
return null;
}
retv = reference;
}
}

var range = reference.identifier.range;
return start <= range[0] && range[1] <= end;
};
return retv;
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = function(context) {
var variables = null;

/**
* Searches and reports variables that are never reassigned after declared.
* @param {Scope} scope - A scope of the search domain.
* Reports a given variable if the singular WriteReference of the variable
* exists in the same scope as the declaration.
*
* @param {escope.Variable} variable - A variable to check.
* @returns {void}
*/
function checkForVariables(scope) {
// Skip the TDZ type.
if (scope.type === "TDZ") {
function checkVariable(variable) {
if (variable.eslintUsed) {
return;
}

var variables = scope.variables;
for (var i = 0; i < variables.length; ++i) {
var variable = variables[i];
var def = variable.defs[0];
var declaration = def && def.parent;
var statement = declaration && declaration.parent;
var references = variable.references;
var identifier = variable.identifiers[0];

// Skips excludes `let`.
// And skips if it's at `ForStatement.init`.
if (!declaration ||
declaration.type !== "VariableDeclaration" ||
declaration.kind !== "let" ||
(statement.type === "ForStatement" && statement.init === declaration)
) {
continue;
}

// Checks references.
// - One WriteReference exists.
// - Two or more WriteReference don't exist.
// - Every ReadReference exists after the WriteReference.
// - The WriteReference doesn't exist in a loop condition.
// - If `eslintUsed` is true, we cannot know where it was used from.
// In this case, if the scope of the variable would change, it
// skips the variable.
var writer = getWriteReferenceIfOnce(variable);
if (writer &&
!(variable.eslintUsed && variable.scope !== writer.from) &&
!isInLoopHead(writer) &&
references.every(isInScope(writer))
) {
context.report({
node: identifier,
message: "'{{name}}' is never reassigned, use 'const' instead.",
data: identifier
});
}
var writer = getWriteReferenceIfOnce(variable);
if (writer &&
writer.from === variable.scope &&
canBecomeVariableDeclaration(writer.identifier)
) {
context.report({
node: writer.identifier,
message: "'{{name}}' is never reassigned, use 'const' instead.",
data: variable
});
}
}

/**
* Adds multiple items to the tail of an array.
* @param {any[]} array - A destination to add.
* @param {any[]} values - Items to be added.
* @returns {void}
*/
var pushAll = Function.apply.bind(Array.prototype.push);

return {
"Program": function() {
variables = [];
},

"Program:exit": function() {
var stack = [context.getScope()];
while (stack.length) {
var scope = stack.pop();
pushAll(stack, scope.childScopes);
variables.forEach(checkVariable);
variables = null;
},

checkForVariables(scope);
"VariableDeclaration": function(node) {
if (node.kind === "let" && !isInitOfForStatement(node)) {
pushAll(variables, context.getDeclaredVariables(node));
}
}
};

};

module.exports.schema = [];
30 changes: 9 additions & 21 deletions tests/lib/rules/prefer-const.js
View file
Expand Up @@ -62,7 +62,15 @@ ruleTester.run("prefer-const", rule, {
].join("\n"),
parserOptions: { ecmaVersion: 6 }
},
{ code: "/*exported a*/ let a; function init() { a = foo(); }", parserOptions: { ecmaVersion: 6 } }
{ code: "/*exported a*/ let a; function init() { a = foo(); }", parserOptions: { ecmaVersion: 6 } },

// The assignment is located in a different scope.
// Those are warned by prefer-smaller-scope.
{ code: "let x; { x = 0; foo(x); }", parserOptions: { ecmaVersion: 6 } },
{ code: "(function() { let x; { x = 0; foo(x); } })();", parserOptions: { ecmaVersion: 6 } },
{ code: "let x; for (const a of [1,2,3]) { x = foo(); bar(x); }", parserOptions: { ecmaVersion: 6 } },
{ code: "(function() { let x; for (const a of [1,2,3]) { x = foo(); bar(x); } })();", parserOptions: { ecmaVersion: 6 } },
{ code: "let x; for (x of array) { x; }", parserOptions: { ecmaVersion: 6 } }
],
invalid: [
{
Expand Down Expand Up @@ -143,26 +151,6 @@ ruleTester.run("prefer-const", rule, {
code: "(function() { let x; x = 1; })();",
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
},
{
code: "let x; { x = 0; foo(x); }",
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
},
{
code: "(function() { let x; { x = 0; foo(x); } })();",
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
},
{
code: "let x; for (const a of [1,2,3]) { x = foo(); bar(x); }",
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
},
{
code: "(function() { let x; for (const a of [1,2,3]) { x = foo(); bar(x); } })();",
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
}
]
});

0 comments on commit 2204693

Please sign in to comment.