Repository: eslint/eslint
Pull request #12709 (2019-12-25 01:15:35)

Chore: fix separateRequires tests for one-var rule
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[X] Other, please explain:

Some tests for the `separateRequires` option of the `one-var` rule had this:

```js
parserOptions: { env: { node: true } }
```

These are incorrect settings because `env` shouldn't be in `parserOptions`. It's just ignored as an unknown parser option.

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

Since the tests are passing without the `node` environment being actually set, I wasn't sure if the rule was working as intended. In particular, should the option apply to _global_ `require` or any `require` call.

By #9441, in particular [this thread](https://github.com/eslint/eslint/pull/9441#pullrequestreview-76335806) and [this comment](https://github.com/eslint/eslint/pull/9441#discussion_r158590263) it seems that the rule wasn't supposed to check if the `require` reference is global, so instead of fixing `env` I just removed these.

**Is there anything you'd like reviewers to focus on?**

One test became a duplicate, so it's completely removed.





Patch:
diff --git a/docs/developer-guide/working-with-rules.md b/docs/developer-guide/working-with-rules.md
index deac450b0..608a38be7 100644
--- a/docs/developer-guide/working-with-rules.md
+++ b/docs/developer-guide/working-with-rules.md
@@ -142,7 +142,7 @@ Additionally, the `context` object has the following methods:
 * `getScope()` - returns the [scope](./scope-manager-interface.md#scope-interface) of the currently-traversed node. This information can be used to track references to variables.
 * `getSourceCode()` - returns a [`SourceCode`](#contextgetsourcecode) object that you can use to work with the source that was passed to ESLint.
 * `markVariableAsUsed(name)` - marks a variable with the given name in the current scope as used. This affects the [no-unused-vars](../rules/no-unused-vars.md) rule. Returns `true` if a variable with the given name was found and marked as used, otherwise `false`.
-* `report(descriptor)` - reports a problem in the code (see the [dedicated section](#contextreport)).
+* `report(descriptor)` - reports a problem in the code (see the [dedicated section](#context-report)).
 
 **Note:** Earlier versions of ESLint supported additional methods on the `context` object. Those methods were removed in the new format and should not be relied upon.
 
diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js
index 01c6b47b8..b4e8e2980 100644
--- a/lib/rules/utils/ast-utils.js
+++ b/lib/rules/utils/ast-utils.js
@@ -865,6 +865,44 @@ module.exports = {
         return isFunction(node) && module.exports.isEmptyBlock(node.body);
     },
 
+    /**
+     * Returns the result of the string conversion applied to the evaluated value of the given expression node,
+     * if it can be determined statically.
+     *
+     * This function returns a `string` value for all `Literal` nodes and simple `TemplateLiteral` nodes only.
+     * In all other cases, this function returns `null`.
+     * @param {ASTNode} node Expression node.
+     * @returns {string|null} String value if it can be determined. Otherwise, `null`.
+     */
+    getStaticStringValue(node) {
+        switch (node.type) {
+            case "Literal":
+                if (node.value === null) {
+                    if (module.exports.isNullLiteral(node)) {
+                        return String(node.value); // "null"
+                    }
+                    if (node.regex) {
+                        return `/${node.regex.pattern}/${node.regex.flags}`;
+                    }
+
+                    // Otherwise, this is an unknown literal. The function will return null.
+
+                } else {
+                    return String(node.value);
+                }
+                break;
+            case "TemplateLiteral":
+                if (node.expressions.length === 0 && node.quasis.length === 1) {
+                    return node.quasis[0].value.cooked;
+                }
+                break;
+
+            // no default
+        }
+
+        return null;
+    },
+
     /**
      * Gets the property name of a given node.
      * The node can be a MemberExpression, a Property, or a MethodDefinition.
@@ -911,23 +949,12 @@ module.exports = {
             // no default
         }
 
-        switch (prop && prop.type) {
-            case "Literal":
-                return String(prop.value);
-
-            case "TemplateLiteral":
-                if (prop.expressions.length === 0 && prop.quasis.length === 1) {
-                    return prop.quasis[0].value.cooked;
-                }
-                break;
-
-            case "Identifier":
-                if (!node.computed) {
-                    return prop.name;
-                }
-                break;
+        if (prop) {
+            if (prop.type === "Identifier" && !node.computed) {
+                return prop.name;
+            }
 
-            // no default
+            return module.exports.getStaticStringValue(prop);
         }
 
         return null;


Test patch:
diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js
index 44e01dadf..4b8c38692 100644
--- a/lib/rule-tester/rule-tester.js
+++ b/lib/rule-tester/rule-tester.js
@@ -50,6 +50,52 @@ const
 
 const ajv = require("../shared/ajv")({ strictDefaults: true });
 
+
+//------------------------------------------------------------------------------
+// Typedefs
+//------------------------------------------------------------------------------
+
+/**
+ * A test case that is expected to pass lint.
+ * @typedef {Object} ValidTestCase
+ * @property {string} code Code for the test case.
+ * @property {any[]} [options] Options for the test case.
+ * @property {{ [name: string]: any }} [settings] Settings for the test case.
+ * @property {string} [filename] The fake filename for the test case. Useful for rules that make assertion about filenames.
+ * @property {string} [parser] The absolute path for the parser.
+ * @property {{ [name: string]: any }} [parserOptions] Options for the parser.
+ * @property {{ [name: string]: "readonly" | "writable" | "off" }} [globals] The additional global variables.
+ * @property {{ [name: string]: boolean }} [env] Environments for the test case.
+ */
+
+/**
+ * A test case that is expected to fail lint.
+ * @typedef {Object} InvalidTestCase
+ * @property {string} code Code for the test case.
+ * @property {number | Array<TestCaseError | string | RegExp>} errors Expected errors.
+ * @property {string | null} [output] The expected code after autofixes are applied. If set to `null`, the test runner will assert that no autofix is suggested.
+ * @property {any[]} [options] Options for the test case.
+ * @property {{ [name: string]: any }} [settings] Settings for the test case.
+ * @property {string} [filename] The fake filename for the test case. Useful for rules that make assertion about filenames.
+ * @property {string} [parser] The absolute path for the parser.
+ * @property {{ [name: string]: any }} [parserOptions] Options for the parser.
+ * @property {{ [name: string]: "readonly" | "writable" | "off" }} [globals] The additional global variables.
+ * @property {{ [name: string]: boolean }} [env] Environments for the test case.
+ */
+
+/**
+ * A description of a reported error used in a rule tester test.
+ * @typedef {Object} TestCaseError
+ * @property {string | RegExp} [message] Message.
+ * @property {string} [messageId] Message ID.
+ * @property {string} [type] The type of the reported AST node.
+ * @property {{ [name: string]: string }} [data] The data used to fill the message template.
+ * @property {number} [line] The 1-based line number of the reported start location.
+ * @property {number} [column] The 1-based column number of the reported start location.
+ * @property {number} [endLine] The 1-based line number of the reported end location.
+ * @property {number} [endColumn] The 1-based column number of the reported end location.
+ */
+
 //------------------------------------------------------------------------------
 // Private Members
 //------------------------------------------------------------------------------
@@ -273,7 +319,10 @@ class RuleTester {
      * Adds a new rule test to execute.
      * @param {string} ruleName The name of the rule to run.
      * @param {Function} rule The rule to test.
-     * @param {Object} test The collection of tests to run.
+     * @param {{
+     *   valid: (ValidTestCase | string)[],
+     *   invalid: InvalidTestCase[]
+     * }} test The collection of tests to run.
      * @returns {void}
      */
     run(ruleName, rule, test) {
diff --git a/tests/lib/rules/no-dupe-keys.js b/tests/lib/rules/no-dupe-keys.js
index a0a1e7a8c..a6062a2a2 100644
--- a/tests/lib/rules/no-dupe-keys.js
+++ b/tests/lib/rules/no-dupe-keys.js
@@ -32,6 +32,7 @@ ruleTester.run("no-dupe-keys", rule, {
         { code: "var x = { a: b, ...c }", parserOptions: { ecmaVersion: 2018 } },
         { code: "var x = { get a() {}, set a (value) {} };", parserOptions: { ecmaVersion: 6 } },
         { code: "var x = { a: 1, b: { a: 2 } };", parserOptions: { ecmaVersion: 6 } },
+        { code: "var x = ({ null: 1, [/(?<zero>0)/]: 2 })", parserOptions: { ecmaVersion: 2018 } },
         { code: "var {a, a} = obj", parserOptions: { ecmaVersion: 6 } }
     ],
     invalid: [
@@ -44,6 +45,7 @@ ruleTester.run("no-dupe-keys", rule, {
         { code: "var foo = {\n  bar: 1,\n  bar: 1,\n}", errors: [{ messageId: "unexpected", data: { name: "bar" }, line: 3, column: 3 }] },
         { code: "var x = { a: 1, get a() {} };", parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpected", data: { name: "a" }, type: "ObjectExpression" }] },
         { code: "var x = { a: 1, set a(value) {} };", parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpected", data: { name: "a" }, type: "ObjectExpression" }] },
-        { code: "var x = { a: 1, b: { a: 2 }, get b() {} };", parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpected", data: { name: "b" }, type: "ObjectExpression" }] }
+        { code: "var x = { a: 1, b: { a: 2 }, get b() {} };", parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpected", data: { name: "b" }, type: "ObjectExpression" }] },
+        { code: "var x = ({ '/(?<zero>0)/': 1, [/(?<zero>0)/]: 2 })", parserOptions: { ecmaVersion: 2018 }, errors: [{ messageId: "unexpected", data: { name: "/(?<zero>0)/" }, type: "ObjectExpression" }] }
     ]
 });
diff --git a/tests/lib/rules/no-restricted-properties.js b/tests/lib/rules/no-restricted-properties.js
index c1be7a12c..9c5c57950 100644
--- a/tests/lib/rules/no-restricted-properties.js
+++ b/tests/lib/rules/no-restricted-properties.js
@@ -95,6 +95,12 @@ ruleTester.run("no-restricted-properties", rule, {
             options: [{
                 object: "foo"
             }]
+        }, {
+            code: "foo[/(?<zero>0)/]",
+            options: [{
+                property: "null"
+            }],
+            parserOptions: { ecmaVersion: 2018 }
         }, {
             code: "let bar = foo;",
             options: [{ object: "foo", property: "bar" }],
@@ -248,6 +254,11 @@ ruleTester.run("no-restricted-properties", rule, {
             code: "foo.bar.baz();",
             options: [{ property: "bar" }],
             errors: [{ message: "'bar' is restricted from being used.", type: "MemberExpression" }]
+        }, {
+            code: "foo[/(?<zero>0)/]",
+            options: [{ property: "/(?<zero>0)/" }],
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [{ message: "'/(?<zero>0)/' is restricted from being used.", type: "MemberExpression" }]
         }, {
             code: "require.call({}, 'foo')",
             options: [{
diff --git a/tests/lib/rules/no-self-assign.js b/tests/lib/rules/no-self-assign.js
index eaa0d02d1..c20c01fcb 100644
--- a/tests/lib/rules/no-self-assign.js
+++ b/tests/lib/rules/no-self-assign.js
@@ -50,6 +50,7 @@ ruleTester.run("no-self-assign", rule, {
         { code: "a[b] = a.b", options: [{ props: true }] },
         { code: "a.b().c = a.b().c", options: [{ props: true }] },
         { code: "b().c = b().c", options: [{ props: true }] },
+        { code: "a.null = a[/(?<zero>0)/]", options: [{ props: true }], parserOptions: { ecmaVersion: 2018 } },
         { code: "a[b + 1] = a[b + 1]", options: [{ props: true }] }, // it ignores non-simple computed properties.
         {
             code: "a.b = a.b",
@@ -133,6 +134,7 @@ ruleTester.run("no-self-assign", rule, {
             code: "this.x = this.x",
             options: [{ props: true }],
             errors: ["'this.x' is assigned to itself."]
-        }
+        },
+        { code: "a['/(?<zero>0)/'] = a[/(?<zero>0)/]", options: [{ props: true }], parserOptions: { ecmaVersion: 2018 }, errors: ["'a[/(?<zero>0)/]' is assigned to itself."] }
     ]
 });
diff --git a/tests/lib/rules/one-var.js b/tests/lib/rules/one-var.js
index e8937d637..83fe567b7 100644
--- a/tests/lib/rules/one-var.js
+++ b/tests/lib/rules/one-var.js
@@ -201,13 +201,11 @@ ruleTester.run("one-var", rule, {
         },
         {
             code: "var foo = require('foo'), bar;",
-            options: [{ separateRequires: false, var: "always" }],
-            parserOptions: { env: { node: true } }
+            options: [{ separateRequires: false, var: "always" }]
         },
         {
             code: "var foo = require('foo'), bar = require('bar');",
-            options: [{ separateRequires: true, var: "always" }],
-            parserOptions: { env: { node: true } }
+            options: [{ separateRequires: true, var: "always" }]
         },
         {
             code: "var bar = 'bar'; var foo = require('foo');",
@@ -217,11 +215,6 @@ ruleTester.run("one-var", rule, {
             code: "var foo = require('foo'); var bar = 'bar';",
             options: [{ separateRequires: true, var: "always" }]
         },
-        {
-            code: "var foo = require('foo'); var bar = 'bar';",
-            options: [{ separateRequires: true, var: "always" }],
-            parserOptions: { env: { node: true } }
-        },
 
         // https://github.com/eslint/eslint/issues/4680
         {
@@ -968,7 +961,6 @@ ruleTester.run("one-var", rule, {
             code: "var foo = require('foo'), bar;",
             output: null,
             options: [{ separateRequires: true, var: "always" }],
-            parserOptions: { env: { node: true } },
             errors: [{
                 message: "Split requires to be separated into a single block.",
                 type: "VariableDeclaration",
@@ -980,7 +972,6 @@ ruleTester.run("one-var", rule, {
             code: "var foo, bar = require('bar');",
             output: null,
             options: [{ separateRequires: true, var: "always" }],
-            parserOptions: { env: { node: true } },
             errors: [{
                 message: "Split requires to be separated into a single block.",
                 type: "VariableDeclaration",
@@ -992,7 +983,6 @@ ruleTester.run("one-var", rule, {
             code: "let foo, bar = require('bar');",
             output: null,
             options: [{ separateRequires: true, let: "always" }],
-            parserOptions: { env: { node: true } },
             errors: [{
                 message: "Split requires to be separated into a single block.",
                 type: "VariableDeclaration",
@@ -1004,7 +994,6 @@ ruleTester.run("one-var", rule, {
             code: "const foo = 0, bar = require('bar');",
             output: null,
             options: [{ separateRequires: true, const: "always" }],
-            parserOptions: { env: { node: true } },
             errors: [{
                 message: "Split requires to be separated into a single block.",
                 type: "VariableDeclaration",
@@ -1016,7 +1005,6 @@ ruleTester.run("one-var", rule, {
             code: "const foo = require('foo'); const bar = require('bar');",
             output: "const foo = require('foo'),  bar = require('bar');",
             options: [{ separateRequires: true, const: "always" }],
-            parserOptions: { env: { node: true } },
             errors: [{
                 message: "Combine this with the previous 'const' statement.",
                 type: "VariableDeclaration",
diff --git a/tests/lib/rules/sort-keys.js b/tests/lib/rules/sort-keys.js
index eb26c5250..31ca1c2ac 100644
--- a/tests/lib/rules/sort-keys.js
+++ b/tests/lib/rules/sort-keys.js
@@ -33,6 +33,7 @@ ruleTester.run("sort-keys", rule, {
         { code: "var obj = {$:1, A:3, _:2, a:4}", options: [] },
         { code: "var obj = {1:1, '11':2, 2:4, A:3}", options: [] },
         { code: "var obj = {'#':1, 'Z':2, À:3, è:4}", options: [] },
+        { code: "var obj = { [/(?<zero>0)/]: 1, '/(?<zero>0)/': 2 }", options: [], parserOptions: { ecmaVersion: 2018 } },
 
         // ignore non-simple computed properties.
         { code: "var obj = {a:1, b:3, [a + b]: -1, c:2}", options: [], parserOptions: { ecmaVersion: 6 } },
@@ -204,6 +205,11 @@ ruleTester.run("sort-keys", rule, {
             code: "var obj = {'#':1, À:3, 'Z':2, è:4}",
             errors: ["Expected object keys to be in ascending order. 'Z' should be before 'À'."]
         },
+        {
+            code: "var obj = { null: 1, [/(?<zero>0)/]: 2 }",
+            parserOptions: { ecmaVersion: 2018 },
+            errors: ["Expected object keys to be in ascending order. '/(?<zero>0)/' should be before 'null'."]
+        },
 
         // not ignore properties not separated by spread properties
         {
diff --git a/tests/lib/rules/utils/ast-utils.js b/tests/lib/rules/utils/ast-utils.js
index fb5526ff6..1acc22701 100644
--- a/tests/lib/rules/utils/ast-utils.js
+++ b/tests/lib/rules/utils/ast-utils.js
@@ -403,6 +403,83 @@ describe("ast-utils", () => {
         });
     });
 
+    describe("getStaticStringValue", () => {
+
+        /* eslint-disable quote-props */
+        const expectedResults = {
+
+            // string literals
+            "''": "",
+            "'foo'": "foo",
+
+            // boolean literals
+            "false": "false",
+            "true": "true",
+
+            // null literal
+            "null": "null",
+
+            // number literals
+            "0": "0",
+            "0.": "0",
+            ".0": "0",
+            "1": "1",
+            "1.": "1",
+            ".1": "0.1",
+            "12": "12",
+            ".12": "0.12",
+            "0.12": "0.12",
+            "12.34": "12.34",
+            "12e3": "12000",
+            "12e-3": "0.012",
+            "12.34e5": "1234000",
+            "12.34e-5": "0.0001234",
+            "011": "9",
+            "081": "81",
+            "0b11": "3",
+            "0b011": "3",
+            "0o11": "9",
+            "0o011": "9",
+            "0x11": "17",
+            "0x011": "17",
+
+            // regexp literals
+            "/a/": "/a/",
+            "/a/i": "/a/i",
+            "/[0-9]/": "/[0-9]/",
+            "/(?<zero>0)/": "/(?<zero>0)/",
+            "/(?<zero>0)/s": "/(?<zero>0)/s",
+            "/(?<=a)b/s": "/(?<=a)b/s",
+
+            // simple template literals
+            "``": "",
+            "`foo`": "foo",
+
+            // unsupported
+            "`${''}`": null,
+            "`${0}`": null,
+            "tag``": null,
+            "-0": null,
+            "-1": null,
+            "1 + 2": null,
+            "[]": null,
+            "({})": null,
+            "foo": null,
+            "undefined": null,
+            "this": null,
+            "(function () {})": null
+        };
+        /* eslint-enable quote-props */
+
+        Object.keys(expectedResults).forEach(key => {
+            it(`should return ${expectedResults[key]} for ${key}`, () => {
+                const ast = espree.parse(key, { ecmaVersion: 2018 });
+
+                assert.strictEqual(astUtils.getStaticStringValue(ast.body[0].expression), expectedResults[key]);
+            });
+        });
+    });
+
     describe("getStaticPropertyName", () => {
         it("should return 'b' for `a.b`", () => {
             const ast = espree.parse("a.b");
@@ -509,6 +586,13 @@ describe("ast-utils", () => {
             assert.strictEqual(astUtils.getStaticPropertyName(node), "100");
         });
 
+        it("should return '/(?<zero>0)/' for `[/(?<zero>0)/]: 1`", () => {
+            const ast = espree.parse("({[/(?<zero>0)/]: 1})", { ecmaVersion: 2018 });
+            const node = ast.body[0].expression.properties[0];
+
+            assert.strictEqual(astUtils.getStaticPropertyName(node), "/(?<zero>0)/");
+        });
+
         it("should return null for `[b]: 1`", () => {
             const ast = espree.parse("({[b]: 1})", { ecmaVersion: 6 });
             const node = ast.body[0].expression.properties[0];
diff --git a/tests/lib/rules/yoda.js b/tests/lib/rules/yoda.js
index 7908499d8..414cb5a04 100644
--- a/tests/lib/rules/yoda.js
+++ b/tests/lib/rules/yoda.js
@@ -91,6 +91,10 @@ ruleTester.run("yoda", rule, {
         }, {
             code: "if (0 <= a.b && a[\"b\"] <= 100) {}",
             options: ["never", { exceptRange: true }]
+        }, {
+            code: "if (1 <= a['/(?<zero>0)/'] && a[/(?<zero>0)/] <= 100) {}",
+            options: ["never", { exceptRange: true }],
+            parserOptions: { ecmaVersion: 2018 }
         },
 
         // onlyEquality
@@ -412,6 +416,19 @@ ruleTester.run("yoda", rule, {
                 }
             ]
         },
+        {
+            code: "if (0 <= a.null && a[/(?<zero>0)/] <= 1) {}",
+            output: "if (a.null >= 0 && a[/(?<zero>0)/] <= 1) {}",
+            options: ["never", { exceptRange: true }],
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                {
+                    messageId: "expected",
+                    data: { expectedSide: "right", operator: "<=" },
+                    type: "BinaryExpression"
+                }
+            ]
+        },
         {
             code: "if (3 == a) {}",
             output: "if (a == 3) {}",