Fix regex-related rules: Skip issues with inconsistent locations (#4337)

packages/jsts/src/rules/S5843/rule.ts

@@ -266,12 +266,14 @@ class ComplexityCalculator {
       message += ` (incl ${increment - 1} for nesting)`;
     }
     const loc = getRegexpLocation(this.regexPart, node, this.context, offset);
-    this.components.push({
-      location: {
-        loc,
-      },
-      message,
-    });
+    if (loc) {
+      this.components.push({
+        location: {
+          loc,
+        },
+        message,
+      });
+    }
   }
 
   private onDisjunctionEnter(node: Disjunction) {

packages/jsts/src/rules/S5843/unit.test.ts

@@ -110,6 +110,42 @@ ruleTesterThreshold0.run(
         ],
         options: [0],
       },
+      {
+        code: `RegExp('\\r?');`,
+        errors: [
+          {
+            message: JSON.stringify({
+              message: `Simplify this regular expression to reduce its complexity from 1 to the 0 allowed.`,
+              cost: 1,
+              secondaryLocations: [], // Secondary location removed when invalid (start===end)
+            }),
+            line: 1,
+            endLine: 1,
+            column: 8,
+            endColumn: 13,
+          },
+        ],
+        options: [0],
+      },
+      {
+        code: `RegExp('\\\\r?');`,
+        errors: [
+          {
+            message: JSON.stringify({
+              message: `Simplify this regular expression to reduce its complexity from 1 to the 0 allowed.`,
+              cost: 1,
+              secondaryLocations: [
+                { message: '+1', column: 11, line: 1, endColumn: 12, endLine: 1 },
+              ],
+            }),
+            line: 1,
+            endLine: 1,
+            column: 8,
+            endColumn: 14,
+          },
+        ],
+        options: [0],
+      },
       {
         code: `/(?<=abc)/`,
         errors: [

packages/jsts/src/rules/S5860/rule.ts

@@ -19,7 +19,7 @@
  */
 // https://sonarsource.github.io/rspec/#/rspec/S5860/javascript
 
-import { Rule, Scope } from 'eslint';
+import { AST, Rule, Scope } from 'eslint';
 import * as estree from 'estree';
 import * as regexpp from '@eslint-community/regexpp';
 import { Backreference, CapturingGroup, RegExpLiteral } from '@eslint-community/regexpp/ast';
@@ -34,6 +34,7 @@ import {
   isObjectDestructuring,
   isRequiredParserServices,
   isString,
+  LocationHolder,
   RequiredParserServices,
   toEncodedMessage,
 } from '../helpers';
@@ -116,13 +117,17 @@ function checkStringReplaceGroupReferences(
       });
       const indexedGroups = regex.groups.filter(group => indexes.has(group.index));
       if (indexedGroups.length > 0) {
+        const { locations, messages } = prepareSecondaries(
+          regex,
+          indexedGroups,
+          intellisense,
+          'Group',
+        );
         intellisense.context.report({
           message: toEncodedMessage(
             `Directly use the group names instead of their numbers.`,
-            indexedGroups.map(group => ({
-              loc: getRegexpLocation(regex.node, group.node, intellisense.context),
-            })),
-            indexedGroups.map(group => `Group '${group.name}'`),
+            locations,
+            messages,
           ),
           node: substr,
         });
@@ -144,11 +149,12 @@ function checkIndexBasedGroupReference(
       const group = regex.groups.find(grp => grp.index === index);
       if (group) {
         group.used = true;
+        const { locations, messages } = prepareSecondaries(regex, [group], intellisense, 'Group');
         intellisense.context.report({
           message: toEncodedMessage(
             `Directly use '${group.name}' instead of its group number.`,
-            [{ loc: getRegexpLocation(regex.node, group.node, intellisense.context) }],
-            [`Group '${group.name}'`],
+            locations,
+            messages,
           ),
           node: property,
         });
@@ -172,13 +178,18 @@ function checkNonExistingGroupReference(
       if (group) {
         group.used = true;
       } else {
+        const { locations, messages } = prepareSecondaries(
+          regex,
+          regex.groups,
+          intellisense,
+          'Named group',
+        );
+
         intellisense.context.report({
           message: toEncodedMessage(
             `There is no group named '${groupName}' in the regular expression.`,
-            regex.groups.map(grp => ({
-              loc: getRegexpLocation(regex.node, grp.node, intellisense.context),
-            })),
-            regex.groups.map(grp => `Named group '${grp.name}'`),
+            locations,
+            messages,
           ),
           node: groupNode,
         });
@@ -238,13 +249,18 @@ function checkUnusedGroups(intellisense: RegexIntelliSense) {
     if (regex.matched) {
       const unusedGroups = regex.groups.filter(group => !group.used);
       if (unusedGroups.length) {
+        const { locations, messages } = prepareSecondaries(
+          regex,
+          unusedGroups,
+          intellisense,
+          'Named group',
+        );
+
         intellisense.context.report({
           message: toEncodedMessage(
             'Use the named groups of this regex or remove the names.',
-            unusedGroups.map(grp => ({
-              loc: getRegexpLocation(regex.node, grp.node, intellisense.context),
-            })),
-            unusedGroups.map(grp => `Named group '${grp.name}'`),
+            locations,
+            messages,
           ),
           node: regex.node,
         });
@@ -253,22 +269,47 @@ function checkUnusedGroups(intellisense: RegexIntelliSense) {
   });
 }
 
+function prepareSecondaries(
+  regex: RegexKnowledge,
+  groups: GroupKnowledge[],
+  intellisense: RegexIntelliSense,
+  label: string,
+) {
+  const locations: LocationHolder[] = [];
+  const messages: string[] = [];
+  for (const grp of groups) {
+    const loc: AST.SourceLocation | null = getRegexpLocation(
+      regex.node,
+      grp.node,
+      intellisense.context,
+    );
+    if (loc) {
+      locations.push({ loc });
+      messages.push(`${label} '${grp.name}'`);
+    }
+  }
+  return { locations, messages };
+}
+
 function checkIndexedGroups(intellisense: RegexIntelliSense) {
   intellisense.getKnowledge().forEach(regex => {
-    regex.groups.forEach(group =>
+    regex.groups.forEach(group => {
+      const { locations, messages } = prepareSecondaries(regex, [group], intellisense, 'Group');
+
       group.node.references.forEach(reference => {
-        if (typeof reference.ref === 'number') {
+        const loc = getRegexpLocation(regex.node, reference, intellisense.context);
+        if (loc && typeof reference.ref === 'number') {
           intellisense.context.report({
             message: toEncodedMessage(
               `Directly use '${group.name}' instead of its group number.`,
-              [{ loc: getRegexpLocation(regex.node, group.node, intellisense.context) }],
-              [`Group '${group.name}'`],
+              locations,
+              messages,
             ),
-            loc: getRegexpLocation(regex.node, reference, intellisense.context),
+            loc,
           });
         }
-      }),
-    );
+      });
+    });
   });
 }
 

packages/jsts/src/rules/S5867/rule.ts

@@ -21,8 +21,7 @@
 
 import { Rule } from 'eslint';
 import { Character, Quantifier, RegExpLiteral } from '@eslint-community/regexpp/ast';
-import { SourceLocation } from 'estree';
-import { toEncodedMessage } from '../helpers';
+import { LocationHolder, toEncodedMessage } from '../helpers';
 import { createRegExpRule, getRegexpLocation } from '../helpers/regex';
 import { SONAR_RUNTIME } from '../../linter/parameters';
 
@@ -123,17 +122,21 @@ export const rule: Rule.RuleModule = createRegExpRule(
       },
       onRegExpLiteralLeave: (regexp: RegExpLiteral) => {
         if (!isUnicodeEnabled && (unicodeProperties.length > 0 || unicodeCharacters.length > 0)) {
-          const secondaryLocations: { loc: SourceLocation }[] = [];
+          const secondaryLocations: LocationHolder[] = [];
           const secondaryMessages: string[] = [];
           unicodeProperties.forEach(p => {
-            secondaryLocations.push({
-              loc: getRegexpLocation(context.node, p.character, context, [0, p.offset]),
-            });
-            secondaryMessages.push('Unicode property');
+            const loc = getRegexpLocation(context.node, p.character, context, [0, p.offset]);
+            if (loc) {
+              secondaryLocations.push({ loc });
+              secondaryMessages.push('Unicode property');
+            }
           });
           unicodeCharacters.forEach(c => {
-            secondaryLocations.push({ loc: getRegexpLocation(context.node, c, context) });
-            secondaryMessages.push('Unicode character');
+            const loc = getRegexpLocation(context.node, c, context);
+            if (loc) {
+              secondaryLocations.push({ loc });
+              secondaryMessages.push('Unicode character');
+            }
           });
           context.reportRegExpNode({
             message: toEncodedMessage(

packages/jsts/src/rules/S5869/rule.ts

@@ -19,9 +19,9 @@
  */
 // https://sonarsource.github.io/rspec/#/rspec/S5869/javascript
 
-import { Rule } from 'eslint';
+import { AST, Rule } from 'eslint';
 import { CharacterClass, Flags, Node, RegExpLiteral } from '@eslint-community/regexpp/ast';
-import { toEncodedMessage } from '../helpers';
+import { LocationHolder, toEncodedMessage } from '../helpers';
 import {
   createRegExpRule,
   getRegexpLocation,
@@ -51,11 +51,24 @@ export const rule: Rule.RuleModule = createRegExpRule(
         });
         if (duplicates.size > 0) {
           const [primary, ...secondaries] = duplicates;
+          const secondaryLocations: LocationHolder[] = [];
+          const messages: string[] = [];
+          for (const secondary of secondaries) {
+            const loc: AST.SourceLocation | null = getRegexpLocation(
+              context.node,
+              secondary,
+              context,
+            );
+            if (loc) {
+              secondaryLocations.push({ loc });
+              messages.push('Additional duplicate');
+            }
+          }
           context.reportRegExpNode({
             message: toEncodedMessage(
               'Remove duplicates in this character class.',
-              secondaries.map(snd => ({ loc: getRegexpLocation(context.node, snd, context) })),
-              secondaries.map(_ => 'Additional duplicate'),
+              secondaryLocations,
+              messages,
             ),
             node: context.node,
             regexpNode: primary,

packages/jsts/src/rules/helpers/regex/location.ts

@@ -36,16 +36,22 @@ export function getRegexpLocation(
   regexpNode: regexpp.AST.Node,
   context: Rule.RuleContext,
   offset = [0, 0],
-): AST.SourceLocation {
+): AST.SourceLocation | null {
   let loc: AST.SourceLocation;
   if (isRegexLiteral(node) || isStringLiteral(node)) {
     const source = context.sourceCode;
     const [start] = node.range!;
     const [reStart, reEnd] = getRegexpRange(node, regexpNode);
-    loc = {
-      start: source.getLocFromIndex(start + reStart + offset[0]),
-      end: source.getLocFromIndex(start + reEnd + offset[1]),
-    };
+    const locationStart = start + reStart + offset[0];
+    const locationEnd = start + reEnd + offset[1];
+    if (locationStart === locationEnd) {
+      return null;
+    } else {
+      loc = {
+        start: source.getLocFromIndex(locationStart),
+        end: source.getLocFromIndex(locationEnd),
+      };
+    }
   } else {
     loc = node.loc!;
   }

packages/jsts/src/rules/helpers/regex/rule-template.ts

@@ -74,7 +74,9 @@ export function createRegExpRule(
       function reportRegExpNode(descriptor: RegexReportDescriptor) {
         const { node, regexpNode, offset = [0, 0], ...rest } = descriptor;
         const loc = getRegexpLocation(node, regexpNode, context, offset);
-        context.report({ ...rest, loc });
+        if (loc) {
+          context.report({ ...rest, loc });
+        }
       }
 
       function checkLiteral(literal: estree.Literal) {