Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for ES2025 duplicate named capturing groups #752

Merged
merged 19 commits into from
Sep 12, 2024
5 changes: 5 additions & 0 deletions .changeset/odd-snakes-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-regexp": minor
---

Add support for ES2025 duplicate named capturing groups
9 changes: 3 additions & 6 deletions .github/workflows/NodeCI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,10 @@ jobs:
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- name: Install ESLint ${{ matrix.eslint }}
# We use --legacy-peer-deps because we get a lot of dependency warnings when installing eslint v9
run: npm i -D eslint@${{ matrix.eslint }} --legacy-peer-deps
- name: Install Packages
# run: npm ci
# We use `npm i` because there is an error regarding dependencies when installing eslint v9.
run: npm i -f
run: npm ci
- name: Install ESLint ${{ matrix.eslint }}
run: npm i -D eslint@${{ matrix.eslint }}
- name: Build
run: npm run build
- name: Test
Expand Down
86 changes: 73 additions & 13 deletions lib/rules/no-useless-backreference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ import type { RegExpContext } from "../utils"
import { createRule, defineRegexpVisitor } from "../utils"
import { mention } from "../utils/mention"

type MessageId =
| "nested"
| "disjunctive"
| "intoNegativeLookaround"
| "forward"
| "backward"
| "empty"

/**
* Returns whether the list of ancestors from `from` to `to` contains a negated
* lookaround.
Expand All @@ -35,16 +43,67 @@ function hasNegatedLookaroundInBetween(
return false
}

/**
* Returns the problem information specifying the reason why the backreference is
* useless.
*/
function getUselessProblem(
backRef: Backreference,
flags: ReadonlyFlags,
): { messageId: MessageId; group: CapturingGroup; otherGroups: string } | null {
const groups = [backRef.resolved].flat()

const problems: { messageId: MessageId; group: CapturingGroup }[] = []
for (const group of groups) {
const messageId = getUselessMessageId(backRef, group, flags)
if (!messageId) {
return null
}
problems.push({ messageId, group })
}
if (problems.length === 0) {
return null
}

let problemsToReport

// Gets problems that appear in the same disjunction.
const problemsInSameDisjunction = problems.filter(
(problem) => problem.messageId !== "disjunctive",
)

if (problemsInSameDisjunction.length) {
// Only report problems that appear in the same disjunction.
problemsToReport = problemsInSameDisjunction
} else {
// If all groups appear in different disjunctions, report it.
problemsToReport = problems
}

const [{ messageId, group }, ...other] = problemsToReport
let otherGroups = ""

if (other.length === 1) {
otherGroups = " and another group"
} else if (other.length > 1) {
otherGroups = ` and other ${other.length} groups`
}
return {
messageId,
group,
otherGroups,
}
}

/**
* Returns the message id specifying the reason why the backreference is
* useless.
*/
function getUselessMessageId(
backRef: Backreference,
group: CapturingGroup,
flags: ReadonlyFlags,
): string | null {
const group = backRef.resolved

): MessageId | null {
const closestAncestor = getClosestAncestor(backRef, group)

if (closestAncestor === group) {
Expand Down Expand Up @@ -93,16 +152,16 @@ export default createRule("no-useless-backreference", {
},
schema: [],
messages: {
nested: "Backreference {{ bref }} will be ignored. It references group {{ group }} from within that group.",
nested: "Backreference {{ bref }} will be ignored. It references group {{ group }}{{ otherGroups }} from within that group.",
forward:
"Backreference {{ bref }} will be ignored. It references group {{ group }} which appears later in the pattern.",
"Backreference {{ bref }} will be ignored. It references group {{ group }}{{ otherGroups }} which appears later in the pattern.",
backward:
"Backreference {{ bref }} will be ignored. It references group {{ group }} which appears before in the same lookbehind.",
"Backreference {{ bref }} will be ignored. It references group {{ group }}{{ otherGroups }} which appears before in the same lookbehind.",
disjunctive:
"Backreference {{ bref }} will be ignored. It references group {{ group }} which is in another alternative.",
"Backreference {{ bref }} will be ignored. It references group {{ group }}{{ otherGroups }} which is in another alternative.",
intoNegativeLookaround:
"Backreference {{ bref }} will be ignored. It references group {{ group }} which is in a negative lookaround.",
empty: "Backreference {{ bref }} will be ignored. It references group {{ group }} which always captures zero characters.",
"Backreference {{ bref }} will be ignored. It references group {{ group }}{{ otherGroups }} which is in a negative lookaround.",
empty: "Backreference {{ bref }} will be ignored. It references group {{ group }}{{ otherGroups }} which always captures zero characters.",
},
type: "suggestion", // "problem",
},
Expand All @@ -114,16 +173,17 @@ export default createRule("no-useless-backreference", {
}: RegExpContext): RegExpVisitor.Handlers {
return {
onBackreferenceEnter(backRef) {
const messageId = getUselessMessageId(backRef, flags)
const problem = getUselessProblem(backRef, flags)

if (messageId) {
if (problem) {
context.report({
node,
loc: getRegexpLocation(backRef),
messageId,
messageId: problem.messageId,
data: {
bref: mention(backRef),
group: mention(backRef.resolved),
group: mention(problem.group),
otherGroups: problem.otherGroups,
},
})
}
Expand Down
6 changes: 5 additions & 1 deletion lib/rules/prefer-named-backreference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ export default createRule("prefer-named-backreference", {
}: RegExpContext): RegExpVisitor.Handlers {
return {
onBackreferenceEnter(bNode) {
if (bNode.resolved.name && !bNode.raw.startsWith("\\k<")) {
if (
!bNode.ambiguous &&
bNode.resolved.name &&
!bNode.raw.startsWith("\\k<")
) {
context.report({
node,
loc: getRegexpLocation(bNode),
Expand Down
32 changes: 29 additions & 3 deletions lib/utils/regexp-ast/case-variation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type {
Alternative,
Backreference,
CapturingGroup,
CharacterClass,
CharacterClassElement,
CharacterSet,
Expand All @@ -15,6 +17,7 @@ import {
toCharSet,
isEmptyBackreference,
toUnicodeSet,
getClosestAncestor,
} from "regexp-ast-analysis"
import { assertNever, cachedFn } from "../util"

Expand Down Expand Up @@ -139,19 +142,28 @@ export function isCaseVariant(
// case-variant in Unicode mode
return unicodeLike && d.kind === "word"

case "Backreference":
case "Backreference": {
// we need to check whether the associated capturing group
// is case variant
if (hasSomeDescendant(element, d.resolved)) {

const outside = getReferencedGroupsFromBackreference(
d,
).filter(
(resolved) => !hasSomeDescendant(element, resolved),
)
if (outside.length === 0) {
// the capturing group is part of the root element, so
// we don't need to make an extra check
return false
}

return (
!isEmptyBackreference(d, flags) &&
isCaseVariant(d.resolved, flags)
outside.some((resolved) =>
isCaseVariant(resolved, flags),
)
)
}

case "Character":
case "CharacterClassRange":
Expand Down Expand Up @@ -179,3 +191,17 @@ export function isCaseVariant(
},
)
}

/**
* Returns the actually referenced capturing group from the given backreference.
*/
function getReferencedGroupsFromBackreference(
backRef: Backreference,
): CapturingGroup[] {
return [backRef.resolved].flat().filter((group) => {
const closestAncestor = getClosestAncestor(backRef, group)
return (
closestAncestor !== group && closestAncestor.type === "Alternative"
)
})
}
Loading
Loading