New: no-global-assign rule (fixes #6586) (#6746)

Makefile.js

@@ -208,21 +208,28 @@ function generateRuleIndexPage(basedir) {
 
     find(path.join(basedir, "/lib/rules/")).filter(fileType("js")).forEach(function(filename) {
         let rule = require(filename);
+        let basename = path.basename(filename, ".js");
 
-        let basename = path.basename(filename, ".js"),
-            output = {
+        if (rule.meta.deprecated) {
+            categoriesData.deprecated.rules.push({
                 name: basename,
-                description: rule.meta.docs.description,
-                recommended: rule.meta.docs.recommended || false,
-                fixable: !!rule.meta.fixable
-            },
-            category = lodash.find(categoriesData.categories, {name: rule.meta.docs.category});
+                replacedBy: rule.meta.docs.replacedBy
+            });
+        } else {
+            let output = {
+                    name: basename,
+                    description: rule.meta.docs.description,
+                    recommended: rule.meta.docs.recommended || false,
+                    fixable: !!rule.meta.fixable
+                },
+                category = lodash.find(categoriesData.categories, {name: rule.meta.docs.category});
+
+            if (!category.rules) {
+                category.rules = [];
+            }
 
-        if (!category.rules) {
-            category.rules = [];
+            category.rules.push(output);
         }
-
-        category.rules.push(output);
     });
 
     let output = yaml.safeDump(categoriesData, {sortKeys: true});

conf/category-list.json

@@ -8,6 +8,11 @@
         { "name": "Stylistic Issues", "description": "These rules relate to style guidelines, and are therefore quite subjective:" },
         { "name": "ECMAScript 6", "description": "These rules relate to ES6, also known as ES2015:" }
     ],
+    "deprecated": {
+        "name": "Deprecated",
+        "description": "These rules have been deprecated and replaced by newer rules:",
+        "rules": []
+    },
     "removed": {
         "name": "Removed",
         "description": "These rules from older versions of ESLint have been replaced by newer rules:",
@@ -32,4 +37,4 @@
             { "removed": "spaced-line-comment", "replacedBy": ["spaced-comment"] }
         ]
     }
-}
+}

conf/eslint.json

@@ -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",

docs/rules/no-extend-native.md

@@ -72,4 +72,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)

docs/rules/no-global-assign.md

@@ -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)

docs/rules/no-native-reassign.md

@@ -1,5 +1,7 @@
 # Disallow Reassignment of Native Objects (no-native-reassign)
 
+This rule was **deprecated** in ESLint v3.3.0 and replaced by the [no-global-assign](no-global-assign.md) rule.
+
 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

docs/user-guide/configuring.md

@@ -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
 

lib/rules/no-global-assign.js

@@ -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: false
+        },
+
+        schema: [
+            {
+                type: "object",
+                properties: {
+                    exceptions: {
+                        type: "array",
+                        items: {type: "string"},
+                        uniqueItems: true
+                    }
+                },
+                additionalProperties: false
+            }
+        ]
+    },
+
+    create: function(context) {
+        let config = context.options[0];
+        let 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) {
+            let 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() {
+                let globalScope = context.getScope();
+
+                globalScope.variables.forEach(checkVariable);
+            }
+        };
+    }
+};

lib/rules/no-native-reassign.js

@@ -1,6 +1,7 @@
 /**
  * @fileoverview Rule to disallow assignments to native objects or read-only global variables
  * @author Ilya Volodin
+ * @deprecated in ESLint v3.3.0
  */
 
 "use strict";
@@ -14,9 +15,12 @@ module.exports = {
         docs: {
             description: "disallow assignments to native objects or read-only global variables",
             category: "Best Practices",
-            recommended: true
+            recommended: true,
+            replacedBy: ["no-global-assign"]
         },
 
+        deprecated: true,
+
         schema: [
             {
                 type: "object",

packages/eslint-config-eslint/default.yml

@@ -44,6 +44,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"
@@ -54,7 +55,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"

tests/lib/cli.js

@@ -472,7 +472,7 @@ describe("cli", function() {
     describe("when executing with global flag", function() {
         it("should default defined variables to read-only", function() {
             let filePath = getFixturePath("undef.js");
-            let exit = cli.execute("--global baz,bat --no-ignore --rule no-native-reassign:2 " + filePath);
+            let exit = cli.execute("--global baz,bat --no-ignore --rule no-global-assign:2 " + filePath);
 
             assert.isTrue(log.info.calledOnce);
             assert.equal(exit, 1);

tests/lib/rules/no-global-assign.js

@@ -0,0 +1,61 @@
+/**
+ * @fileoverview Tests for no-global-assign rule.
+ * @author Ilya Volodin
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+let rule = require("../../../lib/rules/no-global-assign"),
+    RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+let 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"}] }
+    ]
+});

tests/lib/rules/no-native-reassign.js

@@ -1,6 +1,7 @@
 /**
  * @fileoverview Tests for no-native-reassign rule.
  * @author Ilya Volodin
+ * @deprecated in ESLint v3.3.0
  */
 
 "use strict";