From 38639bfd3b237636426d0f6656bb007d3961e17d Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 16 Jul 2016 00:08:00 +0900 Subject: [PATCH] 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. --- docs/rules/README.md | 2 +- docs/rules/no-var.md | 2 + lib/rules/no-var.js | 135 ++++++++++++++++++++++++++++++++++++-- tests/lib/rules/no-var.js | 59 ++++++++++++++++- 4 files changed, 192 insertions(+), 6 deletions(-) diff --git a/docs/rules/README.md b/docs/rules/README.md index e5953d5a7c5..eba1b7be8d0 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -247,7 +247,7 @@ These rules relate to ES6, also known as ES2015: * [no-useless-computed-key](no-useless-computed-key.md): disallow unnecessary computed property keys in object literals * [no-useless-constructor](no-useless-constructor.md): disallow unnecessary constructors * [no-useless-rename](no-useless-rename.md): disallow renaming import, export, and destructured assignments to the same name (fixable) -* [no-var](no-var.md): require `let` or `const` instead of `var` +* [no-var](no-var.md): require `let` or `const` instead of `var` (fixable) * [object-shorthand](object-shorthand.md): require or disallow method and property shorthand syntax for object literals (fixable) * [prefer-arrow-callback](prefer-arrow-callback.md): require arrow functions as callbacks * [prefer-const](prefer-const.md): require `const` declarations for variables that are never reassigned after declared (fixable) diff --git a/docs/rules/no-var.md b/docs/rules/no-var.md index 056b733a335..f4859d5937a 100644 --- a/docs/rules/no-var.md +++ b/docs/rules/no-var.md @@ -1,5 +1,7 @@ # require `let` or `const` instead of `var` (no-var) +(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixed problems reported by this rule. + ECMAScript 6 allows programmers to create variables with block scope instead of function scope using the `let` and `const` keywords. Block scope is common in many other programming languages and helps programmers avoid mistakes such as: diff --git a/lib/rules/no-var.js b/lib/rules/no-var.js index 2e9948a330e..2a41301bd41 100644 --- a/lib/rules/no-var.js +++ b/lib/rules/no-var.js @@ -5,6 +5,72 @@ "use strict"; +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +var SCOPE_NODE_TYPE = /^(?:Program|BlockStatement|SwitchStatement|ForStatement|ForInStatement|ForOfStatement)$/; + +/** + * Gets the scope node which directly contains a given node. + * + * @param {ASTNode} node - A node to get. This is a `VariableDeclaration` or + * an `Identifier`. + * @returns {ASTNode} A scope node. This is one of `Program`, `BlockStatement`, + * `SwitchStatement`, `ForStatement`, `ForInStatement`, and + * `ForOfStatement`. + */ +function getScopeNode(node) { + while (node) { + if (SCOPE_NODE_TYPE.test(node.type)) { + return node; + } + + node = node.parent; + } + + /* istanbul ignore next : unreachable */ + return null; +} + +/** + * Checks whether a given variable is redeclared or not. + * + * @param {escope.Variable} variable - A variable to check. + * @returns {boolean} `true` if the variable is redeclared. + */ +function isRedeclared(variable) { + return variable.defs.length >= 2; +} + +/** + * Checks whether a given variable is used from outside of the specified scope. + * + * @param {ASTNode} scopeNode - A scope node to check. + * @returns {Function} The predicate function which checks whether a given + * variable is used from outside of the specified scope. + */ +function isUsedFromOutsideOf(scopeNode) { + + /** + * Checks whether a given reference is inside of the specified scope or not. + * + * @param {escope.Reference} reference - A reference to check. + * @returns {boolean} `true` if the reference is inside of the specified + * scope. + */ + function isOutsideOfScope(reference) { + var scope = scopeNode.range; + var id = reference.identifier.range; + + return id[0] < scope[0] || id[1] > scope[1]; + } + + return function(variable) { + return variable.references.some(isOutsideOfScope); + }; +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -17,19 +83,80 @@ module.exports = { recommended: false }, - schema: [] + schema: [], + fixable: "code" }, create: function(context) { + var sourceCode = context.getSourceCode(); + + /** + * Checks whether it can fix a given variable declaration or not. + * It cannot fix if the following cases: + * + * - A variable is declared on a SwitchCase node. + * - A variable is redeclared. + * - A variable is used from outside the scope. + * + * ## A variable is declared on a SwitchCase node. + * + * If this rule modifies 'var' declarations on a SwitchCase node, it + * would generate the warnings of 'no-case-declarations' rule. And the + * 'eslint:recommended' preset includes 'no-case-declarations' rule, so + * this rule doesn't modify those declarations. + * + * ## A variable is redeclared. + * + * The language spec disallows redeclarations of `let` declarations. + * Those variables would cause syntax errors. + * + * ## A variable is used from outside the scope. + * + * The language spec disallows accesses from outside of the scope for + * `let` declarations. Those variables would cause reference errors. + * + * @param {ASTNode} node - A variable declaration node to check. + * @returns {boolean} `true` if it can fix the node. + */ + function canFix(node) { + var variables = context.getDeclaredVariables(node); + var scopeNode = getScopeNode(node); + + return !( + node.parent.type === "SwitchCase" || + variables.some(isRedeclared) || + variables.some(isUsedFromOutsideOf(scopeNode)) + ); + } + + /** + * Reports a given variable declaration node. + * + * @param {ASTNode} node - A variable declaration node to report. + * @returns {void} + */ + function report(node) { + var varToken = sourceCode.getFirstToken(node); + + context.report({ + node: node, + message: "Unexpected var, use let or const instead.", + + fix: function(fixer) { + if (canFix(node)) { + return fixer.replaceText(varToken, "let"); + } + return null; + } + }); + } return { VariableDeclaration: function(node) { if (node.kind === "var") { - context.report(node, "Unexpected var, use let or const instead."); + report(node); } } - }; - } }; diff --git a/tests/lib/rules/no-var.js b/tests/lib/rules/no-var.js index 8b47c31f37a..fa071c1e42c 100644 --- a/tests/lib/rules/no-var.js +++ b/tests/lib/rules/no-var.js @@ -33,6 +33,7 @@ ruleTester.run("no-var", rule, { invalid: [ { code: "var foo = bar;", + output: "let foo = bar;", parserOptions: { ecmaVersion: 6 }, errors: [ { @@ -43,6 +44,7 @@ ruleTester.run("no-var", rule, { }, { code: "var foo = bar, toast = most;", + output: "let foo = bar, toast = most;", parserOptions: { ecmaVersion: 6 }, errors: [ { @@ -53,6 +55,7 @@ ruleTester.run("no-var", rule, { }, { code: "var foo = bar; let toast = most;", + output: "let foo = bar; let toast = most;", parserOptions: { ecmaVersion: 6 }, errors: [ { @@ -60,6 +63,60 @@ ruleTester.run("no-var", rule, { type: "VariableDeclaration" } ] - } + }, + + // Not fix if it's redeclared or it's used from outside of the scope or it's declared on a case chunk. + { + code: "var a, b, c; var a;", + output: "var a, b, c; var a;", + errors: [ + "Unexpected var, use let or const instead.", + "Unexpected var, use let or const instead." + ] + }, + { + code: "var a; if (b) { var a; }", + output: "var a; if (b) { var a; }", + errors: [ + "Unexpected var, use let or const instead.", + "Unexpected var, use let or const instead." + ] + }, + { + code: "if (foo) { var a, b, c; } a;", + output: "if (foo) { var a, b, c; } a;", + errors: [ + "Unexpected var, use let or const instead." + ] + }, + { + code: "for (var i = 0; i < 10; ++i) {} i;", + output: "for (var i = 0; i < 10; ++i) {} i;", + errors: [ + "Unexpected var, use let or const instead." + ] + }, + { + code: "for (var a in obj) {} a;", + output: "for (var a in obj) {} a;", + errors: [ + "Unexpected var, use let or const instead." + ] + }, + { + code: "for (var a of list) {} a;", + output: "for (var a of list) {} a;", + parserOptions: {ecmaVersion: 6}, + errors: [ + "Unexpected var, use let or const instead." + ] + }, + { + code: "switch (a) { case 0: var b = 1 }", + output: "switch (a) { case 0: var b = 1 }", + errors: [ + "Unexpected var, use let or const instead." + ] + }, ] });