diff --git a/.changeset/beige-fireants-dress.md b/.changeset/beige-fireants-dress.md new file mode 100644 index 000000000..17edd21d1 --- /dev/null +++ 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 index 09b7f2c15..fe4155357 100644 --- 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 index a550d6cfb..cb3eda10c 100644 --- 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 index 9817d020b..6986c7184 100644 --- 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 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 - >() + private readonly nativeCache = new WeakMap>() 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[] { + 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 { + 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 { 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 { + private nativeParseElement(element: Parsable): NoParent { 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 { + private nativeParseElementUncached(element: Parsable): NoParent { + 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 index 71521e0c1..cf0128d6f 100644 --- 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`