Skip to content

Commit e040e79

Browse files
ota-meshiota-meshi
authored
Improve prefer-lookaround rule to report when there are leading/trailing assertions (#480)
1 parent 8ceed16 commit e040e79

File tree

2 files changed

+579
-44
lines changed

2 files changed

+579
-44
lines changed

‎lib/rules/prefer-lookaround.ts

Lines changed: 216 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import type { RegExpVisitor } from "regexpp/visitor"
2-
import type { CapturingGroup, Element } from "regexpp/ast"
2+
import type {
3+
CapturingGroup,
4+
Element,
5+
LookaroundAssertion,
6+
Pattern,
7+
} from "regexpp/ast"
38
import type { RegExpContext } from "../utils"
49
import { createRule, defineRegexpVisitor } from "../utils"
510
import { createTypeTracker } from "../utils/type-tracker"
@@ -11,7 +16,10 @@ import {
1116
extractExpressionReferences,
1217
isKnownMethodCall,
1318
} from "../utils/ast-utils"
14-
import type { PatternReplaceRange } from "../utils/ast-utils/pattern-source"
19+
import type {
20+
PatternRange,
21+
PatternReplaceRange,
22+
} from "../utils/ast-utils/pattern-source"
1523
import type { Expression, Literal } from "estree"
1624
import type { Rule } from "eslint"
1725
import { mention } from "../utils/mention"
@@ -172,6 +180,184 @@ function getSideEffectsWhenReplacingCapturingGroup(
172180
}
173181
}
174182

183+
/** Checks whether the given element is a capturing group of length 1 or greater. */
184+
function isCapturingGroupAndNotZeroLength(
185+
element: Element,
186+
): element is CapturingGroup {
187+
return element.type === "CapturingGroup" && !isZeroLength(element)
188+
}
189+
190+
type ParsedStartPattern = {
191+
// A list of zero-length elements placed before the start capturing group.
192+
// e.g.
193+
// /^(foo)bar/ -> ^
194+
// /\b(foo)bar/ -> \b
195+
// /(?:^|\b)(foo)bar/ -> (?:^|\b)
196+
// /(?<=f)(oo)bar/ -> (?<=f)
197+
// /(foo)bar/ -> null
198+
leadingElements: Element[]
199+
// Capturing group used to replace the starting string.
200+
capturingGroup: CapturingGroup
201+
// The pattern used when replacing lookbehind assertions.
202+
replacedAssertion: string
203+
range: PatternRange
204+
}
205+
type ParsedEndPattern = {
206+
// Capturing group used to replace the ending string.
207+
capturingGroup: CapturingGroup
208+
// A list of zero-length elements placed after the end capturing group.
209+
// e.g.
210+
// /foo(bar)$/ -> $
211+
// /foo(bar)\b/ -> \b
212+
// /foo(bar)(?:\b|$)/ -> (?:\b|$)
213+
// /foo(ba)(?=r)/ -> (?=r)
214+
// /foo(bar)/ -> null
215+
trailingElements: Element[]
216+
// The pattern used when replacing lookahead assertions.
217+
replacedAssertion: string
218+
range: PatternRange
219+
}
220+
type ParsedElements = {
221+
// All elements
222+
elements: readonly Element[]
223+
start: ParsedStartPattern | null
224+
end: ParsedEndPattern | null
225+
}
226+
227+
/**
228+
* Parse the elements of the pattern.
229+
*/
230+
function parsePatternElements(node: Pattern): ParsedElements | null {
231+
if (node.alternatives.length > 1) {
232+
return null
233+
}
234+
const elements = node.alternatives[0].elements
235+
const leadingElements: Element[] = []
236+
let start: ParsedStartPattern | null = null
237+
238+
for (const element of elements) {
239+
if (isZeroLength(element)) {
240+
leadingElements.push(element)
241+
continue
242+
}
243+
if (isCapturingGroupAndNotZeroLength(element)) {
244+
const capturingGroup = element
245+
start = {
246+
leadingElements,
247+
capturingGroup,
248+
replacedAssertion: startElementsToLookbehindAssertionText(
249+
leadingElements,
250+
capturingGroup,
251+
),
252+
range: {
253+
start: (leadingElements[0] || capturingGroup).start,
254+
end: capturingGroup.end,
255+
},
256+
}
257+
}
258+
break
259+
}
260+
261+
let end: ParsedEndPattern | null = null
262+
const trailingElements: Element[] = []
263+
for (const element of [...elements].reverse()) {
264+
if (isZeroLength(element)) {
265+
trailingElements.unshift(element)
266+
continue
267+
}
268+
269+
if (isCapturingGroupAndNotZeroLength(element)) {
270+
const capturingGroup = element
271+
end = {
272+
capturingGroup,
273+
trailingElements,
274+
replacedAssertion: endElementsToLookaheadAssertionText(
275+
capturingGroup,
276+
trailingElements,
277+
),
278+
range: {
279+
start: capturingGroup.start,
280+
end: (
281+
trailingElements[trailingElements.length - 1] ||
282+
capturingGroup
283+
).end,
284+
},
285+
}
286+
}
287+
break
288+
}
289+
if (!start && !end) {
290+
// No capturing groups.
291+
return null
292+
}
293+
if (start && end && start.capturingGroup === end.capturingGroup) {
294+
// There is only one capturing group.
295+
return null
296+
}
297+
298+
return {
299+
elements,
300+
start,
301+
end,
302+
}
303+
}
304+
305+
/** Convert end capturing group to lookahead assertion text. */
306+
function endElementsToLookaheadAssertionText(
307+
capturingGroup: CapturingGroup,
308+
trailingElements: Element[],
309+
): string {
310+
const groupPattern = capturingGroup.alternatives.map((a) => a.raw).join("|")
311+
312+
const trailing = leadingTrailingElementsToLookaroundAssertionPatternText(
313+
trailingElements,
314+
"lookahead",
315+
)
316+
if (trailing && capturingGroup.alternatives.length !== 1) {
317+
return `(?=(?:${groupPattern})${trailing})`
318+
}
319+
return `(?=${groupPattern}${trailing})`
320+
}
321+
322+
/** Convert start capturing group to lookbehind assertion text. */
323+
function startElementsToLookbehindAssertionText(
324+
leadingElements: Element[],
325+
capturingGroup: CapturingGroup,
326+
): string {
327+
const leading = leadingTrailingElementsToLookaroundAssertionPatternText(
328+
leadingElements,
329+
"lookbehind",
330+
)
331+
const groupPattern = capturingGroup.alternatives.map((a) => a.raw).join("|")
332+
if (leading && capturingGroup.alternatives.length !== 1) {
333+
return `(?<=${leading}(?:${groupPattern}))`
334+
}
335+
return `(?<=${leading}${groupPattern})`
336+
}
337+
338+
/** Convert leading/trailing elements to lookaround assertion pattern text. */
339+
function leadingTrailingElementsToLookaroundAssertionPatternText(
340+
leadingTrailingElements: Element[],
341+
lookaroundAssertionKind: LookaroundAssertion["kind"],
342+
): string {
343+
if (
344+
leadingTrailingElements.length === 1 &&
345+
leadingTrailingElements[0].type === "Assertion"
346+
) {
347+
const assertion = leadingTrailingElements[0]
348+
if (
349+
assertion.kind === lookaroundAssertionKind &&
350+
!assertion.negate &&
351+
assertion.alternatives.length === 1
352+
) {
353+
// If the leading/trailing assertion is simple (single alternative, and positive) lookaround assertion, unwrap the parens.
354+
return assertion.alternatives[0].raw
355+
}
356+
}
357+
358+
return leadingTrailingElements.map((e) => e.raw).join("")
359+
}
360+
175361
/**
176362
* Parse option
177363
*/
@@ -230,10 +416,8 @@ export default createRule("prefer-lookaround", {
230416
regexpContext: RegExpContext,
231417
): RegExpVisitor.Handlers {
232418
const { regexpNode, patternAst } = regexpContext
233-
if (
234-
patternAst.alternatives.length > 1 ||
235-
patternAst.alternatives[0].elements.length < 2
236-
) {
419+
const parsedElements = parsePatternElements(patternAst)
420+
if (!parsedElements) {
237421
return {}
238422
}
239423
const replaceReferenceList: ReplaceReferences[] = []
@@ -297,6 +481,7 @@ export default createRule("prefer-lookaround", {
297481
}
298482
return createVerifyVisitor(
299483
regexpContext,
484+
parsedElements,
300485
new ReplaceReferencesList(replaceReferenceList),
301486
)
302487
}
@@ -408,6 +593,7 @@ export default createRule("prefer-lookaround", {
408593
*/
409594
function createVerifyVisitor(
410595
regexpContext: RegExpContext,
596+
parsedElements: ParsedElements,
411597
replaceReferenceList: ReplaceReferencesList,
412598
): RegExpVisitor.Handlers {
413599
type RefState = {
@@ -457,43 +643,29 @@ export default createRule("prefer-lookaround", {
457643
}
458644
}
459645
},
460-
onPatternLeave(pNode) {
646+
onPatternLeave() {
461647
// verify
462-
const alt = pNode.alternatives[0]
463648
let reportStart = null
464649
if (
465650
!startRefState.isUseOther &&
466651
startRefState.capturingGroups.length === 1 && // It will not be referenced from more than one, but check it just in case.
467-
startRefState.capturingGroups[0] === alt.elements[0] &&
468-
!isZeroLength(startRefState.capturingGroups[0])
652+
startRefState.capturingGroups[0] ===
653+
parsedElements.start?.capturingGroup
469654
) {
470-
const capturingGroup = startRefState.capturingGroups[0]
471-
reportStart = {
472-
capturingGroup,
473-
expr: `(?<=${capturingGroup.alternatives
474-
.map((a) => a.raw)
475-
.join("|")})`,
476-
}
655+
reportStart = parsedElements.start
477656
}
478657
let reportEnd = null
479658
if (
480659
!endRefState.isUseOther &&
481660
endRefState.capturingGroups.length === 1 && // It will not be referenced from more than one, but check it just in case.
482661
endRefState.capturingGroups[0] ===
483-
alt.elements[alt.elements.length - 1] &&
484-
!isZeroLength(endRefState.capturingGroups[0])
662+
parsedElements.end?.capturingGroup
485663
) {
486-
const capturingGroup = endRefState.capturingGroups[0]
487-
reportEnd = {
488-
capturingGroup,
489-
expr: `(?=${capturingGroup.alternatives
490-
.map((a) => a.raw)
491-
.join("|")})`,
492-
}
664+
reportEnd = parsedElements.end
493665
}
494666
const sideEffects =
495667
getSideEffectsWhenReplacingCapturingGroup(
496-
alt.elements,
668+
parsedElements.elements,
497669
reportStart?.capturingGroup,
498670
reportEnd?.capturingGroup,
499671
regexpContext,
@@ -530,12 +702,14 @@ export default createRule("prefer-lookaround", {
530702
for (const report of [reportStart, reportEnd]) {
531703
context.report({
532704
loc: regexpContext.getRegexpLocation(
533-
report.capturingGroup,
705+
report.range,
534706
),
535707
messageId: "preferLookarounds",
536708
data: {
537-
expr1: mention(reportStart.expr),
538-
expr2: mention(reportEnd.expr),
709+
expr1: mention(
710+
reportStart.replacedAssertion,
711+
),
712+
expr2: mention(reportEnd.replacedAssertion),
539713
},
540714
fix,
541715
})
@@ -559,12 +733,12 @@ export default createRule("prefer-lookaround", {
559733
)
560734
context.report({
561735
loc: regexpContext.getRegexpLocation(
562-
reportStart.capturingGroup,
736+
reportStart.range,
563737
),
564738
messageId: "prefer",
565739
data: {
566740
kind: "lookbehind assertion",
567-
expr: mention(reportStart.expr),
741+
expr: mention(reportStart.replacedAssertion),
568742
},
569743
fix,
570744
})
@@ -595,12 +769,12 @@ export default createRule("prefer-lookaround", {
595769
)
596770
context.report({
597771
loc: regexpContext.getRegexpLocation(
598-
reportEnd.capturingGroup,
772+
reportEnd.range,
599773
),
600774
messageId: "prefer",
601775
data: {
602776
kind: "lookahead assertion",
603-
expr: mention(reportEnd.expr),
777+
expr: mention(reportEnd.replacedAssertion),
604778
},
605779
fix,
606780
})
@@ -614,10 +788,7 @@ export default createRule("prefer-lookaround", {
614788
*/
615789
function buildFixer(
616790
regexpContext: RegExpContext,
617-
replaceCapturingGroups: {
618-
capturingGroup: CapturingGroup
619-
expr: string
620-
}[],
791+
replaceCapturingGroups: (ParsedStartPattern | ParsedEndPattern)[],
621792
replaceReferenceList: ReplaceReferencesList,
622793
getRemoveRanges: (
623794
replaceReference: ReplaceReferences,
@@ -638,17 +809,17 @@ export default createRule("prefer-lookaround", {
638809
}
639810
const replaces: {
640811
replaceRange: PatternReplaceRange
641-
expr: string
812+
replacedAssertion: string
642813
}[] = []
643-
for (const { capturingGroup, expr } of replaceCapturingGroups) {
814+
for (const { range, replacedAssertion } of replaceCapturingGroups) {
644815
const replaceRange =
645-
regexpContext.patternSource.getReplaceRange(capturingGroup)
816+
regexpContext.patternSource.getReplaceRange(range)
646817
if (!replaceRange) {
647818
return null
648819
}
649820
replaces.push({
650821
replaceRange,
651-
expr,
822+
replacedAssertion,
652823
})
653824
}
654825

@@ -660,10 +831,11 @@ export default createRule("prefer-lookaround", {
660831
fix: () => fixer.removeRange(removeRange),
661832
})
662833
}
663-
for (const { replaceRange, expr } of replaces) {
834+
for (const { replaceRange, replacedAssertion } of replaces) {
664835
list.push({
665836
offset: replaceRange.range[0],
666-
fix: () => replaceRange.replace(fixer, expr),
837+
fix: () =>
838+
replaceRange.replace(fixer, replacedAssertion),
667839
})
668840
}
669841
return list

0 commit comments

Comments
 (0)