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

@@ -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)

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:

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);
                 }
             }
-
         };
-
     }
 };

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,13 +55,68 @@ ruleTester.run("no-var", rule, {
         },
         {
             code: "var foo = bar; let toast = most;",
+            output: "let foo = bar; let toast = most;",
             parserOptions: { ecmaVersion: 6 },
             errors: [
                 {
                     message: "Unexpected var, use let or const instead.",
                     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."
+            ]
+        },
     ]
 });