Repository: eslint/eslint
Pull request #14915 (2021-08-10 22:49:54)

Fix: handle computed class fields in operator-linebreak (refs #14857)
<!--
    Thank you for contributing!

    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

#### Prerequisites checklist

- [x] I have read the [contributing guidelines](https://github.com/eslint/eslint/blob/master/CONTRIBUTING.md).

#### What is the purpose of this pull request? (put an "X" next to an item)

<!--
    The following template is intentionally not a markdown checkbox list for the reasons
    explained in https://github.com/eslint/eslint/pull/12848#issuecomment-580302888
-->

[ ] Documentation update
[x] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

<!--
    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.)
-->

refs #14857, fixes https://github.com/eslint/eslint/pull/14591#issuecomment-874963573

<!--
    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)

Updated the `operator-linebreak` rule to account for `]` of computed class fields by finding the operator token by `value`. Also switched to searching from the right side in order to better handle language extensions (e.g., typescript allows separate type annotation nodes with their own tokens between the left side node and the operator).

#### Is there anything you'd like reviewers to focus on?



Patch:
diff --git a/docs/rules/operator-linebreak.md b/docs/rules/operator-linebreak.md
index 159bd528a..6ae484ea5 100644
--- a/docs/rules/operator-linebreak.md
+++ b/docs/rules/operator-linebreak.md
@@ -60,6 +60,16 @@ if (someCondition
 answer = everything
   ? 42
   : foo;
+
+class Foo {
+    a
+        = 1;
+    [b]
+        = 2;
+    [c
+    ]
+        = 3;
+}
 ```
 
 Examples of **correct** code for this rule with the `"after"` option:
@@ -82,6 +92,17 @@ if (someCondition ||
 answer = everything ?
   42 :
   foo;
+
+class Foo {
+    a =
+        1;
+    [b] =
+        2;
+    [c
+    ] =
+        3;
+    d = 4;
+}
 ```
 
 ### before
@@ -104,6 +125,16 @@ if (someCondition ||
 answer = everything ?
   42 :
   foo;
+
+class Foo {
+    a =
+        1;
+    [b] =
+        2;
+    [c
+    ] =
+        3;
+}
 ```
 
 Examples of **correct** code for this rule with the `"before"` option:
@@ -126,6 +157,17 @@ if (someCondition
 answer = everything
   ? 42
   : foo;
+
+class Foo {
+    a
+        = 1;
+    [b]
+        = 2;
+    [c
+    ]
+        = 3;
+    d = 4;
+}
 ```
 
 ### none
@@ -156,6 +198,23 @@ answer = everything
 answer = everything ?
   42 :
   foo;
+
+class Foo {
+    a =
+        1;
+    [b] =
+        2;
+    [c
+    ] =
+        3;
+    d
+        = 4;
+    [e]
+        = 5;
+    [f
+    ]
+        = 6;
+}
 ```
 
 Examples of **correct** code for this rule with the `"none"` option:
@@ -171,6 +230,17 @@ if (someCondition || otherCondition) {
 }
 
 answer = everything ? 42 : foo;
+
+class Foo {
+    a = 1;
+    [b] = 2;
+    [c
+    ] = 3;
+    d = 4;
+    [e] = 5;
+    [f
+    ] = 6;
+}
 ```
 
 ### overrides
diff --git a/lib/rules/operator-linebreak.js b/lib/rules/operator-linebreak.js
index 40ca88057..2a0bfbddf 100644
--- a/lib/rules/operator-linebreak.js
+++ b/lib/rules/operator-linebreak.js
@@ -136,23 +136,21 @@ module.exports = {
         /**
          * Checks the operator placement
          * @param {ASTNode} node The node to check
-         * @param {ASTNode} leftSide The node that comes before the operator in `node`
+         * @param {ASTNode} rightSide The node that comes after the operator in `node`
+         * @param {string} operator The operator
          * @private
          * @returns {void}
          */
-        function validateNode(node, leftSide) {
+        function validateNode(node, rightSide, operator) {
 
             /*
-             * When the left part of a binary expression is a single expression wrapped in
-             * parentheses (ex: `(a) + b`), leftToken will be the last token of the expression
-             * and operatorToken will be the closing parenthesis.
-             * The leftToken should be the last closing parenthesis, and the operatorToken
-             * should be the token right after that.
+             * Find the operator token by searching from the right side, because between the left side and the operator
+             * there could be additional tokens from type annotations. Search specifically for the token which
+             * value equals the operator, in order to skip possible opening parentheses before the right side node.
              */
-            const operatorToken = sourceCode.getTokenAfter(leftSide, astUtils.isNotClosingParenToken);
+            const operatorToken = sourceCode.getTokenBefore(rightSide, token => token.value === operator);
             const leftToken = sourceCode.getTokenBefore(operatorToken);
             const rightToken = sourceCode.getTokenAfter(operatorToken);
-            const operator = operatorToken.value;
             const operatorStyleOverride = styleOverrides[operator];
             const style = operatorStyleOverride || globalStyle;
             const fix = getFixer(operatorToken, style);
@@ -222,7 +220,7 @@ module.exports = {
          * @returns {void}
          */
         function validateBinaryExpression(node) {
-            validateNode(node, node.left);
+            validateNode(node, node.right, node.operator);
         }
 
         //--------------------------------------------------------------------------
@@ -235,17 +233,17 @@ module.exports = {
             AssignmentExpression: validateBinaryExpression,
             VariableDeclarator(node) {
                 if (node.init) {
-                    validateNode(node, node.id);
+                    validateNode(node, node.init, "=");
                 }
             },
             PropertyDefinition(node) {
                 if (node.value) {
-                    validateNode(node, node.key);
+                    validateNode(node, node.value, "=");
                 }
             },
             ConditionalExpression(node) {
-                validateNode(node, node.test);
-                validateNode(node, node.consequent);
+                validateNode(node, node.consequent, "?");
+                validateNode(node, node.alternate, ":");
             }
         };
     }


Test patch:
diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js
index f54d4df03..c86e67ecf 100644
--- a/tests/lib/rules/no-restricted-imports.js
+++ b/tests/lib/rules/no-restricted-imports.js
@@ -29,6 +29,19 @@ ruleTester.run("no-restricted-imports", rule, {
         { code: "import \"foo/bar\";", options: ["foo"] },
         { code: "import withPaths from \"foo/bar\";", options: [{ paths: ["foo", "bar"] }] },
         { code: "import withPatterns from \"foo/bar\";", options: [{ patterns: ["foo/c*"] }] },
+        { code: "import foo from 'foo';", options: ["../foo"] },
+        { code: "import foo from 'foo';", options: [{ paths: ["../foo"] }] },
+        { code: "import foo from 'foo';", options: [{ patterns: ["../foo"] }] },
+        { code: "import foo from 'foo';", options: ["/foo"] },
+        { code: "import foo from 'foo';", options: [{ paths: ["/foo"] }] },
+        "import relative from '../foo';",
+        { code: "import relative from '../foo';", options: ["../notFoo"] },
+        { code: "import relativeWithPaths from '../foo';", options: [{ paths: ["../notFoo"] }] },
+        { code: "import relativeWithPatterns from '../foo';", options: [{ patterns: ["notFoo"] }] },
+        "import absolute from '/foo';",
+        { code: "import absolute from '/foo';", options: ["/notFoo"] },
+        { code: "import absoluteWithPaths from '/foo';", options: [{ paths: ["/notFoo"] }] },
+        { code: "import absoluteWithPatterns from '/foo';", options: [{ patterns: ["notFoo"] }] },
         {
             code: "import withPatternsAndPaths from \"foo/bar\";",
             options: [{ paths: ["foo"], patterns: ["foo/c*"] }]
@@ -815,6 +828,72 @@ ruleTester.run("no-restricted-imports", rule, {
             column: 15,
             endColumn: 29
         }]
+    },
+    {
+        code: "import relative from '../foo';",
+        options: ["../foo"],
+        errors: [{
+            message: "'../foo' import is restricted from being used.",
+            type: "ImportDeclaration",
+            line: 1,
+            column: 1,
+            endColumn: 31
+        }]
+    },
+    {
+        code: "import relativeWithPaths from '../foo';",
+        options: [{ paths: ["../foo"] }],
+        errors: [{
+            message: "'../foo' import is restricted from being used.",
+            type: "ImportDeclaration",
+            line: 1,
+            column: 1,
+            endColumn: 40
+        }]
+    },
+    {
+        code: "import relativeWithPatterns from '../foo';",
+        options: [{ patterns: ["../foo"] }],
+        errors: [{
+            message: "'../foo' import is restricted from being used by a pattern.",
+            type: "ImportDeclaration",
+            line: 1,
+            column: 1,
+            endColumn: 43
+        }]
+    },
+    {
+        code: "import absolute from '/foo';",
+        options: ["/foo"],
+        errors: [{
+            message: "'/foo' import is restricted from being used.",
+            type: "ImportDeclaration",
+            line: 1,
+            column: 1,
+            endColumn: 29
+        }]
+    },
+    {
+        code: "import absoluteWithPaths from '/foo';",
+        options: [{ paths: ["/foo"] }],
+        errors: [{
+            message: "'/foo' import is restricted from being used.",
+            type: "ImportDeclaration",
+            line: 1,
+            column: 1,
+            endColumn: 38
+        }]
+    },
+    {
+        code: "import absoluteWithPatterns from '/foo';",
+        options: [{ patterns: ["foo"] }],
+        errors: [{
+            message: "'/foo' import is restricted from being used by a pattern.",
+            type: "ImportDeclaration",
+            line: 1,
+            column: 1,
+            endColumn: 41
+        }]
     }
     ]
 });
diff --git a/tests/lib/rules/no-restricted-modules.js b/tests/lib/rules/no-restricted-modules.js
index 08cc7c092..5e89a6a73 100644
--- a/tests/lib/rules/no-restricted-modules.js
+++ b/tests/lib/rules/no-restricted-modules.js
@@ -32,7 +32,20 @@ ruleTester.run("no-restricted-modules", rule, {
         { code: "var withPatternsAndPaths = require(\"foo/bar\");", options: [{ paths: ["foo"], patterns: ["foo/c*"] }] },
         { code: "var withGitignores = require(\"foo/bar\");", options: [{ paths: ["foo"], patterns: ["foo/*", "!foo/bar"] }] },
         { code: "require(`fs`)", options: ["crypto"], parserOptions: { ecmaVersion: 6 } },
-        { code: "require(`foo${bar}`)", options: ["foo"], parserOptions: { ecmaVersion: 6 } }
+        { code: "require(`foo${bar}`)", options: ["foo"], parserOptions: { ecmaVersion: 6 } },
+        { code: "var foo = require('foo');", options: ["../foo"] },
+        { code: "var foo = require('foo');", options: [{ paths: ["../foo"] }] },
+        { code: "var foo = require('foo');", options: [{ patterns: ["../foo"] }] },
+        { code: "var foo = require('foo');", options: ["/foo"] },
+        { code: "var foo = require('foo');", options: [{ paths: ["/foo"] }] },
+        "var relative = require('../foo');",
+        { code: "var relative = require('../foo');", options: ["../notFoo"] },
+        { code: "var relativeWithPaths = require('../foo');", options: [{ paths: ["../notFoo"] }] },
+        { code: "var relativeWithPatterns = require('../foo');", options: [{ patterns: ["notFoo"] }] },
+        "var absolute = require('/foo');",
+        { code: "var absolute = require('/foo');", options: ["/notFoo"] },
+        { code: "var absoluteWithPaths = require('/foo');", options: [{ paths: ["/notFoo"] }] },
+        { code: "var absoluteWithPatterns = require('/foo');", options: [{ patterns: ["notFoo"] }] }
     ],
     invalid: [{
         code: "require(\"fs\")",
@@ -111,5 +124,71 @@ ruleTester.run("no-restricted-modules", rule, {
         options: ["crypto"],
         parserOptions: { ecmaVersion: 6 },
         errors: [{ messageId: "defaultMessage", data: { name: "crypto" }, type: "CallExpression" }]
+    },
+    {
+        code: "var relative = require('../foo');",
+        options: ["../foo"],
+        errors: [{
+            message: "'../foo' module is restricted from being used.",
+            type: "CallExpression",
+            line: 1,
+            column: 16,
+            endColumn: 33
+        }]
+    },
+    {
+        code: "var relativeWithPaths = require('../foo');",
+        options: [{ paths: ["../foo"] }],
+        errors: [{
+            message: "'../foo' module is restricted from being used.",
+            type: "CallExpression",
+            line: 1,
+            column: 25,
+            endColumn: 42
+        }]
+    },
+    {
+        code: "var relativeWithPatterns = require('../foo');",
+        options: [{ patterns: ["../foo"] }],
+        errors: [{
+            message: "'../foo' module is restricted from being used by a pattern.",
+            type: "CallExpression",
+            line: 1,
+            column: 28,
+            endColumn: 45
+        }]
+    },
+    {
+        code: "var absolute = require('/foo');",
+        options: ["/foo"],
+        errors: [{
+            message: "'/foo' module is restricted from being used.",
+            type: "CallExpression",
+            line: 1,
+            column: 16,
+            endColumn: 31
+        }]
+    },
+    {
+        code: "var absoluteWithPaths = require('/foo');",
+        options: [{ paths: ["/foo"] }],
+        errors: [{
+            message: "'/foo' module is restricted from being used.",
+            type: "CallExpression",
+            line: 1,
+            column: 25,
+            endColumn: 40
+        }]
+    },
+    {
+        code: "var absoluteWithPatterns = require('/foo');",
+        options: [{ patterns: ["foo"] }],
+        errors: [{
+            message: "'/foo' module is restricted from being used by a pattern.",
+            type: "CallExpression",
+            line: 1,
+            column: 28,
+            endColumn: 43
+        }]
     }]
 });
diff --git a/tests/lib/rules/operator-linebreak.js b/tests/lib/rules/operator-linebreak.js
index e94d48c11..8810c0407 100644
--- a/tests/lib/rules/operator-linebreak.js
+++ b/tests/lib/rules/operator-linebreak.js
@@ -56,6 +56,7 @@ ruleTester.run("operator-linebreak", rule, {
         { code: "\n1 + 1", options: ["none"] },
         { code: "1 + 1\n", options: ["none"] },
         { code: "answer = everything ? 42 : foo;", options: ["none"] },
+        { code: "(a\n) + (\nb)", options: ["none"] },
         { code: "answer = everything \n?\n 42 : foo;", options: [null, { overrides: { "?": "ignore" } }] },
         { code: "answer = everything ? 42 \n:\n foo;", options: [null, { overrides: { ":": "ignore" } }] },
 
@@ -100,6 +101,7 @@ ruleTester.run("operator-linebreak", rule, {
             parserOptions: { ecmaVersion: 2021 }
         },
 
+        // class fields
         {
             code: "class C { foo =\n0 }",
             parserOptions: { ecmaVersion: 2022 }
@@ -108,6 +110,41 @@ ruleTester.run("operator-linebreak", rule, {
             code: "class C { foo\n= 0 }",
             options: ["before"],
             parserOptions: { ecmaVersion: 2022 }
+        },
+        {
+            code: "class C { [foo\n]= 0 }",
+            options: ["before"],
+            parserOptions: { ecmaVersion: 2022 }
+        },
+        {
+            code: "class C { [foo]\n= 0 }",
+            options: ["before"],
+            parserOptions: { ecmaVersion: 2022 }
+        },
+        {
+            code: "class C { [foo\n]\n= 0 }",
+            options: ["before"],
+            parserOptions: { ecmaVersion: 2022 }
+        },
+        {
+            code: "class C { [foo\n]= 0 }",
+            options: ["after"],
+            parserOptions: { ecmaVersion: 2022 }
+        },
+        {
+            code: "class C { [foo\n]=\n0 }",
+            options: ["after"],
+            parserOptions: { ecmaVersion: 2022 }
+        },
+        {
+            code: "class C { [foo\n]= 0 }",
+            options: ["none"],
+            parserOptions: { ecmaVersion: 2022 }
+        },
+        {
+            code: "class C { foo\n=\n0 }",
+            options: ["none", { overrides: { "=": "ignore" } }],
+            parserOptions: { ecmaVersion: 2022 }
         }
     ],
 
@@ -782,6 +819,7 @@ ruleTester.run("operator-linebreak", rule, {
             }]
         },
 
+        // class fields
         {
             code: "class C { a\n= 0; }",
             output: "class C { a =\n0; }",
@@ -826,6 +864,51 @@ ruleTester.run("operator-linebreak", rule, {
                 endLine: 1,
                 endColumn: 14
             }]
+        },
+        {
+            code: "class C { [a]\n= 0; }",
+            output: "class C { [a] =\n0; }",
+            options: ["after"],
+            parserOptions: { ecmaVersion: 2022 },
+            errors: [{
+                messageId: "operatorAtEnd",
+                data: { operator: "=" },
+                type: "PropertyDefinition",
+                line: 2,
+                column: 1,
+                endLine: 2,
+                endColumn: 2
+            }]
+        },
+        {
+            code: "class C { [a] =\n0; }",
+            output: "class C { [a]\n= 0; }",
+            options: ["before"],
+            parserOptions: { ecmaVersion: 2022 },
+            errors: [{
+                messageId: "operatorAtBeginning",
+                data: { operator: "=" },
+                type: "PropertyDefinition",
+                line: 1,
+                column: 15,
+                endLine: 1,
+                endColumn: 16
+            }]
+        },
+        {
+            code: "class C { [a]\n =0; }",
+            output: "class C { [a] =0; }",
+            options: ["none"],
+            parserOptions: { ecmaVersion: 2022 },
+            errors: [{
+                messageId: "noLinebreak",
+                data: { operator: "=" },
+                type: "PropertyDefinition",
+                line: 2,
+                column: 2,
+                endLine: 2,
+                endColumn: 3
+            }]
         }
     ]
 });