Skip to content

Commit c2723ce

Browse files
authored
Fix false positives for disallowNeverMatch option in regexp/no-dupe-disjunctions rule (#80)
1 parent 4dfddd0 commit c2723ce

File tree

4 files changed

+112
-1
lines changed

4 files changed

+112
-1
lines changed

lib/rules/no-dupe-disjunctions.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type {
55
Group,
66
LookaroundAssertion,
77
Pattern,
8+
Quantifier,
89
} from "regexpp/ast"
910
import { createRule, defineRegexpVisitor, getRegexpLocation } from "../utils"
1011
import { isCoveredNode, isEqualNodes } from "../utils/regexp-ast"
@@ -39,6 +40,35 @@ export default createRule("no-dupe-disjunctions", {
3940
)
4041
const sourceCode = context.getSourceCode()
4142

43+
/**
44+
* Check has after pattern
45+
*/
46+
function hasAfterPattern(
47+
node: Group | CapturingGroup | Pattern | LookaroundAssertion,
48+
): boolean {
49+
if (node.type === "Assertion") {
50+
return false
51+
}
52+
if (node.type === "Pattern") {
53+
return false
54+
}
55+
let target: Group | CapturingGroup | Quantifier = node
56+
let parent = target.parent
57+
while (parent) {
58+
if (parent.type === "Alternative") {
59+
const index = parent.elements.indexOf(target)
60+
return index < parent.elements.length - 1
61+
}
62+
if (parent.type === "Quantifier") {
63+
target = parent
64+
parent = target.parent
65+
continue
66+
}
67+
return false
68+
}
69+
return false
70+
}
71+
4272
/**
4373
* Create visitor
4474
* @param node
@@ -56,12 +86,15 @@ export default createRule("no-dupe-disjunctions", {
5686
| Pattern
5787
| LookaroundAssertion,
5888
) {
89+
const canOmitRight =
90+
disallowNeverMatch && !hasAfterPattern(regexpNode)
5991
const leftAlts = []
6092
for (const alt of regexpNode.alternatives) {
6193
const dupeAlt = disallowNeverMatch
6294
? leftAlts.find((leftAlt) =>
6395
isCoveredNode(leftAlt, alt, {
6496
flags: { left: flags, right: flags },
97+
canOmitRight,
6598
}),
6699
)
67100
: leftAlts.find((leftAlt) =>

lib/utils/regexp-ast/is-covered.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type Options = {
3232
left: string
3333
right: string
3434
}
35+
canOmitRight: boolean
3536
}
3637
interface NormalizedNodeBase {
3738
readonly type: string
@@ -777,6 +778,9 @@ function isCoveredAltNodes(
777778
right: NormalizedNode[],
778779
options: Options,
779780
) {
781+
// if (options.canOmitRight) {
782+
// return isCoveredAltNodesForCanOmitRight(left, right, options)
783+
// }
780784
let leftIndex = 0
781785
let rightIndex = 0
782786
while (leftIndex < left.length && rightIndex < right.length) {
@@ -820,6 +824,11 @@ function isCoveredAltNodes(
820824
leftIndex++
821825
rightIndex++
822826
}
827+
if (!options.canOmitRight) {
828+
if (rightIndex < right.length) {
829+
return false
830+
}
831+
}
823832
while (leftIndex < left.length) {
824833
const le = left[leftIndex]
825834
if (le.type !== "NormalizedOptional") {
@@ -829,3 +838,62 @@ function isCoveredAltNodes(
829838
}
830839
return leftIndex >= left.length
831840
}
841+
842+
/** Check whether the right nodes is covered by the left nodes. */
843+
// function isCoveredAltNodesForCanOmitRight(
844+
// left: NormalizedNode[],
845+
// right: NormalizedNode[],
846+
// options: Options,
847+
// ) {
848+
// let leftIndex = 0
849+
// let rightIndex = 0
850+
// while (leftIndex < left.length && rightIndex < right.length) {
851+
// const le = left[leftIndex]
852+
// const re = right[rightIndex]
853+
854+
// if (re.type === "NormalizedOptional") {
855+
// rightIndex++
856+
// continue
857+
// } else if (le.type === "NormalizedOptional") {
858+
// // Checks if skipped.
859+
// const skippedLeftItems = left.slice(leftIndex + 1)
860+
// if (
861+
// isCoveredAltNodes(
862+
// skippedLeftItems,
863+
// right.slice(rightIndex),
864+
// options,
865+
// )
866+
// ) {
867+
// return true
868+
// }
869+
// if (!isCoveredForNormalizedNode(le.element, re, options)) {
870+
// // I know it won't match if I skip it.
871+
// return false
872+
// }
873+
// if (le.max >= 2) {
874+
// // Check for multiple iterations.
875+
// if (
876+
// isCoveredAltNodes(
877+
// [le.decrementMax(), ...skippedLeftItems],
878+
// right.slice(rightIndex + 1),
879+
// options,
880+
// )
881+
// ) {
882+
// return true
883+
// }
884+
// }
885+
// } else if (!isCoveredForNormalizedNode(le, re, options)) {
886+
// return false
887+
// }
888+
// leftIndex++
889+
// rightIndex++
890+
// }
891+
// while (leftIndex < left.length) {
892+
// const le = left[leftIndex]
893+
// if (le.type !== "NormalizedOptional") {
894+
// return false
895+
// }
896+
// leftIndex++
897+
// }
898+
// return leftIndex >= left.length
899+
// }

tests/lib/rules/no-dupe-disjunctions.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,16 @@ const tester = new RuleTester({
1010

1111
tester.run("no-dupe-disjunctions", rule as any, {
1212
valid: [
13-
...[`/a|b/`, `/(a|b)/`, `/(?:a|b)/`, `/((?:ab|ba)|(?:ba|ac))/`].reduce(
13+
...[
14+
`/a|b/`,
15+
`/(a|b)/`,
16+
`/(?:a|b)/`,
17+
`/((?:ab|ba)|(?:ba|ac))/`,
18+
`/(?:js|json)$/`,
19+
`/(?:js|jso?n?)$/`,
20+
`/(?:js|json)abc/`,
21+
`/(?:js|json)?abc/`,
22+
].reduce(
1423
(acc, x) =>
1524
acc.concat(x, {
1625
code: x,

tests/lib/utils/regexp-ast.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,7 @@ describe("regexp-ast isCoveredNode", () => {
511511
assert.deepStrictEqual(
512512
isCoveredNode(ast1, ast2, {
513513
flags: { left: ast1.flags.raw, right: ast2.flags.raw },
514+
canOmitRight: true,
514515
}),
515516
testCase.result,
516517
)

0 commit comments

Comments
 (0)