Improve no-dupe-disjunctions by reordering alternatives (#281)

* Improve `no-dupe-disjunctions` by reordering alternatives * Don't ignore capturing groups
ota-meshi · Jul 28, 2021 · 61ae942 · 61ae942
1 parent 6ea7524
commit 61ae942
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 3 deletions.
diff --git a/lib/rules/no-dupe-disjunctions.ts b/lib/rules/no-dupe-disjunctions.ts
@@ -29,6 +29,7 @@ import {
 } from "regexp-ast-analysis"
 import { RegExpParser } from "regexpp"
 import { UsageOfPattern } from "../utils/get-usage-of-pattern"
+import { canReorder } from "../utils/reorder-alternatives"
 
 type ParentNode = Group | CapturingGroup | Pattern | LookaroundAssertion
 
@@ -482,6 +483,7 @@ function* findPrefixDuplicationNfa(
  */
 function* findDuplicationNfa(
     alternatives: Alternative[],
+    context: RegExpContext,
     { hasNothingAfter, parser, ignoreOverlap }: Options,
 ): Iterable<Result> {
     const previous: [NFA, boolean, Alternative][] = []
@@ -524,9 +526,28 @@ function* findDuplicationNfa(
                     yield { type: "Subset", alternative, others }
                     break
 
-                case SubsetRelation.leftSupersetOfRight:
-                    yield { type: "Superset", alternative, others }
+                case SubsetRelation.leftSupersetOfRight: {
+                    const reorder = canReorder(
+                        [alternative, ...others],
+                        context,
+                    )
+
+                    if (reorder) {
+                        // We are allowed to freely reorder the alternatives.
+                        // This means that we can reverse the order of our
+                        // alternatives to convert the superset into a subset.
+                        for (const other of others) {
+                            yield {
+                                type: "Subset",
+                                alternative: other,
+                                others: [alternative],
+                            }
+                        }
+                    } else {
+                        yield { type: "Superset", alternative, others }
+                    }
                     break
+                }
 
                 case SubsetRelation.none:
                 case SubsetRelation.unknown:
@@ -574,7 +595,7 @@ function* findDuplication(
 
     // NFA-based approach
     if (!options.noNfa) {
-        yield* findDuplicationNfa(alternatives, options)
+        yield* findDuplicationNfa(alternatives, context, options)
     }
 }
 

diff --git a/tests/lib/rules/no-dupe-disjunctions.ts b/tests/lib/rules/no-dupe-disjunctions.ts
@@ -365,6 +365,21 @@ tester.run("no-dupe-disjunctions", rule as any, {
         },
         {
             code: String(/\b(?:\d|foo|\w+)\b/),
+            errors: [
+                {
+                    message:
+                        "Unexpected useless alternative. This alternative is a strict subset of '\\w+' and can be removed.",
+                    column: 7,
+                },
+                {
+                    message:
+                        "Unexpected useless alternative. This alternative is a strict subset of '\\w+' and can be removed.",
+                    column: 10,
+                },
+            ],
+        },
+        {
+            code: String(/(?:\d|foo|\w+)a/),
             options: [{ report: "interesting" }],
             errors: [
                 "Unexpected superset. This alternative is a superset of '\\d|foo'. It might be possible to remove the other alternative(s).",