Fix error when computing precise issue location in regexp string lite…

…ral (#2734)
SonarSource · Aug 3, 2021 · af5374d · af5374d
1 parent 76e80f7
commit af5374d
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 8 deletions.
diff --git a/eslint-bridge/src/utils/utils-regex.ts b/eslint-bridge/src/utils/utils-regex.ts
@@ -26,6 +26,7 @@ import { getUniqueWriteUsage, isRegexLiteral, isStringLiteral } from './utils-as
 import { ParserServices, TSESTree } from '@typescript-eslint/experimental-utils';
 import { tokenizeString } from './utils-string-literal';
 import { isString } from './utils-type';
+import { last } from './utils-collection';
 
 /**
  * An alternation is a regexpp node that has an `alternatives` field.
@@ -137,9 +138,29 @@ function getRegexpRange(node: estree.Node, regexpNode: regexpp.AST.Node): AST.Ra
     }
     const s = node.raw!;
     const tokens = tokenizeString(unquote(s));
-    const start = tokens[regexpNode.start - 1].range[0];
-    const end = tokens[regexpNode.end - 2].range[1];
-    return [start, end];
+    if (regexpNode.start === regexpNode.end) {
+      // this happens in case of empty alternative node like '|'
+      if (regexpNode.start - 1 < tokens.length) {
+        // '|' first empty alternative will have start = 1, end = 1
+        // +1 is to account for string quote
+        return [
+          tokens[regexpNode.start - 1].range[0] + 1,
+          tokens[regexpNode.start - 1].range[0] + 1,
+        ];
+      } else {
+        // '|' second empty alternative regex node will have start = 2, end = 2
+        // +1 is to account for string quote
+        return [last(tokens).range[1] + 1, last(tokens).range[1] + 1];
+      }
+    }
+    // regexpNode positions are 1 - based, we need to -1 to report as 0 - based
+    const startToken = regexpNode.start - 1;
+    const start = tokens[startToken].range[0];
+    // it's possible for node to be outside of range, eg new RegExp('\n(|)')
+    const endToken = Math.min(regexpNode.end - 2, tokens.length - 1);
+    const end = tokens[endToken].range[1];
+    // +1 is needed to account for string quotes
+    return [start + 1, end + 1];
   }
   throw new Error(`Expected regexp or string literal, got ${node.type}`);
 }

diff --git a/eslint-bridge/tests/rules/no-empty-alternatives.test.ts b/eslint-bridge/tests/rules/no-empty-alternatives.test.ts
@@ -119,5 +119,36 @@ ruleTester.run('Alternation with empty alternatives', rule, {
       code: `/(a|){1,3}/`,
       errors: 1,
     },
+    {
+      code: `new RegExp('|')`,
+      errors: [
+        {
+          message: 'Remove this empty alternative.',
+          line: 1,
+          endLine: 1,
+          column: 13,
+          endColumn: 14,
+        },
+        {
+          message: 'Remove this empty alternative.',
+          line: 1,
+          endLine: 1,
+          column: 13,
+          endColumn: 14,
+        },
+      ],
+    },
+    {
+      code: `new RegExp('a\\n(|)')`,
+      errors: [
+        {
+          message: 'Remove this empty alternative.',
+          line: 1,
+          endLine: 1,
+          column: 18,
+          endColumn: 19,
+        },
+      ],
+    },
   ],
 });
diff --git a/eslint-bridge/tests/rules/no-empty-group.test.ts b/eslint-bridge/tests/rules/no-empty-group.test.ts
@@ -59,8 +59,8 @@ ruleTester.run('Empty groups', rule, {
           message: 'Remove this empty group.',
           line: 1,
           endLine: 1,
-          column: 28,
-          endColumn: 30,
+          column: 29,
+          endColumn: 31,
         },
       ],
     },
@@ -76,5 +76,42 @@ ruleTester.run('Empty groups', rule, {
       code: `new RegExp('')`, // parsed as /(?:)/
       errors: 1,
     },
+    {
+      // \u0009 is unicode escape for TAB
+      code: `new RegExp('\\u0009(|)')`,
+      errors: [
+        {
+          message: 'Remove this empty group.',
+          line: 1,
+          endLine: 1,
+          column: 19,
+          endColumn: 22,
+        },
+      ],
+    },
+    {
+      code: `new RegExp('\\t(|)')`,
+      errors: [
+        {
+          message: 'Remove this empty group.',
+          line: 1,
+          endLine: 1,
+          column: 15,
+          endColumn: 18,
+        },
+      ],
+    },
+    {
+      code: `new RegExp('\\n(|)')`,
+      errors: [
+        {
+          message: 'Remove this empty group.',
+          line: 1,
+          endLine: 1,
+          column: 16,
+          endColumn: 18,
+        },
+      ],
+    },
   ],
 });
diff --git a/eslint-bridge/tests/rules/regex-rule-template.test.ts b/eslint-bridge/tests/rules/regex-rule-template.test.ts
@@ -120,15 +120,23 @@ typeAwareRuleTester.run('Template for regular expressions rules', rule, {
         {
           message: `Found character 'a'.`,
           line: 1,
-          column: 19,
+          column: 20,
           endLine: 1,
-          endColumn: 20,
+          endColumn: 21,
         },
       ],
     },
     {
       code: `const re = new RegExp('a');`,
-      errors: 1,
+      errors: [
+        {
+          message: `Found character 'a'.`,
+          line: 1,
+          column: 24,
+          endLine: 1,
+          endColumn: 25,
+        },
+      ],
     },
     {
       code: `const re = new RegExp('\u0061', 'u');`,