Skip to content

Commit

Permalink
New: no-global-assign rule (fixes #6585)
Browse files Browse the repository at this point in the history
  • Loading branch information
alberto committed Jul 22, 2016
1 parent 5320a6c commit 2c9eaef
Show file tree
Hide file tree
Showing 10 changed files with 243 additions and 4 deletions.
1 change: 1 addition & 0 deletions conf/eslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"no-fallthrough": "error",
"no-floating-decimal": "off",
"no-func-assign": "error",
"no-global-assign": "off",
"no-implicit-coercion": "off",
"no-implicit-globals": "off",
"no-implied-eval": "off",
Expand Down
4 changes: 3 additions & 1 deletion docs/rules/no-extend-native.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Disallow Extending of Native Objects (no-extend-native)

This rule was **deprecated** in ESLint v3.2.0 and replaced by the [no-global-assign](no-global-assign.md) rule.

In JavaScript, you can extend any object, including builtin or "native" objects. Sometimes people change the behavior of these native objects in ways that break the assumptions made about them in other parts of the code.

For example here we are overriding a builtin method that will then affect all Objects, even other builtins.
Expand Down Expand Up @@ -72,4 +74,4 @@ You may want to disable this rule when working with polyfills that try to patch

## Related Rules

* [no-native-reassign](no-native-reassign.md)
* [no-global-assign](no-global-assign.md)
89 changes: 89 additions & 0 deletions docs/rules/no-global-assign.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Disallow assignment to native objects or read-only global variables (no-global-assign)

JavaScript environments contain a number of built-in global variables, such as `window` in browsers and `process` in Node.js. In almost all cases, you don't want to assign a value to these global variables as doing so could result in losing access to important functionality. For example, you probably don't want to do this in browser code:

```js
window = {};
```

While examples such as `window` are obvious, there are often hundreds of built-in global objects provided by JavaScript environments. It can be hard to know if you're assigning to a global variable or not.

## Rule Details

This rule disallows modifications to read-only global variables.

ESLint has the capability to configure global variables as read-only.

* [Specifying Environments](../user-guide/configuring#specifying-environments)
* [Specifying Globals](../user-guide/configuring#specifying-globals)

Examples of **incorrect** code for this rule:

```js
/*eslint no-global-assign: "error"*/

Object = null
undefined = 1
```

```js
/*eslint no-global-assign: "error"*/
/*eslint-env browser*/

window = {}
length = 1
top = 1
```

```js
/*eslint no-global-assign: "error"*/
/*globals a:false*/

a = 1
```

Examples of **correct** code for this rule:

```js
/*eslint no-global-assign: "error"*/

a = 1
var b = 1
b = 2
```

```js
/*eslint no-global-assign: "error"*/
/*eslint-env browser*/

onload = function() {}
```

```js
/*eslint no-global-assign: "error"*/
/*globals a:true*/

a = 1
```

## Options

This rule accepts an `exceptions` option, which can be used to specify a list of builtins for which reassignments will be allowed:

```json
{
"rules": {
"no-global-assign": ["error", {"exceptions": ["Object"]}]
}
}
```

## When Not To Use It

If you are trying to override one of the native objects.

## Related Rules

* [no-extend-native](no-extend-native.md)
* [no-redeclare](no-redeclare.md)
* [no-shadow](no-shadow.md)
2 changes: 1 addition & 1 deletion docs/user-guide/configuring.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ And in YAML:

These examples allow `var1` to be overwritten in your code, but disallow it for `var2`.

**Note:** Enable the [no-native-reassign](../rules/no-native-reassign.md) rule to disallow modifications to read-only global variables in your code.
**Note:** Enable the [no-global-assign](../rules/no-global-assign.md) rule to disallow modifications to read-only global variables in your code.

## Configuring Plugins

Expand Down
1 change: 1 addition & 0 deletions lib/rules/no-extend-native.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @fileoverview Rule to flag adding properties to native object's prototypes.
* @author David Nelson
* @deprecated in ESLint v3.2.0
*/

"use strict";
Expand Down
83 changes: 83 additions & 0 deletions lib/rules/no-global-assign.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* @fileoverview Rule to disallow assignments to native objects or read-only global variables
* @author Ilya Volodin
*/

"use strict";

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: "disallow assignments to native objects or read-only global variables",
category: "Best Practices",
recommended: true
},

schema: [
{
type: "object",
properties: {
exceptions: {
type: "array",
items: {type: "string"},
uniqueItems: true
}
},
additionalProperties: false
}
]
},

create: function(context) {
var config = context.options[0];
var exceptions = (config && config.exceptions) || [];

/**
* Reports write references.
* @param {Reference} reference - A reference to check.
* @param {int} index - The index of the reference in the references.
* @param {Reference[]} references - The array that the reference belongs to.
* @returns {void}
*/
function checkReference(reference, index, references) {
var identifier = reference.identifier;

if (reference.init === false &&
reference.isWrite() &&

// Destructuring assignments can have multiple default value,
// so possibly there are multiple writeable references for the same identifier.
(index === 0 || references[index - 1].identifier !== identifier)
) {
context.report({
node: identifier,
message: "Read-only global '{{name}}' should not be modified.",
data: identifier
});
}
}

/**
* Reports write references if a given variable is read-only builtin.
* @param {Variable} variable - A variable to check.
* @returns {void}
*/
function checkVariable(variable) {
if (variable.writeable === false && exceptions.indexOf(variable.name) === -1) {
variable.references.forEach(checkReference);
}
}

return {
Program: function() {
var globalScope = context.getScope();

globalScope.variables.forEach(checkVariable);
}
};
}
};
3 changes: 2 additions & 1 deletion packages/eslint-config-eslint/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ rules:
no-extra-bind: "error"
no-fallthrough: "error"
no-floating-decimal: "error"
no-global-assign: "error"
no-implied-eval: "error"
no-invalid-this: "error"
no-iterator: "error"
Expand All @@ -49,7 +50,7 @@ rules:
no-mixed-spaces-and-tabs: ["error", false]
no-multi-spaces: "error"
no-multi-str: "error"
no-native-reassign: "error"
no-native-reassign: "off"
no-nested-ternary: "error"
no-new: "error"
no-new-func: "error"
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ describe("cli", function() {
describe("when executing with global flag", function() {
it("should default defined variables to read-only", function() {
var filePath = getFixturePath("undef.js");
var exit = cli.execute("--global baz,bat --no-ignore --rule no-native-reassign:2 " + filePath);
var exit = cli.execute("--global baz,bat --no-ignore --rule no-global-assign:2 " + filePath);

assert.isTrue(log.info.calledOnce);
assert.equal(exit, 1);
Expand Down
1 change: 1 addition & 0 deletions tests/lib/rules/no-extend-native.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @fileoverview Tests for no-extend-native rule.
* @author David Nelson
* @deprecated in ESLint v3.2.0
*/

"use strict";
Expand Down
61 changes: 61 additions & 0 deletions tests/lib/rules/no-global-assign.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* @fileoverview Tests for no-global-assign rule.
* @author Ilya Volodin
*/

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

var rule = require("../../../lib/rules/no-global-assign"),
RuleTester = require("../../../lib/testers/rule-tester");

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

var ruleTester = new RuleTester();

ruleTester.run("no-global-assign", rule, {
valid: [
"string = 'hello world';",
"var string;",
{ code: "Object = 0;", options: [{exceptions: ["Object"]}] },
{ code: "top = 0;" },
{ code: "onload = 0;", env: {browser: true} },
{ code: "require = 0;" },
{ code: "a = 1", globals: {a: true}},
"/*global a:true*/ a = 1"
],
invalid: [
{ code: "String = 'hello world';", errors: [{ message: "Read-only global 'String' should not be modified.", type: "Identifier"}] },
{ code: "String++;", errors: [{ message: "Read-only global 'String' should not be modified.", type: "Identifier"}] },
{
code: "({Object = 0, String = 0} = {});",
parserOptions: { ecmaVersion: 6 },
errors: [
{message: "Read-only global 'Object' should not be modified.", type: "Identifier"},
{message: "Read-only global 'String' should not be modified.", type: "Identifier"}
]
},
{
code: "top = 0;",
env: {browser: true},
errors: [{ message: "Read-only global 'top' should not be modified.", type: "Identifier"}]
},
{
code: "require = 0;",
env: {node: true},
errors: [{ message: "Read-only global 'require' should not be modified.", type: "Identifier"}]
},

// Notifications of readonly are moved from no-undef: https://github.com/eslint/eslint/issues/4504
{ code: "/*global b:false*/ function f() { b = 1; }", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier"}] },
{ code: "function f() { b = 1; }", global: { b: false }, errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier"}] },
{ code: "/*global b:false*/ function f() { b++; }", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier"}] },
{ code: "/*global b*/ b = 1;", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier"}] },
{ code: "Array = 1;", errors: [{ message: "Read-only global 'Array' should not be modified.", type: "Identifier"}] }
]
});

0 comments on commit 2c9eaef

Please sign in to comment.