From d80b9d0752d36148d4852f47903f675ca2f272db Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 28 Oct 2017 05:01:08 +0900 Subject: [PATCH] Fix: no-var don't fix globals (fixes #9520) (#9525) --- lib/rules/no-var.js | 11 +++++++ tests/lib/rules/no-var.js | 65 ++++++++++++++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/lib/rules/no-var.js b/lib/rules/no-var.js index c74e0b9ad9f..d3c163e5577 100644 --- a/lib/rules/no-var.js +++ b/lib/rules/no-var.js @@ -15,6 +15,15 @@ const astUtils = require("../ast-utils"); // Helpers //------------------------------------------------------------------------------ +/** + * Check whether a given variable is a global variable or not. + * @param {eslint-scope.Variable} variable The variable to check. + * @returns {boolean} `true` if the variable is a global variable. + */ +function isGlobal(variable) { + return Boolean(variable.scope) && variable.scope.type === "global"; +} + /** * Finds the nearest function scope or global scope walking up the scope * hierarchy. @@ -203,6 +212,7 @@ module.exports = { * Checks whether it can fix a given variable declaration or not. * It cannot fix if the following cases: * + * - A variable is a global variable. * - A variable is declared on a SwitchCase node. * - A variable is redeclared. * - A variable is used from outside the scope. @@ -256,6 +266,7 @@ module.exports = { if (node.parent.type === "SwitchCase" || node.declarations.some(hasSelfReferenceInTDZ) || + variables.some(isGlobal) || variables.some(isRedeclared) || variables.some(isUsedFromOutsideOf(scopeNode)) ) { diff --git a/tests/lib/rules/no-var.js b/tests/lib/rules/no-var.js index 8301041805b..48511d82592 100644 --- a/tests/lib/rules/no-var.js +++ b/tests/lib/rules/no-var.js @@ -21,13 +21,22 @@ const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } }); ruleTester.run("no-var", rule, { valid: [ "const JOE = 'schmoe';", - "let moo = 'car';" + "let moo = 'car';", + { + code: "const JOE = 'schmoe';", + parserOptions: { ecmaFeatures: { globalReturn: true } } + }, + { + code: "let moo = 'car';", + parserOptions: { ecmaFeatures: { globalReturn: true } } + } ], invalid: [ { code: "var foo = bar;", output: "let foo = bar;", + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", @@ -38,6 +47,7 @@ ruleTester.run("no-var", rule, { { code: "var foo = bar, toast = most;", output: "let foo = bar, toast = most;", + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", @@ -48,6 +58,7 @@ ruleTester.run("no-var", rule, { { code: "var foo = bar; let toast = most;", output: "let foo = bar; let toast = most;", + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", @@ -58,6 +69,7 @@ ruleTester.run("no-var", rule, { { code: "for (var a of b) { console.log(a); }", output: "for (let a of b) { console.log(a); }", + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", @@ -68,6 +80,7 @@ ruleTester.run("no-var", rule, { { code: "for (var a in b) { console.log(a); }", output: "for (let a in b) { console.log(a); }", + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", @@ -78,6 +91,7 @@ ruleTester.run("no-var", rule, { { code: "for (let a of b) { var c = 1; console.log(c); }", output: "for (let a of b) { let c = 1; console.log(c); }", + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", @@ -88,6 +102,7 @@ ruleTester.run("no-var", rule, { { code: "for (var i = 0; i < list.length; ++i) { foo(i) }", output: "for (let i = 0; i < list.length; ++i) { foo(i) }", + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", type: "VariableDeclaration" } ] @@ -95,6 +110,7 @@ ruleTester.run("no-var", rule, { { code: "for (var i = 0, i = 0; false;);", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", type: "VariableDeclaration" } ] @@ -102,6 +118,7 @@ ruleTester.run("no-var", rule, { { code: "var i = 0; for (var i = 1; false;); console.log(i);", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", type: "VariableDeclaration" }, { message: "Unexpected var, use let or const instead.", type: "VariableDeclaration" } @@ -112,6 +129,7 @@ ruleTester.run("no-var", rule, { { code: "var a, b, c; var a;", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead.", "Unexpected var, use let or const instead." @@ -120,6 +138,7 @@ ruleTester.run("no-var", rule, { { code: "var a; if (b) { var a; }", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead.", "Unexpected var, use let or const instead." @@ -128,6 +147,7 @@ ruleTester.run("no-var", rule, { { code: "if (foo) { var a, b, c; } a;", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -135,6 +155,7 @@ ruleTester.run("no-var", rule, { { code: "for (var i = 0; i < 10; ++i) {} i;", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -142,6 +163,7 @@ ruleTester.run("no-var", rule, { { code: "for (var a in obj) {} a;", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -149,6 +171,7 @@ ruleTester.run("no-var", rule, { { code: "for (var a of list) {} a;", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -156,6 +179,7 @@ ruleTester.run("no-var", rule, { { code: "switch (a) { case 0: var b = 1 }", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -165,6 +189,7 @@ ruleTester.run("no-var", rule, { { code: "for (var a of b) { arr.push(() => a); }", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -172,6 +197,7 @@ ruleTester.run("no-var", rule, { { code: "for (let a of b) { var c; console.log(c); c = 'hello'; }", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -181,6 +207,7 @@ ruleTester.run("no-var", rule, { { code: "var a = a", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -188,7 +215,7 @@ ruleTester.run("no-var", rule, { { code: "var {a = a} = {}", output: null, - parserOptions: { ecmaVersion: 2015 }, + parserOptions: { ecmaVersion: 2015, ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -196,7 +223,7 @@ ruleTester.run("no-var", rule, { { code: "var {a = b, b} = {}", output: null, - parserOptions: { ecmaVersion: 2015 }, + parserOptions: { ecmaVersion: 2015, ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -204,7 +231,7 @@ ruleTester.run("no-var", rule, { { code: "var {a, b = a} = {}", output: "let {a, b = a} = {}", - parserOptions: { ecmaVersion: 2015 }, + parserOptions: { ecmaVersion: 2015, ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -212,7 +239,7 @@ ruleTester.run("no-var", rule, { { code: "var a = b, b = 1", output: null, - parserOptions: { ecmaVersion: 2015 }, + parserOptions: { ecmaVersion: 2015, ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -220,7 +247,7 @@ ruleTester.run("no-var", rule, { { code: "var a = b; var b = 1", output: "let a = b; var b = 1", - parserOptions: { ecmaVersion: 2015 }, + parserOptions: { ecmaVersion: 2015, ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead.", "Unexpected var, use let or const instead." @@ -232,7 +259,7 @@ ruleTester.run("no-var", rule, { { code: "function foo() { a } var a = 1; foo()", output: null, - parserOptions: { ecmaVersion: 2015 }, + parserOptions: { ecmaVersion: 2015, ecmaFeatures: { globalReturn: true } }, errors: [ "Unexpected var, use let or const instead." ] @@ -242,9 +269,33 @@ ruleTester.run("no-var", rule, { { code: "if (foo) var bar = 1;", output: null, + parserOptions: { ecmaFeatures: { globalReturn: true } }, errors: [ { message: "Unexpected var, use let or const instead.", type: "VariableDeclaration" } ] + }, + + // https://github.com/eslint/eslint/issues/9520 + { + code: "var foo = 1", + output: null, + errors: ["Unexpected var, use let or const instead."] + }, + { + code: "{ var foo = 1 }", + output: null, + errors: ["Unexpected var, use let or const instead."] + }, + { + code: "if (true) { var foo = 1 }", + output: null, + errors: ["Unexpected var, use let or const instead."] + }, + { + code: "var foo = 1", + output: "let foo = 1", + parserOptions: { sourceType: "module" }, + errors: ["Unexpected var, use let or const instead."] } ] });