diff --git a/docs/rules/no-lexical-globals.md b/docs/rules/no-lexical-globals.md new file mode 100644 index 00000000000..b2e652f8856 --- /dev/null +++ b/docs/rules/no-lexical-globals.md @@ -0,0 +1,97 @@ +# Disallow `const`, `let` and `class` declarations in the global scope (no-lexical-globals) + +Lexical declarations `const` and `let`, as well as `class` declarations, create variables that are block-scoped. + +However, when declared in the top-level of a browser script these variables are not "script-scoped". +They are actually created in the global scope and could produce name collisions with +`var`, `const` and `let` variables and `function` and `class` declarations from other scripts. + +This does not apply to ES and Node.js modules since they have a module scope. + +It is the best practice to avoid polluting the global namespace with variables that are intended to be local to the script. +You can wrap the code with a block or with an immediately-invoked function expression (IIFE). + +Examples of **incorrect** code for this rule: + +```js +const a = 1; +let b; +class C {} +``` + +Examples of **correct** code for this rule: + +```js +{ + const a = 1; + let b; + class C {}; +} + +(function() { + const a = 1; + let b; + class C {}; +}()); +``` + +Even if you intend to create a global variable to be used from other scripts, there are some differences +compared to the common methods, which are `var` declarations and assigning to a property of the global `window` object: + +* Lexically declared variables cannot be conditionally created. A script cannot check for the existence of +a variable and then create a new one. `var` variables are also always created, but redeclarations do not +cause runtime exceptions. +* Lexically declared variables do not create properties on the global object, which is what a consuming script might expect. +* Lexically declared variables are shadowing properties of the global object, which might produce errors if a +consuming script is using both the variable and the property. +* Lexically declared variables can produce a permanent Temporal Dead Zone (TDZ) if the initialization throws an exception. +Even the `typeof` check is not safe from TDZ reference exceptions. + +Examples of **incorrect** code for this rule: + +```js +const MyGobalFunction = (function() { + const a = 1; + let b = 2; + return function() { + return a + b; + } +}()); +``` + +Examples of **correct** code for this rule: + +```js + +var MyGobalFunction = (function() { + const a = 1; + let b = 2; + return function() { + return a + b; + } +}()); + +window.MyGobalFunction = (function() { + const a = 1; + let b = 2; + return function() { + return a + b; + } +}()); +``` + +## When Not To Use It + +This rule does not report any warnings in ES modules and Node.js modules. + +If you have a browser script, turn this rule off if you want to allow global lexical declarations. + +## Further Reading + +* [const variables](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const) +* [let variables](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let) +* [Temporal Dead Zone](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#Temporal_dead_zone) + +## Related Rules + +* [no-implicit-globals](no-implicit-globals.md) diff --git a/lib/rules/index.js b/lib/rules/index.js index 45045904bb4..03040761248 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -138,6 +138,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({ "no-iterator": () => require("./no-iterator"), "no-label-var": () => require("./no-label-var"), "no-labels": () => require("./no-labels"), + "no-lexical-globals": () => require("./no-lexical-globals"), "no-lone-blocks": () => require("./no-lone-blocks"), "no-lonely-if": () => require("./no-lonely-if"), "no-loop-func": () => require("./no-loop-func"), diff --git a/lib/rules/no-lexical-globals.js b/lib/rules/no-lexical-globals.js new file mode 100644 index 00000000000..b3fbb3e3f92 --- /dev/null +++ b/lib/rules/no-lexical-globals.js @@ -0,0 +1,48 @@ +/** + * @fileoverview Rule to disallow const, let and class declarations in the global scope + * @author Milos Djermanovic + */ + +"use strict"; + +module.exports = { + meta: { + type: "suggestion", + + docs: { + description: "disallow `const`, `let` and `class` declarations in the global scope", + category: "Best Practices", + recommended: false, + url: "https://eslint.org/docs/rules/no-lexical-globals" + }, + + schema: [], + + messages: { + unexpected: "Unexpected '{{kind}}' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." + } + }, + + create(context) { + return { + Program() { + const scope = context.getScope(); + + scope.variables.forEach(variable => { + variable.defs.forEach(def => { + if (def.type === "ClassName" || + (def.type === "Variable" && (def.parent.kind === "let" || def.parent.kind === "const"))) { + context.report({ + node: def.node, + messageId: "unexpected", + data: { + kind: def.type === "ClassName" ? "class" : def.parent.kind + } + }); + } + }); + }); + } + }; + } +}; diff --git a/tests/lib/rules/no-lexical-globals.js b/tests/lib/rules/no-lexical-globals.js new file mode 100644 index 00000000000..612e7d3bb49 --- /dev/null +++ b/tests/lib/rules/no-lexical-globals.js @@ -0,0 +1,127 @@ +/** + * @fileoverview Tests for no-lexical-globals rule. + * @author Milos Djermanovic + */ + +"use strict"; + +const rule = require("../../../lib/rules/no-lexical-globals"); +const { RuleTester } = require("../../../lib/rule-tester"); + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2015 + } +}); + +ruleTester.run("no-lexical-globals", rule, { + valid: [ + + // not a responsibility of this rule + "a = 1;", + "var a = 1;", + "function a(){}", + "function *a(){}", + "/* global a:readonly */", + "/* global a:writable */", + + // class expressions + "var foo = class {}", + "var foo = class A {}", + "typeof class A {}", + + // not in the global scope + "{ const a = 1; let b; class C {} }", + "function foo() { const a = 1; let b; class C {} }", + + // different scoping + { + code: "const a = 1; let b; class C {}", + parserOptions: { sourceType: "module" } + }, + { + code: "const a = 1; let b; class C {}", + env: { node: true } + }, + { + code: "const a = 1; let b; class C {}", + env: { commonjs: true } + }, + { + code: "const a = 1; let b; class C {}", + parserOptions: { ecmaFeatures: { globalReturn: true } } + } + + ], + invalid: [ + { + code: "const a = 1;", + errors: [{ + message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." + }] + }, + { + code: "let a;", + errors: [{ + message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." + }] + }, + { + code: "let a = 1;", + errors: [{ + message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." + }] + }, + { + code: "class A {}", + errors: [{ + message: "Unexpected 'class' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." + }] + }, + { + code: "const a = 1; const b = 2;", + errors: [ + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." } + ] + }, + { + code: "const a = 1, b = 2;", + errors: [ + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." } + ] + }, + { + code: "let a, b = 1;", + errors: [ + { message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." } + ] + }, + { + code: "const a = 1; let b; class C {}", + errors: [ + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'class' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." } + ] + }, + { + code: "const [a, b, ...c] = [];", + errors: [ + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'const' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." } + ] + }, + { + code: "let { a, foo: b, bar: { c } } = {};", + errors: [ + { message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." }, + { message: "Unexpected 'let' in the global scope, wrap for a local variable, assign to a global property or use var for a global variable." } + ] + } + ] +}); diff --git a/tools/rule-types.json b/tools/rule-types.json index 1239e5f3ffa..578c6867c60 100644 --- a/tools/rule-types.json +++ b/tools/rule-types.json @@ -125,6 +125,7 @@ "no-iterator": "suggestion", "no-label-var": "suggestion", "no-labels": "suggestion", + "no-lexical-globals": "suggestion", "no-lone-blocks": "suggestion", "no-lonely-if": "suggestion", "no-loop-func": "suggestion",