New: Allow globals to be disabled/configured with strings (fixes #9940)

docs/user-guide/configuring.md

@@ -208,19 +208,19 @@ To specify globals using a comment inside of your JavaScript file, use the follo
 /* global var1, var2 */
 ```
 
-This defines two global variables, `var1` and `var2`. If you want to optionally specify that these global variables should never be written to (only read), then you can set each with a `false` flag:
+This defines two global variables, `var1` and `var2`. If you want to optionally specify that these global variables can be written to (rather than only being read), then you can set each with a `writeable` flag:
 
 ```js
-/* global var1:false, var2:false */
+/* global var1:writeable, var2:writeable */
 ```
 
-To configure global variables inside of a configuration file, use the `globals` key and indicate the global variables you want to use. Set each global variable name equal to `true` to allow the variable to be overwritten or `false` to disallow overwriting. For example:
+To configure global variables inside of a configuration file, set the `globals` configuration property to an object containing keys named for each of the global variables you want to use. For each global variable key, set the corresponding value equal to `"writeable"` to allow the variable to be overwritten or `"readable"` to disallow overwriting. For example:
 
 ```json
 {
     "globals": {
-        "var1": true,
-        "var2": false
+        "var1": "writeable",
+        "var2": "readable"
     }
 }
 ```
@@ -230,12 +230,27 @@ And in YAML:
 ```yaml
 ---
   globals:
-    var1: true
-    var2: false
+    var1: writeable
+    var2: readable
 ```
 
 These examples allow `var1` to be overwritten in your code, but disallow it for `var2`.
 
+Globals can be disabled with the string `"off"`. For example, in an environment where most ES2015 globals are available but `Promise` is unavailable, you might use this config:
+
+```json
+{
+    "env": {
+        "es6": true
+    },
+    "globals": {
+        "Promise": "off"
+    }
+}
+```
+
+For historical reasons, the boolean values `false` and `true` can also be used to configure globals, and are equivalent to `"readable"` and `"writeable"`, respectively. However, this usage of booleans is deprecated.
+
 **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/config/config-ops.js

@@ -370,5 +370,33 @@ module.exports = {
 
         return patternList.some(pattern => minimatch(filePath, pattern, opts)) &&
             !excludedPatternList.some(excludedPattern => minimatch(filePath, excludedPattern, opts));
+    },
+
+    /**
+     * Normalizes a value for a global in a config
+     * @param {(boolean|string|null)} configuredValue The value given for a global in configuration or in
+     * a global directive comment
+     * @returns {("readable"|"writeable"|"off")} The value normalized as a string
+     */
+    normalizeConfigGlobal(configuredValue) {
+        switch (configuredValue) {
+            case "off":
+                return "off";
+
+            case true:
+            case "true":
+            case "writeable":
+                return "writeable";
+
+            case null:
+            case false:
+            case "false":
+            case "readable":
+                return "readable";
+
+            // Fallback to minimize compatibility impact
+            default:
+                return "writeable";
+        }
     }
 };

lib/linter.js

@@ -70,30 +70,30 @@ const commentParser = new ConfigCommentParser();
  * @returns {void}
  */
 function addDeclaredGlobals(globalScope, configGlobals, commentDirectives) {
-    Object.keys(configGlobals).forEach(name => {
-        let variable = globalScope.set.get(name);
-
-        if (!variable) {
-            variable = new eslintScope.Variable(name, globalScope);
-            variable.eslintExplicitGlobal = false;
-            globalScope.variables.push(variable);
-            globalScope.set.set(name, variable);
-        }
-        variable.writeable = configGlobals[name];
-    });
-
-    Object.keys(commentDirectives.enabledGlobals).forEach(name => {
-        let variable = globalScope.set.get(name);
+    const mergedGlobalsInfo = Object.assign(
+        {},
+        lodash.mapValues(configGlobals, value => ({ sourceComment: null, value: ConfigOps.normalizeConfigGlobal(value) })),
+        lodash.mapValues(commentDirectives.enabledGlobals, ({ comment, value }) => ({ sourceComment: comment, value: ConfigOps.normalizeConfigGlobal(value) }))
+    );
 
-        if (!variable) {
-            variable = new eslintScope.Variable(name, globalScope);
-            variable.eslintExplicitGlobal = true;
-            variable.eslintExplicitGlobalComment = commentDirectives.enabledGlobals[name].comment;
-            globalScope.variables.push(variable);
-            globalScope.set.set(name, variable);
-        }
-        variable.writeable = commentDirectives.enabledGlobals[name].value;
-    });
+    Object.keys(mergedGlobalsInfo)
+        .filter(name => mergedGlobalsInfo[name].value !== "off")
+        .forEach(name => {
+            let variable = globalScope.set.get(name);
+
+            if (!variable) {
+                variable = new eslintScope.Variable(name, globalScope);
+                if (mergedGlobalsInfo[name].sourceComment === null) {
+                    variable.eslintExplicitGlobal = false;
+                } else {
+                    variable.eslintExplicitGlobal = true;
+                    variable.eslintExplicitGlobalComment = mergedGlobalsInfo[name].sourceComment;
+                }
+                globalScope.variables.push(variable);
+                globalScope.set.set(name, variable);
+            }
+            variable.writeable = (mergedGlobalsInfo[name].value === "writeable");
+        });
 
     // mark all exported variables as such
     Object.keys(commentDirectives.exportedVariables).forEach(name => {
@@ -191,12 +191,12 @@ function getDirectiveComments(filename, ast, ruleMapper) {
         } else if (comment.type === "Block") {
             switch (match[1]) {
                 case "exported":
-                    Object.assign(exportedVariables, commentParser.parseBooleanConfig(directiveValue, comment));
+                    Object.assign(exportedVariables, commentParser.parseStringConfig(directiveValue, comment));
                     break;
 
                 case "globals":
                 case "global":
-                    Object.assign(enabledGlobals, commentParser.parseBooleanConfig(directiveValue, comment));
+                    Object.assign(enabledGlobals, commentParser.parseStringConfig(directiveValue, comment));
                     break;
 
                 case "eslint-disable":

lib/util/config-comment-parser.js

@@ -26,14 +26,14 @@ const debug = require("debug")("eslint:config-comment-parser");
 module.exports = class ConfigCommentParser {
 
     /**
-     * Parses a list of "name:boolean_value" or/and "name" options divided by comma or
+     * Parses a list of "name:string_value" or/and "name" options divided by comma or
      * whitespace. Used for "global" and "exported" comments.
      * @param {string} string The string to parse.
      * @param {Comment} comment The comment node which has the string.
-     * @returns {Object} Result map object of names and boolean values
+     * @returns {Object} Result map object of names and string values, or null values if no value was provided
      */
-    parseBooleanConfig(string, comment) {
-        debug("Parsing Boolean config");
+    parseStringConfig(string, comment) {
+        debug("Parsing String config");
 
         const items = {};
 
@@ -45,13 +45,10 @@ module.exports = class ConfigCommentParser {
                 return;
             }
 
-            // value defaults to "false" (if not provided), e.g: "foo" => ["foo", "false"]
-            const [key, value = "false"] = name.split(":");
+            // value defaults to null (if not provided), e.g: "foo" => ["foo", null]
+            const [key, value = null] = name.split(":");
 
-            items[key] = {
-                value: value === "true",
-                comment
-            };
+            items[key] = { value, comment };
         });
         return items;
     }

tests/lib/config/config-ops.js

@@ -898,4 +898,23 @@ describe("ConfigOps", () => {
         error("foo.js", ["/foo.js"], "Invalid override pattern (expected relative path not containing '..'): /foo.js");
         error("foo.js", ["../**"], "Invalid override pattern (expected relative path not containing '..'): ../**");
     });
+
+    describe("normalizeConfigGlobal", () => {
+        [
+            ["off", "off"],
+            [true, "writeable"],
+            ["true", "writeable"],
+            [false, "readable"],
+            ["false", "readable"],
+            [null, "readable"],
+            ["writeable", "writeable"],
+            ["readable", "readable"],
+            ["writable", "writeable"],
+            ["something else", "writeable"]
+        ].forEach(([input, output]) => {
+            it(util.inspect(input), () => {
+                assert.strictEqual(ConfigOps.normalizeConfigGlobal(input), output);
+            });
+        });
+    });
 });

tests/lib/linter.js

@@ -1124,10 +1124,15 @@ describe("Linter", () => {
     });
 
     describe("when evaluating code containing /*global */ and /*globals */ blocks", () => {
-        const code = "/*global a b:true c:false*/ function foo() {} /*globals d:true*/";
 
         it("variables should be available in global scope", () => {
-            const config = { rules: { checker: "error" } };
+            const config = { rules: { checker: "error" }, globals: { Array: "off", ConfigGlobal: "writeable" } };
+            const code = `
+                /*global a b:true c:false d:readable e:writeable Math:off */
+                function foo() {}
+                /*globals f:true*/
+                /* global ConfigGlobal : readable */
+            `;
             let spy;
 
             linter.defineRule("checker", context => {
@@ -1136,7 +1141,12 @@ describe("Linter", () => {
                     const a = getVariable(scope, "a"),
                         b = getVariable(scope, "b"),
                         c = getVariable(scope, "c"),
-                        d = getVariable(scope, "d");
+                        d = getVariable(scope, "d"),
+                        e = getVariable(scope, "e"),
+                        f = getVariable(scope, "f"),
+                        mathGlobal = getVariable(scope, "Math"),
+                        arrayGlobal = getVariable(scope, "Array"),
+                        configGlobal = getVariable(scope, "ConfigGlobal");
 
                     assert.strictEqual(a.name, "a");
                     assert.strictEqual(a.writeable, false);
@@ -1145,7 +1155,15 @@ describe("Linter", () => {
                     assert.strictEqual(c.name, "c");
                     assert.strictEqual(c.writeable, false);
                     assert.strictEqual(d.name, "d");
-                    assert.strictEqual(d.writeable, true);
+                    assert.strictEqual(d.writeable, false);
+                    assert.strictEqual(e.name, "e");
+                    assert.strictEqual(e.writeable, true);
+                    assert.strictEqual(f.name, "f");
+                    assert.strictEqual(f.writeable, true);
+                    assert.strictEqual(mathGlobal, null);
+                    assert.strictEqual(arrayGlobal, null);
+                    assert.strictEqual(configGlobal.name, "ConfigGlobal");
+                    assert.strictEqual(configGlobal.writeable, false);
                 });
 
                 return { Program: spy };
@@ -1454,6 +1472,26 @@ describe("Linter", () => {
             linter.verify(code, config);
             assert(spy && spy.calledOnce);
         });
+
+        it("ES6 global variables can be disabled when the es6 environment is enabled", () => {
+            const config = { rules: { checker: "error" }, globals: { Promise: "off", Symbol: "off", WeakMap: "off" }, env: { es6: true } };
+            let spy;
+
+            linter.defineRule("checker", context => {
+                spy = sandbox.spy(() => {
+                    const scope = context.getScope();
+
+                    assert.strictEqual(getVariable(scope, "Promise"), null);
+                    assert.strictEqual(getVariable(scope, "Symbol"), null);
+                    assert.strictEqual(getVariable(scope, "WeakMap"), null);
+                });
+
+                return { Program: spy };
+            });
+
+            linter.verify(code, config);
+            assert(spy && spy.calledOnce);
+        });
     });
 
     describe("at any time", () => {

tests/lib/util/config-comment-parser.js

@@ -117,77 +117,77 @@ describe("ConfigCommentParser", () => {
 
     });
 
-    describe("parseBooleanConfig", () => {
+    describe("parseStringConfig", () => {
 
         const comment = {};
 
-        it("should parse Boolean config with one item", () => {
+        it("should parse String config with one item", () => {
             const code = "a: true";
-            const result = commentParser.parseBooleanConfig(code, comment);
+            const result = commentParser.parseStringConfig(code, comment);
 
             assert.deepStrictEqual(result, {
                 a: {
-                    value: true,
+                    value: "true",
                     comment
                 }
             });
         });
 
-        it("should parse Boolean config with one item and no value", () => {
+        it("should parse String config with one item and no value", () => {
             const code = "a";
-            const result = commentParser.parseBooleanConfig(code, comment);
+            const result = commentParser.parseStringConfig(code, comment);
 
             assert.deepStrictEqual(result, {
                 a: {
-                    value: false,
+                    value: null,
                     comment
                 }
             });
         });
 
-        it("should parse Boolean config with two items", () => {
-            const code = "a: true b:false";
-            const result = commentParser.parseBooleanConfig(code, comment);
+        it("should parse String config with two items", () => {
+            const code = "a: five b:three";
+            const result = commentParser.parseStringConfig(code, comment);
 
             assert.deepStrictEqual(result, {
                 a: {
-                    value: true,
+                    value: "five",
                     comment
                 },
                 b: {
-                    value: false,
+                    value: "three",
                     comment
                 }
             });
         });
 
-        it("should parse Boolean config with two comma-separated items", () => {
-            const code = "a: true, b:false";
-            const result = commentParser.parseBooleanConfig(code, comment);
+        it("should parse String config with two comma-separated items", () => {
+            const code = "a: seventy, b:ELEVENTEEN";
+            const result = commentParser.parseStringConfig(code, comment);
 
             assert.deepStrictEqual(result, {
                 a: {
-                    value: true,
+                    value: "seventy",
                     comment
                 },
                 b: {
-                    value: false,
+                    value: "ELEVENTEEN",
                     comment
                 }
             });
         });
 
-        it("should parse Boolean config with two comma-separated items and no values", () => {
+        it("should parse String config with two comma-separated items and no values", () => {
             const code = "a , b";
-            const result = commentParser.parseBooleanConfig(code, comment);
+            const result = commentParser.parseStringConfig(code, comment);
 
             assert.deepStrictEqual(result, {
                 a: {
-                    value: false,
+                    value: null,
                     comment
                 },
                 b: {
-                    value: false,
+                    value: null,
                     comment
                 }
             });