Avoid wrapping multiple layers of non-capture groups

hlysine · Aug 20, 2023 · 058a236 · 058a236
1 parent bdc6537
commit 058a236
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 32 deletions.
diff --git a/src/helper.ts b/src/helper.ts
@@ -1,5 +1,25 @@
 import { IncompleteToken, RegExpLiteral } from './types';
 
+export function escapeRegExp(text: string): string {
+  return text.replace(/[-[/\]{}()*+?.,\\^$|#\s]/g, '\\$&');
+}
+
+export const hexNumber = /^[0-9a-fA-F]+$/;
+
+export const octalNumber = /^[0-7]+$/;
+
+// octal escape sequences are not matched here because they should be wrapped in a character class
+export const negatableCharLiteral = /^(?:\\u[0-9a-fA-F]{4}|\\x[0-9a-fA-F]{2})$/;
+
+// last option refers to octal character or capture group backreference
+export const charLiteral = /^(?:\\u[0-9a-fA-F]{4}|\\x[0-9a-fA-F]{2}|\\\d{1,3})$/;
+
+export const captureName = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
+
+export const negatableTokens = /^\\[sdwb]$/;
+
+export const flagString = /^[gmiyusd]*$/;
+
 /**
  * Check whether a given value is a template strings array.
  * @param arg - The argument to check.
@@ -145,22 +165,14 @@ export function isBracketGroup(regExp: string): boolean {
   return false;
 }
 
-export function escapeRegExp(text: string): string {
-  return text.replace(/[-[/\]{}()*+?.,\\^$|#\s]/g, '\\$&');
+export function wrapIfNeeded(regExp: string): string {
+  if (regExp.length === 1) return regExp;
+  if (regExp.length === 2 && regExp.startsWith('\\')) return regExp;
+  if (charLiteral.test(regExp)) return regExp;
+  if (isCharacterClass(regExp)) return regExp;
+  if (isBracketGroup(regExp)) {
+    // need to wrap lookarounds because they are not directly quantifiable
+    if (!/^\((?:\?=|\?!|\?<=|\?<!)/.test(regExp)) return regExp;
+  }
+  return `(?:${regExp})`;
 }
-
-export const hexNumber = /^[0-9a-fA-F]+$/;
-
-export const octalNumber = /^[0-7]+$/;
-
-// octal escape sequences are not matched here because they should be wrapped in a character class
-export const negatableCharLiteral = /^(?:\\u[0-9a-fA-F]{4}|\\x[0-9a-fA-F]{2})$/;
-
-// last option refers to octal character or capture group backreference
-export const charLiteral = /^(?:\\u[0-9a-fA-F]{4}|\\x[0-9a-fA-F]{2}|\\\d{1,3})$/;
-
-export const captureName = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
-
-export const negatableTokens = /^\\[sdwb]$/;
-
-export const flagString = /^[gmiyusd]*$/;
diff --git a/src/modifiers/GroupModifier.ts b/src/modifiers/GroupModifier.ts
@@ -1,4 +1,5 @@
 import { RegExpModifier } from '../types';
+import { isBracketGroup } from '../helper';
 
 export enum GroupType {
   NonCapture = 'nonCapture',
@@ -18,7 +19,8 @@ export default class GroupModifier implements RegExpModifier {
   public modify(regExp: string): [string, string?] {
     switch (this.groupType) {
       case GroupType.NonCapture:
-        return [`(?:${regExp})`];
+        // do not wrap multiple layers of non-capturing groups
+        return [isBracketGroup(regExp) && regExp.startsWith('(?:') ? regExp : `(?:${regExp})`];
       case GroupType.PositiveLookahead:
         return [`(?=${regExp})`];
       case GroupType.NegativeLookahead:

diff --git a/src/modifiers/QuantityModifier.ts b/src/modifiers/QuantityModifier.ts
@@ -1,20 +1,8 @@
 import { RegExpModifier } from '../types';
-import { charLiteral, isBracketGroup, isCharacterClass } from '../helper';
+import { wrapIfNeeded } from '../helper';
 
 const NOT_QUANTIFIABLE = new Set(['^', '$', '\\b', '\\B']);
 
-function wrapIfNeeded(regExp: string): string {
-  if (regExp.length === 1) return regExp;
-  if (regExp.length === 2 && regExp.startsWith('\\')) return regExp;
-  if (charLiteral.test(regExp)) return regExp;
-  if (isCharacterClass(regExp)) return regExp;
-  if (isBracketGroup(regExp)) {
-    // need to wrap lookarounds because they are not directly quantifiable
-    if (!/^\((?:\?=|\?!|\?<=|\?<!)/.test(regExp)) return regExp;
-  }
-  return `(?:${regExp})`;
-}
-
 export default abstract class QuantityModifier implements RegExpModifier {
   public lazy: boolean;
 

diff --git a/test/functions.test.ts b/test/functions.test.ts
@@ -759,6 +759,10 @@ describe('group', () => {
     expect(() => not.group`foo`.toString()).toThrow();
     expect(() => not(group`foo`).toString()).toThrow();
   });
+  it('does not wrap multiple times', () => {
+    expect(group.group.exactly`foo`.toString()).toBe('(?:foo)');
+    expect(group.oneOf`foo``bar`.toString()).toBe('(?:foo|bar)');
+  });
   it('throws for invalid argument', () => {
     expect(group().toString()).toBe('(?:)');
     // @ts-expect-error - testing invalid arguments