Fix FP S5860 (unused-named-groups): Consider accesses via the index…

… syntax (#3685)
SonarSource · Feb 7, 2023 · cb3db60 · cb3db60
1 parent 7cecf5d
commit cb3db60
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 11 deletions.
diff --git a/eslint-bridge/src/linting/eslint/rules/helpers/ast.ts b/eslint-bridge/src/linting/eslint/rules/helpers/ast.ts
@@ -564,6 +564,12 @@ export function isDotNotation(
   return node.type === 'MemberExpression' && !node.computed && node.property.type === 'Identifier';
 }
 
+export function isIndexNotation(
+  node: estree.Node,
+): node is estree.MemberExpression & { property: StringLiteral } {
+  return node.type === 'MemberExpression' && node.computed && isStringLiteral(node.property);
+}
+
 export function isObjectDestructuring(
   node: estree.Node,
 ): node is

diff --git a/eslint-bridge/src/linting/eslint/rules/unused-named-groups.ts b/eslint-bridge/src/linting/eslint/rules/unused-named-groups.ts
@@ -29,6 +29,7 @@ import {
   getValueOfExpression,
   getVariableFromName,
   isDotNotation,
+  isIndexNotation,
   isMethodCall,
   isObjectDestructuring,
   isRequiredParserServices,
@@ -166,13 +167,14 @@ function checkNonExistingGroupReference(
     /* matcher.groups.<name> / matcher.indices.groups.<name>  */
     const groupNodes = extractGroupNodes(memberExpr, intellisense);
     for (const groupNode of groupNodes) {
-      const group = regex.groups.find(grp => grp.name === groupNode.name);
+      const groupName = groupNode.type === 'Identifier' ? groupNode.name : groupNode.value;
+      const group = regex.groups.find(grp => grp.name === groupName);
       if (group) {
         group.used = true;
       } else {
         intellisense.context.report({
           message: toEncodedMessage(
-            `There is no group named '${groupNode.name}' in the regular expression.`,
+            `There is no group named '${groupName}' in the regular expression.`,
             regex.groups.map(grp => ({
               loc: getRegexpLocation(regex.node, grp.node, intellisense.context),
             })),
@@ -185,10 +187,7 @@ function checkNonExistingGroupReference(
   }
 }
 
-function extractGroupNodes(
-  memberExpr: estree.MemberExpression,
-  intellisense: RegexIntelliSense,
-): estree.Identifier[] {
+function extractGroupNodes(memberExpr: estree.MemberExpression, intellisense: RegexIntelliSense) {
   if (isDotNotation(memberExpr)) {
     const { property } = memberExpr;
     const ancestors = intellisense.context.getAncestors();
@@ -199,10 +198,10 @@ function extractGroupNodes(
     if (parent) {
       switch (property.name) {
         case 'groups':
-          /* matcher.groups.<name> */
+          /* matcher.groups.<name> or matcher.groups['name'] */
           return extractNamedOrDestructuredGroupNodes(parent);
         case 'indices':
-          /* matcher.indices.groups.<name> */
+          /* matcher.indices.groups.<name> or matcher.indices.groups['name'] */
           if (isDotNotation(parent) && parent.property.name === 'groups') {
             parent = ancestors.pop();
             if (parent) {
@@ -215,9 +214,9 @@ function extractGroupNodes(
   return [];
 }
 
-function extractNamedOrDestructuredGroupNodes(node: estree.Node): estree.Identifier[] {
-  if (isDotNotation(node)) {
-    /* matcher.groups.<name> */
+function extractNamedOrDestructuredGroupNodes(node: estree.Node) {
+  if (isDotNotation(node) || isIndexNotation(node)) {
+    /* matcher.groups.<name> or matcher.groups['name'] */
     return [node.property];
   } else if (isObjectDestructuring(node)) {
     /* { <name1>,..<nameN> } = matcher.groups */

diff --git a/eslint-bridge/tests/linting/eslint/rules/unused-named-groups.test.ts b/eslint-bridge/tests/linting/eslint/rules/unused-named-groups.test.ts
@@ -194,6 +194,24 @@ typeAwareRuleTester.run('Regular expressions named groups should be used', rule,
       }
       `,
     },
+    {
+      code: `
+        const pattern = /(?<foo>\\w)/;
+        const matched = 'str'.matchAll(pattern);
+        if (matched) {
+          matched.groups['foo'];
+        }
+      `,
+    },
+    {
+      code: `
+        const pattern = /(?<foo>\\w)/;
+        const matched = pattern.exec('str');
+        if (matched) {
+          matched.indices.groups['foo'];
+        }
+      `,
+    },
   ],
   invalid: [
     {
@@ -552,6 +570,26 @@ typeAwareRuleTester.run('Regular expressions named groups should be used', rule,
       `,
       errors: 2,
     },
+    {
+      code: `
+        const pattern = /(?<foo>\\w)(?<bar>\\w)/; // Noncompliant: unused 'foo'
+        const matched = 'str'.match(pattern);
+        if (matched) {
+          matched.groups['bar'];
+        }
+      `,
+      errors: 1,
+    },
+    {
+      code: `
+        const pattern = /(?<foo>\\w)(?<bar>\\w)/; // Noncompliant: unused 'foo'
+        const matched = pattern.exec('str');
+        if (matched) {
+          matched.indices.groups['bar'];
+        }
+      `,
+      errors: 1,
+    },
   ],
 });