Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: no-lexical-globals rule #11981

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 97 additions & 0 deletions 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)
1 change: 1 addition & 0 deletions lib/rules/index.js
Expand Up @@ -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"),
Expand Down
48 changes: 48 additions & 0 deletions 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
}
});
}
});
});
}
};
}
};
127 changes: 127 additions & 0 deletions 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." }
]
}
]
});
1 change: 1 addition & 0 deletions tools/rule-types.json
Expand Up @@ -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",
Expand Down