Fix: no-var does not fix if causes ReferenceError (fixes #7950) (#7953

)

lib/rules/no-var.js

@@ -128,6 +128,43 @@ function isUsedFromOutsideOf(scopeNode) {
     };
 }
 
+/**
+ * Creates the predicate function which checks whether a variable has their references in TDZ.
+ *
+ * The predicate function would return `true`:
+ *
+ * - if a reference is before the declarator. E.g. (var a = b, b = 1;)(var {a = b, b} = {};)
+ * - if a reference is in the expression of their default value.  E.g. (var {a = a} = {};)
+ * - if a reference is in the expression of their initializer.  E.g. (var a = a;)
+ *
+ * @param {ASTNode} node - The initializer node of VariableDeclarator.
+ * @returns {Function} The predicate function.
+ * @private
+ */
+function hasReferenceInTDZ(node) {
+    const initStart = node.range[0];
+    const initEnd = node.range[1];
+
+    return variable => {
+        const id = variable.defs[0].name;
+        const idStart = id.range[0];
+        const defaultValue = (id.parent.type === "AssignmentPattern" ? id.parent.right : null);
+        const defaultStart = defaultValue && defaultValue.range[0];
+        const defaultEnd = defaultValue && defaultValue.range[1];
+
+        return variable.references.some(reference => {
+            const start = reference.identifier.range[0];
+            const end = reference.identifier.range[1];
+
+            return !reference.init && (
+                start < idStart ||
+                (defaultValue !== null && start >= defaultStart && end <= defaultEnd) ||
+                (start >= initStart && end <= initEnd)
+            );
+        });
+    };
+}
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -147,6 +184,21 @@ module.exports = {
     create(context) {
         const sourceCode = context.getSourceCode();
 
+        /**
+         * Checks whether the variables which are defined by the given declarator node have their references in TDZ.
+         *
+         * @param {ASTNode} declarator - The VariableDeclarator node to check.
+         * @returns {boolean} `true` if one of the variables which are defined by the given declarator node have their references in TDZ.
+         */
+        function hasSelfReferenceInTDZ(declarator) {
+            if (!declarator.init) {
+                return false;
+            }
+            const variables = context.getDeclaredVariables(declarator);
+
+            return variables.some(hasReferenceInTDZ(declarator.init));
+        }
+
         /**
          * Checks whether it can fix a given variable declaration or not.
          * It cannot fix if the following cases:
@@ -156,6 +208,7 @@ module.exports = {
          * - A variable is used from outside the scope.
          * - A variable is used from a closure within a loop.
          * - A variable might be used before it is assigned within a loop.
+         * - A variable might be used in TDZ.
          *
          * ## A variable is declared on a SwitchCase node.
          *
@@ -201,8 +254,10 @@ module.exports = {
             const scopeNode = getScopeNode(node);
 
             if (node.parent.type === "SwitchCase" ||
-                    variables.some(isRedeclared) ||
-                    variables.some(isUsedFromOutsideOf(scopeNode))) {
+                node.declarations.some(hasSelfReferenceInTDZ) ||
+                variables.some(isRedeclared) ||
+                variables.some(isUsedFromOutsideOf(scopeNode))
+            ) {
                 return false;
             }
 
@@ -241,7 +296,7 @@ module.exports = {
         }
 
         return {
-            VariableDeclaration(node) {
+            "VariableDeclaration:exit"(node) {
                 if (node.kind === "var") {
                     report(node);
                 }

tests/lib/rules/no-var.js

@@ -154,6 +154,67 @@ ruleTester.run("no-var", rule, {
             errors: [
                 "Unexpected var, use let or const instead."
             ]
+        },
+
+        // https://github.com/eslint/eslint/issues/7950
+        {
+            code: "var a = a",
+            output: "var a = a",
+            errors: [
+                "Unexpected var, use let or const instead."
+            ]
+        },
+        {
+            code: "var {a = a} = {}",
+            output: "var {a = a} = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [
+                "Unexpected var, use let or const instead."
+            ]
+        },
+        {
+            code: "var {a = b, b} = {}",
+            output: "var {a = b, b} = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [
+                "Unexpected var, use let or const instead."
+            ]
+        },
+        {
+            code: "var {a, b = a} = {}",
+            output: "let {a, b = a} = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [
+                "Unexpected var, use let or const instead."
+            ]
+        },
+        {
+            code: "var a = b, b = 1",
+            output: "var a = b, b = 1",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [
+                "Unexpected var, use let or const instead."
+            ]
+        },
+        {
+            code: "var a = b; var b = 1",
+            output: "let a = b; var b = 1",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [
+                "Unexpected var, use let or const instead.",
+                "Unexpected var, use let or const instead."
+            ]
+        },
+
+        // This case is not in TDZ, but it's very hard to distinguish the reference is in TDZ or not.
+        // So this rule does not fix it for safe.
+        {
+            code: "function foo() { a } var a = 1; foo()",
+            output: "function foo() { a } var a = 1; foo()",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [
+                "Unexpected var, use let or const instead."
+            ]
         }
     ]
 });