Skip to content

Commit 38639bf

Browse files
mysticateanzakas
authored andcommitted
Update: make no-var fixable (fixes #6639) (#6644)
* Update: make `no-var` fixable (fixes #6639) * Fix: modify `no-var` not fixing `var` on a case chunk. * Chore: add the notes of not-fixable cases.
1 parent dfc20e9 commit 38639bf

File tree

4 files changed

+192
-6
lines changed

4 files changed

+192
-6
lines changed

docs/rules/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ These rules relate to ES6, also known as ES2015:
247247
* [no-useless-computed-key](no-useless-computed-key.md): disallow unnecessary computed property keys in object literals
248248
* [no-useless-constructor](no-useless-constructor.md): disallow unnecessary constructors
249249
* [no-useless-rename](no-useless-rename.md): disallow renaming import, export, and destructured assignments to the same name (fixable)
250-
* [no-var](no-var.md): require `let` or `const` instead of `var`
250+
* [no-var](no-var.md): require `let` or `const` instead of `var` (fixable)
251251
* [object-shorthand](object-shorthand.md): require or disallow method and property shorthand syntax for object literals (fixable)
252252
* [prefer-arrow-callback](prefer-arrow-callback.md): require arrow functions as callbacks
253253
* [prefer-const](prefer-const.md): require `const` declarations for variables that are never reassigned after declared (fixable)

docs/rules/no-var.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# require `let` or `const` instead of `var` (no-var)
22

3+
(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixed problems reported by this rule.
4+
35
ECMAScript 6 allows programmers to create variables with block scope instead of function scope using the `let`
46
and `const` keywords. Block scope is common in many other programming languages and helps programmers avoid mistakes
57
such as:

lib/rules/no-var.js

Lines changed: 131 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,72 @@
55

66
"use strict";
77

8+
//------------------------------------------------------------------------------
9+
// Helpers
10+
//------------------------------------------------------------------------------
11+
12+
var SCOPE_NODE_TYPE = /^(?:Program|BlockStatement|SwitchStatement|ForStatement|ForInStatement|ForOfStatement)$/;
13+
14+
/**
15+
* Gets the scope node which directly contains a given node.
16+
*
17+
* @param {ASTNode} node - A node to get. This is a `VariableDeclaration` or
18+
* an `Identifier`.
19+
* @returns {ASTNode} A scope node. This is one of `Program`, `BlockStatement`,
20+
* `SwitchStatement`, `ForStatement`, `ForInStatement`, and
21+
* `ForOfStatement`.
22+
*/
23+
function getScopeNode(node) {
24+
while (node) {
25+
if (SCOPE_NODE_TYPE.test(node.type)) {
26+
return node;
27+
}
28+
29+
node = node.parent;
30+
}
31+
32+
/* istanbul ignore next : unreachable */
33+
return null;
34+
}
35+
36+
/**
37+
* Checks whether a given variable is redeclared or not.
38+
*
39+
* @param {escope.Variable} variable - A variable to check.
40+
* @returns {boolean} `true` if the variable is redeclared.
41+
*/
42+
function isRedeclared(variable) {
43+
return variable.defs.length >= 2;
44+
}
45+
46+
/**
47+
* Checks whether a given variable is used from outside of the specified scope.
48+
*
49+
* @param {ASTNode} scopeNode - A scope node to check.
50+
* @returns {Function} The predicate function which checks whether a given
51+
* variable is used from outside of the specified scope.
52+
*/
53+
function isUsedFromOutsideOf(scopeNode) {
54+
55+
/**
56+
* Checks whether a given reference is inside of the specified scope or not.
57+
*
58+
* @param {escope.Reference} reference - A reference to check.
59+
* @returns {boolean} `true` if the reference is inside of the specified
60+
* scope.
61+
*/
62+
function isOutsideOfScope(reference) {
63+
var scope = scopeNode.range;
64+
var id = reference.identifier.range;
65+
66+
return id[0] < scope[0] || id[1] > scope[1];
67+
}
68+
69+
return function(variable) {
70+
return variable.references.some(isOutsideOfScope);
71+
};
72+
}
73+
874
//------------------------------------------------------------------------------
975
// Rule Definition
1076
//------------------------------------------------------------------------------
@@ -17,19 +83,80 @@ module.exports = {
1783
recommended: false
1884
},
1985

20-
schema: []
86+
schema: [],
87+
fixable: "code"
2188
},
2289

2390
create: function(context) {
91+
var sourceCode = context.getSourceCode();
92+
93+
/**
94+
* Checks whether it can fix a given variable declaration or not.
95+
* It cannot fix if the following cases:
96+
*
97+
* - A variable is declared on a SwitchCase node.
98+
* - A variable is redeclared.
99+
* - A variable is used from outside the scope.
100+
*
101+
* ## A variable is declared on a SwitchCase node.
102+
*
103+
* If this rule modifies 'var' declarations on a SwitchCase node, it
104+
* would generate the warnings of 'no-case-declarations' rule. And the
105+
* 'eslint:recommended' preset includes 'no-case-declarations' rule, so
106+
* this rule doesn't modify those declarations.
107+
*
108+
* ## A variable is redeclared.
109+
*
110+
* The language spec disallows redeclarations of `let` declarations.
111+
* Those variables would cause syntax errors.
112+
*
113+
* ## A variable is used from outside the scope.
114+
*
115+
* The language spec disallows accesses from outside of the scope for
116+
* `let` declarations. Those variables would cause reference errors.
117+
*
118+
* @param {ASTNode} node - A variable declaration node to check.
119+
* @returns {boolean} `true` if it can fix the node.
120+
*/
121+
function canFix(node) {
122+
var variables = context.getDeclaredVariables(node);
123+
var scopeNode = getScopeNode(node);
124+
125+
return !(
126+
node.parent.type === "SwitchCase" ||
127+
variables.some(isRedeclared) ||
128+
variables.some(isUsedFromOutsideOf(scopeNode))
129+
);
130+
}
131+
132+
/**
133+
* Reports a given variable declaration node.
134+
*
135+
* @param {ASTNode} node - A variable declaration node to report.
136+
* @returns {void}
137+
*/
138+
function report(node) {
139+
var varToken = sourceCode.getFirstToken(node);
140+
141+
context.report({
142+
node: node,
143+
message: "Unexpected var, use let or const instead.",
144+
145+
fix: function(fixer) {
146+
if (canFix(node)) {
147+
return fixer.replaceText(varToken, "let");
148+
}
149+
return null;
150+
}
151+
});
152+
}
24153

25154
return {
26155
VariableDeclaration: function(node) {
27156
if (node.kind === "var") {
28-
context.report(node, "Unexpected var, use let or const instead.");
157+
report(node);
29158
}
30159
}
31-
32160
};
33-
34161
}
35162
};

tests/lib/rules/no-var.js

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ ruleTester.run("no-var", rule, {
3333
invalid: [
3434
{
3535
code: "var foo = bar;",
36+
output: "let foo = bar;",
3637
parserOptions: { ecmaVersion: 6 },
3738
errors: [
3839
{
@@ -43,6 +44,7 @@ ruleTester.run("no-var", rule, {
4344
},
4445
{
4546
code: "var foo = bar, toast = most;",
47+
output: "let foo = bar, toast = most;",
4648
parserOptions: { ecmaVersion: 6 },
4749
errors: [
4850
{
@@ -53,13 +55,68 @@ ruleTester.run("no-var", rule, {
5355
},
5456
{
5557
code: "var foo = bar; let toast = most;",
58+
output: "let foo = bar; let toast = most;",
5659
parserOptions: { ecmaVersion: 6 },
5760
errors: [
5861
{
5962
message: "Unexpected var, use let or const instead.",
6063
type: "VariableDeclaration"
6164
}
6265
]
63-
}
66+
},
67+
68+
// Not fix if it's redeclared or it's used from outside of the scope or it's declared on a case chunk.
69+
{
70+
code: "var a, b, c; var a;",
71+
output: "var a, b, c; var a;",
72+
errors: [
73+
"Unexpected var, use let or const instead.",
74+
"Unexpected var, use let or const instead."
75+
]
76+
},
77+
{
78+
code: "var a; if (b) { var a; }",
79+
output: "var a; if (b) { var a; }",
80+
errors: [
81+
"Unexpected var, use let or const instead.",
82+
"Unexpected var, use let or const instead."
83+
]
84+
},
85+
{
86+
code: "if (foo) { var a, b, c; } a;",
87+
output: "if (foo) { var a, b, c; } a;",
88+
errors: [
89+
"Unexpected var, use let or const instead."
90+
]
91+
},
92+
{
93+
code: "for (var i = 0; i < 10; ++i) {} i;",
94+
output: "for (var i = 0; i < 10; ++i) {} i;",
95+
errors: [
96+
"Unexpected var, use let or const instead."
97+
]
98+
},
99+
{
100+
code: "for (var a in obj) {} a;",
101+
output: "for (var a in obj) {} a;",
102+
errors: [
103+
"Unexpected var, use let or const instead."
104+
]
105+
},
106+
{
107+
code: "for (var a of list) {} a;",
108+
output: "for (var a of list) {} a;",
109+
parserOptions: {ecmaVersion: 6},
110+
errors: [
111+
"Unexpected var, use let or const instead."
112+
]
113+
},
114+
{
115+
code: "switch (a) { case 0: var b = 1 }",
116+
output: "switch (a) { case 0: var b = 1 }",
117+
errors: [
118+
"Unexpected var, use let or const instead."
119+
]
120+
},
64121
]
65122
});

0 commit comments

Comments
 (0)