Repository: eslint/eslint
Pull request #12265 (2019-09-12 21:58:57)

Fix: eqeqeq rule reports incorrect locations
<!--
    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] Bug fix

<!--
    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.)
-->

**Tell us about your environment**

* **ESLint Version:**  6.3.0
* **Node Version:** 10.16.0
* **npm Version:** 6.9.0

**What parser (default, Babel-ESLint, etc.) are you using?**

default

**Please show your full configuration:**

<details>
<summary>Configuration</summary>

<!-- Paste your configuration below: -->
```js
module.exports = {
  parserOptions: {
    ecmaVersion: 2015,
  }
};
```

</details>

**What did you do? Please include the actual source code causing the issue.**

[Demo link](https://eslint.org/demo#eyJ0ZXh0IjoiLyogZXNsaW50IGVxZXFlcTplcnJvciAqL1xuXG4oYSArIGIpID09IGM7Iiwib3B0aW9ucyI6eyJwYXJzZXJPcHRpb25zIjp7ImVjbWFWZXJzaW9uIjo2LCJzb3VyY2VUeXBlIjoic2NyaXB0IiwiZWNtYUZlYXR1cmVzIjp7fX0sInJ1bGVzIjp7ImNvbnN0cnVjdG9yLXN1cGVyIjoyLCJmb3ItZGlyZWN0aW9uIjoyLCJnZXR0ZXItcmV0dXJuIjoyLCJuby1hc3luYy1wcm9taXNlLWV4ZWN1dG9yIjoyLCJuby1jYXNlLWRlY2xhcmF0aW9ucyI6Miwibm8tY2xhc3MtYXNzaWduIjoyLCJuby1jb21wYXJlLW5lZy16ZXJvIjoyLCJuby1jb25kLWFzc2lnbiI6Miwibm8tY29uc3QtYXNzaWduIjoyLCJuby1jb25zdGFudC1jb25kaXRpb24iOjIsIm5vLWNvbnRyb2wtcmVnZXgiOjIsIm5vLWRlYnVnZ2VyIjoyLCJuby1kZWxldGUtdmFyIjoyLCJuby1kdXBlLWFyZ3MiOjIsIm5vLWR1cGUtY2xhc3MtbWVtYmVycyI6Miwibm8tZHVwZS1rZXlzIjoyLCJuby1kdXBsaWNhdGUtY2FzZSI6Miwibm8tZW1wdHkiOjIsIm5vLWVtcHR5LWNoYXJhY3Rlci1jbGFzcyI6Miwibm8tZW1wdHktcGF0dGVybiI6Miwibm8tZXgtYXNzaWduIjoyLCJuby1leHRyYS1ib29sZWFuLWNhc3QiOjIsIm5vLWV4dHJhLXNlbWkiOjIsIm5vLWZhbGx0aHJvdWdoIjoyLCJuby1mdW5jLWFzc2lnbiI6Miwibm8tZ2xvYmFsLWFzc2lnbiI6Miwibm8taW5uZXItZGVjbGFyYXRpb25zIjoyLCJuby1pbnZhbGlkLXJlZ2V4cCI6Miwibm8taXJyZWd1bGFyLXdoaXRlc3BhY2UiOjIsIm5vLW1pc2xlYWRpbmctY2hhcmFjdGVyLWNsYXNzIjoyLCJuby1taXhlZC1zcGFjZXMtYW5kLXRhYnMiOjIsIm5vLW5ldy1zeW1ib2wiOjIsIm5vLW9iai1jYWxscyI6Miwibm8tb2N0YWwiOjIsIm5vLXByb3RvdHlwZS1idWlsdGlucyI6Miwibm8tcmVkZWNsYXJlIjoyLCJuby1yZWdleC1zcGFjZXMiOjIsIm5vLXNlbGYtYXNzaWduIjoyLCJuby1zaGFkb3ctcmVzdHJpY3RlZC1uYW1lcyI6Miwibm8tc3BhcnNlLWFycmF5cyI6Miwibm8tdGhpcy1iZWZvcmUtc3VwZXIiOjIsIm5vLXVuZXhwZWN0ZWQtbXVsdGlsaW5lIjoyLCJuby11bnJlYWNoYWJsZSI6Miwibm8tdW5zYWZlLWZpbmFsbHkiOjIsIm5vLXVuc2FmZS1uZWdhdGlvbiI6Miwibm8tdW51c2VkLWxhYmVscyI6Miwibm8tdW51c2VkLXZhcnMiOjIsIm5vLXVzZWxlc3MtY2F0Y2giOjIsIm5vLXVzZWxlc3MtZXNjYXBlIjoyLCJuby13aXRoIjoyLCJyZXF1aXJlLWF0b21pYy11cGRhdGVzIjoyLCJyZXF1aXJlLXlpZWxkIjoyLCJ1c2UtaXNuYW4iOjIsInZhbGlkLXR5cGVvZiI6Mn0sImVudiI6e319fQ==)

```js
/* eslint eqeqeq:error */

(a + b) == c;
```

**What did you expect to happen?**

1 error with the correct location (column 9)

**What actually happened? Please include the actual, raw output from ESLint.**

```
3:7  error  Expected '===' and instead saw '=='  eqeqeq
```
<!--
    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)**

Changed `eqeqeq` to find first operator token between, instead of just the first one which could be a paren.

Also, changed the rule to report `loc` instead of `loc.start`. One test in `cli-engine` had to be modified because of that.

**Is there anything you'd like reviewers to focus on?**

I hope it's ok to change from `loc.start` to `loc`, it always looks better in VS Code.



Patch:
diff --git a/docs/rules/no-sequences.md b/docs/rules/no-sequences.md
index f1f4afea0..602a412f1 100644
--- a/docs/rules/no-sequences.md
+++ b/docs/rules/no-sequences.md
@@ -60,7 +60,7 @@ switch ((val = foo(), val)) {}
 
 while ((val = foo(), val < 42));
 
-// with ((doSomething(), val)) {}
+with ((doSomething(), val)) {}
 ```
 
 ## When Not To Use It
diff --git a/docs/rules/no-useless-catch.md b/docs/rules/no-useless-catch.md
index d320e561d..0562063f6 100644
--- a/docs/rules/no-useless-catch.md
+++ b/docs/rules/no-useless-catch.md
@@ -43,6 +43,12 @@ try {
 } catch (e) {
   handleError(e);
 }
+
+try {
+  doSomethingThatMightThrow();
+} finally {
+  cleanUp();
+}
 ```
 
 ## When Not To Use It
diff --git a/lib/rules/eqeqeq.js b/lib/rules/eqeqeq.js
index 58ff9030d..57926dbed 100644
--- a/lib/rules/eqeqeq.js
+++ b/lib/rules/eqeqeq.js
@@ -116,18 +116,6 @@ module.exports = {
             return astUtils.isNullLiteral(node.right) || astUtils.isNullLiteral(node.left);
         }
 
-        /**
-         * Gets the location (line and column) of the binary expression's operator
-         * @param {ASTNode} node The binary expression node to check
-         * @returns {Object} { line, column } location of operator
-         * @private
-         */
-        function getOperatorLocation(node) {
-            const opToken = sourceCode.getTokenAfter(node.left);
-
-            return { line: opToken.loc.start.line, column: opToken.loc.start.column };
-        }
-
         /**
          * Reports a message for this rule.
          * @param {ASTNode} node The binary expression node that was checked
@@ -136,21 +124,21 @@ module.exports = {
          * @private
          */
         function report(node, expectedOperator) {
+            const operatorToken = sourceCode.getFirstTokenBetween(
+                node.left,
+                node.right,
+                token => token.value === node.operator
+            );
+
             context.report({
                 node,
-                loc: getOperatorLocation(node),
+                loc: operatorToken.loc,
                 messageId: "unexpected",
                 data: { expectedOperator, actualOperator: node.operator },
                 fix(fixer) {
 
                     // If the comparison is a `typeof` comparison or both sides are literals with the same type, then it's safe to fix.
                     if (isTypeOfBinary(node) || areLiteralsAndSameType(node)) {
-                        const operatorToken = sourceCode.getFirstTokenBetween(
-                            node.left,
-                            node.right,
-                            token => token.value === node.operator
-                        );
-
                         return fixer.replaceText(operatorToken, expectedOperator);
                     }
                     return null;
diff --git a/lib/rules/no-octal-escape.js b/lib/rules/no-octal-escape.js
index a2860ca5b..7f6845ec7 100644
--- a/lib/rules/no-octal-escape.js
+++ b/lib/rules/no-octal-escape.js
@@ -20,7 +20,11 @@ module.exports = {
             url: "https://eslint.org/docs/rules/no-octal-escape"
         },
 
-        schema: []
+        schema: [],
+
+        messages: {
+            octalEscapeSequence: "Don't use octal: '\\{{sequence}}'. Use '\\u....' instead."
+        }
     },
 
     create(context) {
@@ -32,15 +36,17 @@ module.exports = {
                     return;
                 }
 
-                const match = node.raw.match(/^([^\\]|\\[^0-7])*\\([0-3][0-7]{1,2}|[4-7][0-7]|[0-7])/u);
+                // \0 represents a valid NULL character if it isn't followed by a digit.
+                const match = node.raw.match(
+                    /^(?:[^\\]|\\.)*?\\([0-3][0-7]{1,2}|[4-7][0-7]|[1-7])/u
+                );
 
                 if (match) {
-                    const octalDigit = match[2];
-
-                    // \0 is actually not considered an octal
-                    if (match[2] !== "0" || typeof match[3] !== "undefined") {
-                        context.report({ node, message: "Don't use octal: '\\{{octalDigit}}'. Use '\\u....' instead.", data: { octalDigit } });
-                    }
+                    context.report({
+                        node,
+                        messageId: "octalEscapeSequence",
+                        data: { sequence: match[1] }
+                    });
                 }
             }
 
diff --git a/lib/rules/no-unsafe-negation.js b/lib/rules/no-unsafe-negation.js
index f394f6742..717e5f6be 100644
--- a/lib/rules/no-unsafe-negation.js
+++ b/lib/rules/no-unsafe-negation.js
@@ -51,7 +51,7 @@ module.exports = {
         },
 
         schema: [],
-        fixable: "code",
+        fixable: null,
         messages: {
             unexpected: "Unexpected negating the left operand of '{{operator}}' operator."
         }
@@ -70,15 +70,7 @@ module.exports = {
                         node,
                         loc: node.left.loc,
                         messageId: "unexpected",
-                        data: { operator: node.operator },
-
-                        fix(fixer) {
-                            const negationToken = sourceCode.getFirstToken(node.left);
-                            const fixRange = [negationToken.range[1], node.range[1]];
-                            const text = sourceCode.text.slice(fixRange[0], fixRange[1]);
-
-                            return fixer.replaceTextRange(fixRange, `(${text})`);
-                        }
+                        data: { operator: node.operator }
                     });
                 }
             }


Test patch:
diff --git a/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js
index 0c6db3ff9..b3039e743 100644
--- a/tests/lib/cli-engine/cli-engine.js
+++ b/tests/lib/cli-engine/cli-engine.js
@@ -1641,6 +1641,8 @@ describe("CLIEngine", () => {
                             {
                                 column: 9,
                                 line: 2,
+                                endColumn: 11,
+                                endLine: 2,
                                 message: "Expected '===' and instead saw '=='.",
                                 messageId: "unexpected",
                                 nodeType: "BinaryExpression",
diff --git a/tests/lib/rules/eqeqeq.js b/tests/lib/rules/eqeqeq.js
index b74e9bc2c..05e33bbcf 100644
--- a/tests/lib/rules/eqeqeq.js
+++ b/tests/lib/rules/eqeqeq.js
@@ -105,6 +105,69 @@ ruleTester.run("eqeqeq", rule, {
                 { messageId: "unexpected", data: wantedNotEqEq, type: "BinaryExpression", line: 1 },
                 { messageId: "unexpected", data: wantedNotEqEq, type: "BinaryExpression", line: 1 }
             ]
+        },
+
+        // location tests
+        {
+            code: "a == b;",
+            errors: [
+                {
+                    messageId: "unexpected",
+                    data: wantedEqEqEq,
+                    type: "BinaryExpression",
+                    column: 3,
+                    endColumn: 5
+                }
+            ]
+        },
+        {
+            code: "a!=b;",
+            errors: [
+                {
+                    messageId: "unexpected",
+                    data: wantedNotEqEq,
+                    type: "BinaryExpression",
+                    column: 2,
+                    endColumn: 4
+                }
+            ]
+        },
+        {
+            code: "(a + b) == c;",
+            errors: [
+                {
+                    messageId: "unexpected",
+                    data: wantedEqEqEq,
+                    type: "BinaryExpression",
+                    column: 9,
+                    endColumn: 11
+                }
+            ]
+        },
+        {
+            code: "(a + b)  !=  c;",
+            errors: [
+                {
+                    messageId: "unexpected",
+                    data: wantedNotEqEq,
+                    type: "BinaryExpression",
+                    column: 10,
+                    endColumn: 12
+                }
+            ]
+        },
+        {
+            code: "((1) )  ==  (2);",
+            output: "((1) )  ===  (2);",
+            errors: [
+                {
+                    messageId: "unexpected",
+                    data: wantedEqEqEq,
+                    type: "BinaryExpression",
+                    column: 9,
+                    endColumn: 11
+                }
+            ]
         }
 
     // If no output is provided, assert that no output is produced.
diff --git a/tests/lib/rules/no-octal-escape.js b/tests/lib/rules/no-octal-escape.js
index b15416ebf..cd555ab60 100644
--- a/tests/lib/rules/no-octal-escape.js
+++ b/tests/lib/rules/no-octal-escape.js
@@ -24,13 +24,34 @@ ruleTester.run("no-octal-escape", rule, {
         "var foo = \"\\x51\";",
         "var foo = \"foo \\\\251 bar\";",
         "var foo = /([abc]) \\1/g;",
-        "var foo = '\\0';"
+        "var foo = '\\0';",
+        "'\\0 '",
+        "'\\0a'",
+        "'\\\\1'",
+        "'\\\\01'",
+        "'\\08'",
+        "'\\09'"
     ],
     invalid: [
+
+        // Test full message
         { code: "var foo = \"foo \\01 bar\";", errors: [{ message: "Don't use octal: '\\01'. Use '\\u....' instead.", type: "Literal" }] },
-        { code: "var foo = \"foo \\251 bar\";", errors: [{ message: "Don't use octal: '\\251'. Use '\\u....' instead.", type: "Literal" }] },
-        { code: "var foo = \"\\751\";", errors: [{ message: "Don't use octal: '\\75'. Use '\\u....' instead.", type: "Literal" }] },
-        { code: "var foo = \"\\3s51\";", errors: [{ message: "Don't use octal: '\\3'. Use '\\u....' instead.", type: "Literal" }] },
-        { code: "var foo = \"\\\\\\751\";", errors: [{ message: "Don't use octal: '\\75'. Use '\\u....' instead.", type: "Literal" }] }
+
+        { code: "var foo = \"foo \\251 bar\";", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "251" }, type: "Literal" }] },
+        { code: "var foo = \"\\751\";", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "75" }, type: "Literal" }] },
+        { code: "var foo = \"\\3s51\";", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "3" }, type: "Literal" }] },
+        { code: "var foo = \"\\\\\\751\";", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "75" }, type: "Literal" }] },
+        { code: "'\\0\\1'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "1" }, type: "Literal" }] },
+        { code: "'\\0 \\1'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "1" }, type: "Literal" }] },
+        { code: "'\\0\\01'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "01" }, type: "Literal" }] },
+        { code: "'\\0 \\01'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "01" }, type: "Literal" }] },
+        { code: "'\\08\\1'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "1" }, type: "Literal" }] },
+        { code: "'\\08\\01'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "01" }, type: "Literal" }] },
+
+        // Only the first one is reported
+        { code: "'\\01\\02'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "01" }, type: "Literal" }] },
+        { code: "'\\02\\01'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "02" }, type: "Literal" }] },
+        { code: "'\\01\\2'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "01" }, type: "Literal" }] },
+        { code: "'\\2\\01'", errors: [{ messageId: "octalEscapeSequence", data: { sequence: "2" }, type: "Literal" }] }
     ]
 });
diff --git a/tests/lib/rules/no-unsafe-negation.js b/tests/lib/rules/no-unsafe-negation.js
index 8316aad02..6ac245412 100644
--- a/tests/lib/rules/no-unsafe-negation.js
+++ b/tests/lib/rules/no-unsafe-negation.js
@@ -34,32 +34,26 @@ ruleTester.run("no-unsafe-negation", rule, {
     invalid: [
         {
             code: "!a in b",
-            output: "!(a in b)",
             errors: [unexpectedInError]
         },
         {
             code: "(!a in b)",
-            output: "(!(a in b))",
             errors: [unexpectedInError]
         },
         {
             code: "!(a) in b",
-            output: "!((a) in b)",
             errors: [unexpectedInError]
         },
         {
             code: "!a instanceof b",
-            output: "!(a instanceof b)",
             errors: [unexpectedInstanceofError]
         },
         {
             code: "(!a instanceof b)",
-            output: "(!(a instanceof b))",
             errors: [unexpectedInstanceofError]
         },
         {
             code: "!(a) instanceof b",
-            output: "!((a) instanceof b)",
             errors: [unexpectedInstanceofError]
         }
     ]