Skip to content

Commit 87d74b2

Browse files
mysticateamysticatea
committed
Fix: prefer-const got to not change scopes (refs #5284)
1 parent ce88bc2 commit 87d74b2

File tree

2 files changed

+79
-184
lines changed

2 files changed

+79
-184
lines changed

‎lib/rules/prefer-const.js

Lines changed: 70 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -10,215 +10,122 @@
1010
// Helpers
1111
//------------------------------------------------------------------------------
1212

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

1815
/**
19-
* Gets a write reference from a given variable if the variable is never
20-
* reassigned.
16+
* Adds multiple items to the tail of an array.
2117
*
22-
* @param {escope.Variable} variable - A variable to get.
23-
* @returns {escope.Reference|null} A write reference or null.
18+
* @param {any[]} array - A destination to add.
19+
* @param {any[]} values - Items to be added.
20+
* @returns {void}
2421
*/
25-
function getWriteReferenceIfOnce(variable) {
26-
var retv = null;
27-
28-
var references = variable.references;
29-
for (var i = 0; i < references.length; ++i) {
30-
var reference = references[i];
31-
32-
if (reference.isWrite()) {
33-
if (retv && !(retv.init && reference.init)) {
34-
// This variable is reassigned.
35-
return null;
36-
}
37-
retv = reference;
38-
}
39-
}
40-
41-
return retv;
42-
}
22+
var pushAll = Function.apply.bind(Array.prototype.push);
4323

4424
/**
45-
* Checks whether or not a given reference is in a loop condition or a
46-
* for-loop's updater.
25+
* Checks whether a given node is located at `ForStatement.init` or not.
4726
*
48-
* @param {escope.Reference} reference - A reference to check.
49-
* @returns {boolean} `true` if the reference is in a loop condition or a
50-
* for-loop's updater.
27+
* @param {ASTNode} node - A node to check.
28+
* @returns {boolean} `true` if the node is located at `ForStatement.init`.
5129
*/
52-
function isInLoopHead(reference) {
53-
var node = reference.identifier;
54-
var parent = node.parent;
55-
var assignment = false;
56-
57-
while (parent) {
58-
if (LOOP_TYPES.test(parent.type)) {
59-
return true;
60-
}
61-
62-
// VariableDeclaration can be at ForInStatement.left
63-
// This is catching the code like `for (const {b = ++a} of foo()) { ... }`
64-
if (assignment &&
65-
parent.type === "VariableDeclaration" &&
66-
FOR_IN_OF_TYPES.test(parent.parent.type) &&
67-
parent.parent.left === parent
68-
) {
69-
return true;
70-
}
71-
if (parent.type === "AssignmentPattern") {
72-
assignment = true;
73-
}
74-
75-
// If a declaration or a statement was found before a loop,
76-
// it's not in the head of a loop.
77-
if (SENTINEL_TYPES.test(parent.type)) {
78-
break;
79-
}
80-
81-
node = parent;
82-
parent = parent.parent;
83-
}
84-
85-
return false;
30+
function isInitOfForStatement(node) {
31+
return node.parent.type === "ForStatement" && node.parent.init === node;
8632
}
8733

8834
/**
89-
* Gets the end position of a given reference.
90-
* This position is used to check every ReadReferences exist after the given
91-
* reference.
92-
*
93-
* If the reference is belonging to AssignmentExpression or UpdateExpression,
94-
* this function returns the most rear position of those nodes.
95-
* The range of those nodes are executed before the assignment.
35+
* Checks whether a given Identifier node becomes a VariableDeclaration or not.
9636
*
97-
* @param {escope.Reference} writer - A reference to get.
98-
* @returns {number} The end position of the reference.
37+
* @param {ASTNode} identifier - An Identifier node to check.
38+
* @returns {boolean} `true` if the node can become a VariableDeclaration.
9939
*/
100-
function getEndPosition(writer) {
101-
var node = writer.identifier;
102-
var end = node.range[1];
103-
104-
// Detects the end position of the assignment expression the reference is
105-
// belonging to.
106-
while ((node = node.parent)) {
107-
if (END_POSITION_TYPES.test(node.type)) {
108-
end = node.range[1];
109-
}
110-
if (SENTINEL_TYPES.test(node.type)) {
111-
break;
112-
}
40+
function canBecomeVariableDeclaration(identifier) {
41+
var node = identifier.parent;
42+
while (PATTERN_TYPE.test(node.type)) {
43+
node = node.parent;
11344
}
11445

115-
return end;
46+
return (
47+
node.type === "VariableDeclarator" ||
48+
(
49+
node.type === "AssignmentExpression" &&
50+
node.parent.type === "ExpressionStatement"
51+
)
52+
);
11653
}
11754

11855
/**
119-
* Gets a function which checks a given reference with the following condition:
120-
*
121-
* - The reference is a ReadReference.
122-
* - The reference exists after a specific WriteReference.
123-
* - The reference exists inside of the scope a specific WriteReference is
124-
* belonging to.
56+
* Gets the WriteReference of a given variable if the variable is never
57+
* reassigned.
12558
*
126-
* @param {escope.Reference} writer - A reference to check.
127-
* @returns {function} A function which checks a given reference.
59+
* @param {escope.Variable} variable - A variable to get.
60+
* @returns {escope.Reference|null} The singular WriteReference or null.
12861
*/
129-
function isInScope(writer) {
130-
var start = getEndPosition(writer);
131-
var end = writer.from.block.range[1];
62+
function getWriteReferenceIfOnce(variable) {
63+
var retv = null;
13264

133-
return function(reference) {
134-
if (!reference.isRead()) {
135-
return true;
65+
var references = variable.references;
66+
for (var i = 0; i < references.length; ++i) {
67+
var reference = references[i];
68+
69+
if (reference.isWrite()) {
70+
if (retv && !(retv.init && reference.init)) {
71+
// This variable is reassigned.
72+
return null;
73+
}
74+
retv = reference;
13675
}
76+
}
13777

138-
var range = reference.identifier.range;
139-
return start <= range[0] && range[1] <= end;
140-
};
78+
return retv;
14179
}
14280

14381
//------------------------------------------------------------------------------
14482
// Rule Definition
14583
//------------------------------------------------------------------------------
14684

14785
module.exports = function(context) {
86+
var variables = null;
14887

14988
/**
150-
* Searches and reports variables that are never reassigned after declared.
151-
* @param {Scope} scope - A scope of the search domain.
89+
* Reports a given variable if the singular WriteReference of the variable
90+
* exists in the same scope as the declaration.
91+
*
92+
* @param {escope.Variable} variable - A variable to check.
15293
* @returns {void}
15394
*/
154-
function checkForVariables(scope) {
155-
// Skip the TDZ type.
156-
if (scope.type === "TDZ") {
95+
function checkVariable(variable) {
96+
if (variable.eslintUsed) {
15797
return;
15898
}
15999

160-
var variables = scope.variables;
161-
for (var i = 0; i < variables.length; ++i) {
162-
var variable = variables[i];
163-
var def = variable.defs[0];
164-
var declaration = def && def.parent;
165-
var statement = declaration && declaration.parent;
166-
var references = variable.references;
167-
var identifier = variable.identifiers[0];
168-
169-
// Skips excludes `let`.
170-
// And skips if it's at `ForStatement.init`.
171-
if (!declaration ||
172-
declaration.type !== "VariableDeclaration" ||
173-
declaration.kind !== "let" ||
174-
(statement.type === "ForStatement" && statement.init === declaration)
175-
) {
176-
continue;
177-
}
178-
179-
// Checks references.
180-
// - One WriteReference exists.
181-
// - Two or more WriteReference don't exist.
182-
// - Every ReadReference exists after the WriteReference.
183-
// - The WriteReference doesn't exist in a loop condition.
184-
// - If `eslintUsed` is true, we cannot know where it was used from.
185-
// In this case, if the scope of the variable would change, it
186-
// skips the variable.
187-
var writer = getWriteReferenceIfOnce(variable);
188-
if (writer &&
189-
!(variable.eslintUsed && variable.scope !== writer.from) &&
190-
!isInLoopHead(writer) &&
191-
references.every(isInScope(writer))
192-
) {
193-
context.report({
194-
node: identifier,
195-
message: "'{{name}}' is never reassigned, use 'const' instead.",
196-
data: identifier
197-
});
198-
}
100+
var writer = getWriteReferenceIfOnce(variable);
101+
if (writer &&
102+
writer.from === variable.scope &&
103+
canBecomeVariableDeclaration(writer.identifier)
104+
) {
105+
context.report({
106+
node: writer.identifier,
107+
message: "'{{name}}' is never reassigned, use 'const' instead.",
108+
data: variable
109+
});
199110
}
200111
}
201112

202-
/**
203-
* Adds multiple items to the tail of an array.
204-
* @param {any[]} array - A destination to add.
205-
* @param {any[]} values - Items to be added.
206-
* @returns {void}
207-
*/
208-
var pushAll = Function.apply.bind(Array.prototype.push);
209-
210113
return {
114+
"Program": function() {
115+
variables = [];
116+
},
117+
211118
"Program:exit": function() {
212-
var stack = [context.getScope()];
213-
while (stack.length) {
214-
var scope = stack.pop();
215-
pushAll(stack, scope.childScopes);
119+
variables.forEach(checkVariable);
120+
variables = null;
121+
},
216122

217-
checkForVariables(scope);
123+
"VariableDeclaration": function(node) {
124+
if (node.kind === "let" && !isInitOfForStatement(node)) {
125+
pushAll(variables, context.getDeclaredVariables(node));
218126
}
219127
}
220128
};
221-
222129
};
223130

224131
module.exports.schema = [];

‎tests/lib/rules/prefer-const.js

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,15 @@ ruleTester.run("prefer-const", rule, {
6262
].join("\n"),
6363
parserOptions: { ecmaVersion: 6 }
6464
},
65-
{ code: "/*exported a*/ let a; function init() { a = foo(); }", parserOptions: { ecmaVersion: 6 } }
65+
{ code: "/*exported a*/ let a; function init() { a = foo(); }", parserOptions: { ecmaVersion: 6 } },
66+
67+
// The assignment is located in a different scope.
68+
// Those are warned by prefer-smaller-scope.
69+
{ code: "let x; { x = 0; foo(x); }", parserOptions: { ecmaVersion: 6 } },
70+
{ code: "(function() { let x; { x = 0; foo(x); } })();", parserOptions: { ecmaVersion: 6 } },
71+
{ code: "let x; for (const a of [1,2,3]) { x = foo(); bar(x); }", parserOptions: { ecmaVersion: 6 } },
72+
{ code: "(function() { let x; for (const a of [1,2,3]) { x = foo(); bar(x); } })();", parserOptions: { ecmaVersion: 6 } },
73+
{ code: "let x; for (x of array) { x; }", parserOptions: { ecmaVersion: 6 } }
6674
],
6775
invalid: [
6876
{
@@ -143,26 +151,6 @@ ruleTester.run("prefer-const", rule, {
143151
code: "(function() { let x; x = 1; })();",
144152
parserOptions: { ecmaVersion: 6 },
145153
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
146-
},
147-
{
148-
code: "let x; { x = 0; foo(x); }",
149-
parserOptions: { ecmaVersion: 6 },
150-
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
151-
},
152-
{
153-
code: "(function() { let x; { x = 0; foo(x); } })();",
154-
parserOptions: { ecmaVersion: 6 },
155-
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
156-
},
157-
{
158-
code: "let x; for (const a of [1,2,3]) { x = foo(); bar(x); }",
159-
parserOptions: { ecmaVersion: 6 },
160-
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
161-
},
162-
{
163-
code: "(function() { let x; for (const a of [1,2,3]) { x = foo(); bar(x); } })();",
164-
parserOptions: { ecmaVersion: 6 },
165-
errors: [{ message: "'x' is never reassigned, use 'const' instead.", type: "Identifier"}]
166154
}
167155
]
168156
});

0 commit comments

Comments
 (0)