Skip to content

Commit 5a6d1af

Browse files
authored
Fix false negatives in prefer-result-array-groups rule (#356)
* Fix false negatives in `prefer-result-array-groups` rule * Add test
1 parent 1432da0 commit 5a6d1af

File tree

4 files changed

+163
-7
lines changed

4 files changed

+163
-7
lines changed

lib/rules/prefer-result-array-groups.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import {
66
getTypeScriptTools,
77
isAny,
88
isClassOrInterface,
9+
isUnionOrIntersection,
10+
isNull,
911
} from "../utils/ts-utils"
12+
import type * as TS from "typescript"
1013
import type { Expression, Super } from "estree"
1114

1215
export default createRule("prefer-result-array-groups", {
@@ -149,16 +152,33 @@ export default createRule("prefer-result-array-groups", {
149152
return "unknown"
150153
}
151154

152-
if (isClassOrInterface(tsType)) {
153-
const name = tsType.symbol.escapedName
154-
return name === "RegExpMatchArray" || name === "RegExpExecArray"
155-
? "RegExpXArray"
156-
: "unknown"
157-
}
158155
if (isAny(tsType)) {
159156
return "any"
160157
}
158+
if (isRegExpMatchArrayOrRegExpExecArray(tsType)) {
159+
return "RegExpXArray"
160+
}
161+
if (isUnionOrIntersection(tsType)) {
162+
if (
163+
tsType.types.every(
164+
(t) =>
165+
isRegExpMatchArrayOrRegExpExecArray(t) || isNull(t),
166+
)
167+
) {
168+
// e.g. (RegExpExecArray | null)
169+
return "RegExpXArray"
170+
}
171+
}
161172
return "unknown"
162173
}
174+
175+
/** Checks whether given type is RegExpMatchArray or RegExpExecArray or not */
176+
function isRegExpMatchArrayOrRegExpExecArray(tsType: TS.Type) {
177+
if (isClassOrInterface(tsType)) {
178+
const name = tsType.symbol.escapedName
179+
return name === "RegExpMatchArray" || name === "RegExpExecArray"
180+
}
181+
return false
182+
}
163183
},
164184
})

lib/utils/ast-utils/extract-expression-references.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,18 @@ import type {
44
ArrayPattern,
55
ArrowFunctionExpression,
66
CallExpression,
7+
ConditionalExpression,
78
Expression,
89
ForOfStatement,
910
FunctionDeclaration,
1011
FunctionExpression,
1112
Identifier,
13+
IfStatement,
14+
LogicalExpression,
1215
MemberExpression,
1316
ObjectPattern,
1417
Pattern,
18+
UnaryExpression,
1519
} from "estree"
1620
import { findFunction, findVariable, getParent } from "./utils"
1721

@@ -100,7 +104,13 @@ function* iterateReferencesForExpression(
100104
): Iterable<ExpressionReference> {
101105
let node = expression
102106
let parent = getParent(node)
103-
while (parent?.type === "ChainExpression") {
107+
while (
108+
parent?.type === "ChainExpression" ||
109+
// @ts-expect-error -- TS AST
110+
parent?.type === "TSNonNullExpression" ||
111+
// @ts-expect-error -- TS AST
112+
parent?.type === "TSAsExpression"
113+
) {
104114
node = parent
105115
parent = getParent(node)
106116
}
@@ -172,11 +182,46 @@ function* iterateReferencesForExpression(
172182
} else {
173183
yield { node, type: "unknown" }
174184
}
185+
} else if (
186+
parent.type === "IfStatement" ||
187+
parent.type === "ConditionalExpression" ||
188+
parent.type === "LogicalExpression" ||
189+
parent.type === "UnaryExpression"
190+
) {
191+
if (isUsedInTest(parent, node)) {
192+
// The expression used in the test does not need to be tracked and returns nothing.
193+
} else {
194+
yield { node, type: "unknown" }
195+
}
175196
} else {
176197
yield { node, type: "unknown" }
177198
}
178199
}
179200

201+
/** Checks whether the expression is used in the test. */
202+
function isUsedInTest(
203+
parent:
204+
| IfStatement
205+
| ConditionalExpression
206+
| LogicalExpression
207+
| UnaryExpression,
208+
node: Expression,
209+
) {
210+
if (parent.type === "IfStatement") {
211+
return parent.test === node
212+
}
213+
if (parent.type === "ConditionalExpression") {
214+
return parent.test === node
215+
}
216+
if (parent.type === "LogicalExpression") {
217+
return parent.operator === "&&" && parent.left === node
218+
}
219+
if (parent.type === "UnaryExpression") {
220+
return parent.operator === "!" && parent.argument === node
221+
}
222+
return false
223+
}
224+
180225
/** Iterate references for the given pattern node. */
181226
function* iterateReferencesForESPattern(
182227
expression: Expression,

lib/utils/ts-utils/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,3 +155,10 @@ export function isBigIntLike(tsType: TS.Type): boolean {
155155
const ts = getTypeScript()!
156156
return (tsType.flags & ts.TypeFlags.BigIntLike) !== 0
157157
}
158+
/**
159+
* Check if a given type is an null type or not.
160+
*/
161+
export function isNull(tsType: TS.Type): boolean {
162+
const ts = getTypeScript()!
163+
return (tsType.flags & ts.TypeFlags.Null) !== 0
164+
}

tests/lib/rules/prefer-result-array-groups.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,5 +409,89 @@ tester.run("prefer-result-array-groups", rule as any, {
409409
"Unexpected indexed access for the named capturing group 'foo' from regexp result array.",
410410
],
411411
},
412+
{
413+
// https://github.com/ota-meshi/eslint-plugin-regexp/issues/355
414+
filename: path.join(__dirname, "prefer-result-array-groups.ts"),
415+
code: `
416+
const match = /(?<foo>foo)/u.exec(str)!
417+
match[1]; // <-
418+
419+
/(?<bar>bar)/u.exec(str)?.[1]; // <-
420+
const match2 = /(?<baz>baz)/u.exec(str)
421+
match2?.[1] // <-
422+
423+
const match3 = /(?<qux>qux)/u.exec(str) as RegExpExecArray
424+
match3[1] // <-
425+
`,
426+
output: `
427+
const match = /(?<foo>foo)/u.exec(str)!
428+
match.groups!.foo; // <-
429+
430+
/(?<bar>bar)/u.exec(str)?.groups!.bar; // <-
431+
const match2 = /(?<baz>baz)/u.exec(str)
432+
match2?.groups!.baz // <-
433+
434+
const match3 = /(?<qux>qux)/u.exec(str) as RegExpExecArray
435+
match3.groups!.qux // <-
436+
`,
437+
parser: require.resolve("@typescript-eslint/parser"),
438+
parserOptions: {
439+
project: require.resolve("../../../tsconfig.json"),
440+
},
441+
errors: [
442+
"Unexpected indexed access for the named capturing group 'foo' from regexp result array.",
443+
"Unexpected indexed access for the named capturing group 'bar' from regexp result array.",
444+
"Unexpected indexed access for the named capturing group 'baz' from regexp result array.",
445+
"Unexpected indexed access for the named capturing group 'qux' from regexp result array.",
446+
],
447+
},
448+
{
449+
code: `
450+
const match = /(?<foo>foo)/u.exec(str)
451+
if (match) {
452+
match[1] // <-
453+
}
454+
455+
const match2 = /(?<bar>bar)/u.exec(str)
456+
match2
457+
? match2[1] // <-
458+
: null;
459+
460+
const match3 = /(?<baz>baz)/u.exec(str)
461+
match3 && match3[1] // <-
462+
463+
const match4 = /(?<qux>qux)/u.exec(str)
464+
if (!match4) {
465+
} else {
466+
match4[1] // <-
467+
}
468+
`,
469+
output: `
470+
const match = /(?<foo>foo)/u.exec(str)
471+
if (match) {
472+
match.groups.foo // <-
473+
}
474+
475+
const match2 = /(?<bar>bar)/u.exec(str)
476+
match2
477+
? match2.groups.bar // <-
478+
: null;
479+
480+
const match3 = /(?<baz>baz)/u.exec(str)
481+
match3 && match3.groups.baz // <-
482+
483+
const match4 = /(?<qux>qux)/u.exec(str)
484+
if (!match4) {
485+
} else {
486+
match4.groups.qux // <-
487+
}
488+
`,
489+
errors: [
490+
"Unexpected indexed access for the named capturing group 'foo' from regexp result array.",
491+
"Unexpected indexed access for the named capturing group 'bar' from regexp result array.",
492+
"Unexpected indexed access for the named capturing group 'baz' from regexp result array.",
493+
"Unexpected indexed access for the named capturing group 'qux' from regexp result array.",
494+
],
495+
},
412496
],
413497
})

0 commit comments

Comments
 (0)