diff --git a/lib/rules/prefer-const.js b/lib/rules/prefer-const.js index f7dc9943332..74d6dc9d145 100644 --- a/lib/rules/prefer-const.js +++ b/lib/rules/prefer-const.js @@ -10,134 +10,72 @@ // 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; } //------------------------------------------------------------------------------ @@ -145,80 +83,49 @@ function isInScope(writer) { //------------------------------------------------------------------------------ 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 = []; diff --git a/tests/lib/rules/prefer-const.js b/tests/lib/rules/prefer-const.js index f8b4d3e6584..29b1b95d87b 100644 --- a/tests/lib/rules/prefer-const.js +++ b/tests/lib/rules/prefer-const.js @@ -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: [ { @@ -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"}] } ] });