Add support for v flag to regexp/no-dupe-disjunctions (#612)

RunDevelopment · web-flow · commit b59ce50f2d9c · 2023-09-22T10:57:15.000+09:00
* Add support for `v` flag to `regexp/no-dupe-disjunctions`

* Create beige-fireants-dress.md
diff --git a/.changeset/beige-fireants-dress.md b/.changeset/beige-fireants-dress.md
@@ -0,0 +1,5 @@
+---
+"eslint-plugin-regexp": patch
+---
+
+Add support for `v` flag to `regexp/no-dupe-disjunctions`
diff --git a/lib/rules/no-dupe-disjunctions.ts b/lib/rules/no-dupe-disjunctions.ts
@@ -2,6 +2,7 @@ import type { RegExpVisitor } from "@eslint-community/regexpp/visitor"
 import type {
     Alternative,
     CapturingGroup,
+    CharacterClassElement,
     Group,
     LookaroundAssertion,
     Node,
@@ -15,6 +16,7 @@ import {
     fixRemoveCharacterClassElement,
     fixRemoveAlternative,
     assertValidFlags,
+    fixRemoveStringAlternative,
 } from "../utils"
 import { getParser, isCoveredNode, isEqualNodes } from "../utils/regexp-ast"
 import type { Expression, FiniteAutomaton, NoParent, ReadonlyNFA } from "refa"
@@ -221,21 +223,46 @@ function* iterateNestedAlternatives(
 
         if (e.type === "CharacterClass" && !e.negate) {
             const nested: NestedAlternative[] = []
-            for (const charElement of e.elements) {
-                if (charElement.type === "CharacterClassRange") {
-                    const min = charElement.min
-                    const max = charElement.max
-                    if (min.value === max.value) {
+
+            // eslint-disable-next-line func-style -- x
+            const addToNested = (charElement: CharacterClassElement) => {
+                switch (charElement.type) {
+                    case "CharacterClassRange": {
+                        const min = charElement.min
+                        const max = charElement.max
+                        if (min.value === max.value) {
+                            nested.push(charElement)
+                        } else if (min.value + 1 === max.value) {
+                            nested.push(min, max)
+                        } else {
+                            nested.push(charElement, min, max)
+                        }
+                        break
+                    }
+                    case "ClassStringDisjunction": {
+                        nested.push(...charElement.alternatives)
+                        break
+                    }
+                    case "CharacterClass": {
+                        if (!charElement.negate) {
+                            charElement.elements.forEach(addToNested)
+                        } else {
+                            nested.push(charElement)
+                        }
+                        break
+                    }
+                    case "Character":
+                    case "CharacterSet":
+                    case "ExpressionCharacterClass": {
                         nested.push(charElement)
-                    } else if (min.value + 1 === max.value) {
-                        nested.push(min, max)
-                    } else {
-                        nested.push(charElement, min, max)
+                        break
                     }
-                } else {
-                    nested.push(charElement)
+                    default:
+                        throw assertNever(charElement)
                 }
             }
+            e.elements.forEach(addToNested)
+
             if (nested.length > 1) yield* nested
         }
     }
@@ -825,7 +852,7 @@ function deduplicateResults(
 }
 
 function mentionNested(nested: NestedAlternative): string {
-    if (nested.type === "Alternative") {
+    if (nested.type === "Alternative" || nested.type === "StringAlternative") {
         return mention(nested)
     }
     return mentionChar(nested)
@@ -842,9 +869,15 @@ function fixRemoveNestedAlternative(
         case "Alternative":
             return fixRemoveAlternative(context, alternative)
 
+        case "StringAlternative":
+            return fixRemoveStringAlternative(context, alternative)
+
         case "Character":
         case "CharacterClassRange":
-        case "CharacterSet": {
+        case "CharacterSet":
+        case "CharacterClass":
+        case "ExpressionCharacterClass":
+        case "ClassStringDisjunction": {
             if (alternative.parent.type !== "CharacterClass") {
                 // This isn't supposed to happen. We can't just remove the only
                 // alternative of its parent
@@ -855,8 +888,6 @@ function fixRemoveNestedAlternative(
         }
 
         default:
-            // FIXME: TS Error
-            // @ts-expect-error -- FIXME
             throw assertNever(alternative)
     }
 }
diff --git a/lib/utils/index.ts b/lib/utils/index.ts
@@ -9,6 +9,7 @@ import type {
     Node,
     Pattern,
     Quantifier,
+    StringAlternative,
 } from "@eslint-community/regexpp/ast"
 import { RegExpParser, visitRegExpAST } from "@eslint-community/regexpp"
 import {
@@ -1206,14 +1207,13 @@ export function fixRemoveCharacterClassElement(
             return null
         }
 
+        const elements: CharacterClassElement[] = cc.elements
+        const elementIndex = elements.indexOf(element)
+
         const elementBefore: CharacterClassElement | undefined =
-            // FIXME: TS Error
-            // @ts-expect-error -- FIXME
-            cc.elements[cc.elements.indexOf(element) - 1]
+            cc.elements[elementIndex - 1]
         const elementAfter: CharacterClassElement | undefined =
-            // FIXME: TS Error
-            // @ts-expect-error -- FIXME
-            cc.elements[cc.elements.indexOf(element) + 1]
+            cc.elements[elementIndex + 1]
 
         if (
             elementBefore &&
@@ -1275,6 +1275,32 @@ export function fixRemoveAlternative(
     })
 }
 
+export function fixRemoveStringAlternative(
+    context: RegExpContext,
+    alternative: StringAlternative,
+): (fixer: Rule.RuleFixer) => Rule.Fix | null {
+    const { parent } = alternative
+    if (parent.alternatives.length === 1) {
+        // We have to remove the string disjunction instead.
+        // We replace it with `[]` to avoid situations like `[&\q{a}&]` -> `[&&]`
+        return context.fixReplaceNode(parent, "[]")
+    }
+
+    return context.fixReplaceNode(parent, () => {
+        let { start, end } = alternative
+        if (parent.alternatives[0] === alternative) {
+            end++
+        } else {
+            start--
+        }
+
+        const before = parent.raw.slice(0, start - parent.start)
+        const after = parent.raw.slice(end - parent.start)
+
+        return before + after
+    })
+}
+
 /**
  * Check the siblings to see if the regex doesn't change when unwrapped.
  */
diff --git a/lib/utils/partial-parser.ts b/lib/utils/partial-parser.ts
@@ -9,9 +9,15 @@ import { JS } from "refa"
 import type { AST } from "@eslint-community/regexpp"
 import { assertNever } from "./util"
 
-export type NestedAlternative = AST.Alternative | AST.CharacterClassElement
+export type NestedAlternative =
+    | AST.Alternative
+    | AST.CharacterClassElement
+    | AST.StringAlternative
 
 class Context {
+    /**
+     * The ancestor nodes of the alternative + the alternative itself.
+     */
     public readonly ancestors: ReadonlySet<AST.Node>
 
     public readonly alternative: NestedAlternative
@@ -27,15 +33,14 @@ class Context {
     }
 }
 
+type Parsable = JS.ParsableElement | AST.CharacterClassElement
+
 export class PartialParser {
     private readonly parser: JS.Parser
 
     private readonly options: JS.ParseOptions
 
-    private readonly nativeCache = new WeakMap<
-        JS.ParsableElement,
-        NoParent<Element>
-    >()
+    private readonly nativeCache = new WeakMap<Parsable, NoParent<Element>>()
 
     public constructor(parser: JS.Parser, options: JS.ParseOptions = {}) {
         this.parser = parser
@@ -117,8 +122,34 @@ export class PartialParser {
         }
     }
 
+    private parseStringAlternatives(
+        alternatives: readonly AST.StringAlternative[],
+        context: Context,
+    ): NoParent<Concatenation>[] {
+        const ancestor = alternatives.find((a) => context.ancestors.has(a))
+        if (ancestor) {
+            return [this.parseStringAlternative(ancestor)]
+        }
+
+        return alternatives.map((a) => this.parseStringAlternative(a))
+    }
+
+    private parseStringAlternative(
+        alternative: AST.StringAlternative,
+    ): NoParent<Concatenation> {
+        return {
+            type: "Concatenation",
+            elements: alternative.elements.map((e) =>
+                this.nativeParseElement(e),
+            ),
+        }
+    }
+
     private parseElement(
-        element: AST.Element,
+        element:
+            | AST.Element
+            | AST.CharacterClassRange
+            | AST.ClassStringDisjunction,
         context: Context,
     ): NoParent<Element> {
         if (!context.ancestors.has(element)) {
@@ -130,11 +161,29 @@ export class PartialParser {
             case "Backreference":
             case "Character":
             case "CharacterSet":
+            case "ExpressionCharacterClass":
+                return this.nativeParseElement(element)
+
+            case "CharacterClassRange":
+                if (context.alternative === element.min) {
+                    return this.nativeParseElement(context.alternative)
+                } else if (context.alternative === element.max) {
+                    return this.nativeParseElement(context.alternative)
+                }
                 return this.nativeParseElement(element)
 
             case "CharacterClass":
                 return this.parseCharacterClass(element, context)
 
+            case "ClassStringDisjunction":
+                return {
+                    type: "Alternation",
+                    alternatives: this.parseStringAlternatives(
+                        element.alternatives,
+                        context,
+                    ),
+                }
+
             case "Group":
             case "CapturingGroup":
                 return {
@@ -175,8 +224,6 @@ export class PartialParser {
             }
 
             default:
-                // FIXME: TS Error
-                // @ts-expect-error -- FIXME
                 throw assertNever(element)
         }
     }
@@ -193,22 +240,17 @@ export class PartialParser {
             return this.nativeParseElement(cc)
         }
 
-        if (context.alternative.type === "CharacterClassRange") {
-            const range: CharRange = {
-                min: context.alternative.min.value,
-                max: context.alternative.max.value,
-            }
-            return {
-                type: "CharacterClass",
-                characters: JS.createCharSet([range], this.parser.flags),
+        for (const e of cc.elements) {
+            if (context.ancestors.has(e)) {
+                return this.parseElement(e, context)
             }
         }
-        // FIXME: TS Error
-        // @ts-expect-error -- FIXME
-        return this.nativeParseElement(context.alternative)
+
+        // this means that cc === context.alternative
+        return this.nativeParseElement(cc)
     }
 
-    private nativeParseElement(element: JS.ParsableElement): NoParent<Element> {
+    private nativeParseElement(element: Parsable): NoParent<Element> {
         let cached = this.nativeCache.get(element)
         if (!cached) {
             cached = this.nativeParseElementUncached(element)
@@ -217,9 +259,25 @@ export class PartialParser {
         return cached
     }
 
-    private nativeParseElementUncached(
-        element: JS.ParsableElement,
-    ): NoParent<Element> {
+    private nativeParseElementUncached(element: Parsable): NoParent<Element> {
+        if (element.type === "CharacterClassRange") {
+            const range: CharRange = {
+                min: element.min.value,
+                max: element.max.value,
+            }
+            return {
+                type: "CharacterClass",
+                characters: JS.createCharSet([range], this.parser.flags),
+            }
+        } else if (element.type === "ClassStringDisjunction") {
+            return {
+                type: "Alternation",
+                alternatives: element.alternatives.map((a) =>
+                    this.parseStringAlternative(a),
+                ),
+            }
+        }
+
         const { expression } = this.parser.parseElement(element, this.options)
 
         if (expression.alternatives.length === 1) {
diff --git a/tests/lib/rules/no-dupe-disjunctions.ts b/tests/lib/rules/no-dupe-disjunctions.ts
@@ -3,7 +3,7 @@ import rule from "../../../lib/rules/no-dupe-disjunctions"
 
 const tester = new RuleTester({
     parserOptions: {
-        ecmaVersion: 2020,
+        ecmaVersion: "latest",
         sourceType: "module",
     },
 })
@@ -19,6 +19,7 @@ tester.run("no-dupe-disjunctions", rule as any, {
         `/(?:js|json)?abc/`,
         `/<("[^"]*"|'[^']*'|[^'">])*>/g`,
         `/c+|[a-f]/`,
+        `/c+|[a-f]/v`,
         String.raw`/b+(?:\w+|[+-]?\d+)/`,
         String.raw`/\d*\.\d+_|\d+\.\d*_/`,
         String.raw`/\d*\.\d+|\d+\.\d*/`,
@@ -149,6 +150,12 @@ tester.run("no-dupe-disjunctions", rule as any, {
                 "Unexpected duplicate alternative. This alternative can be removed.",
             ],
         },
+        {
+            code: `/((?:ab|ba)|(?:ab|ba))/v`,
+            errors: [
+                "Unexpected duplicate alternative. This alternative can be removed.",
+            ],
+        },
         {
             code: `/((?:ab|ba)|(?:ba|ab))/`,
             errors: [
@@ -514,6 +521,12 @@ tester.run("no-dupe-disjunctions", rule as any, {
                 "Unexpected useless alternative. This alternative is a strict subset of 'A*' and can be removed.",
             ],
         },
+        {
+            code: String.raw`/[\q{a|bb}]|bb/v`,
+            errors: [
+                "Unexpected useless alternative. This alternative is a strict subset of '[\\q{a|bb}]' and can be removed.",
+            ],
+        },
         {
             // reportExponentialBacktracking: 'potential'
             code: String.raw`
