Skip to content

Commit ac86a79

Browse files
author
Josh Goldberg
authored
fix(eslint-plugin): [prefer-regexp-exec] factor in union types (#3434)
1 parent c50f70b commit ac86a79

File tree

2 files changed

+92
-115
lines changed

2 files changed

+92
-115
lines changed

‎packages/eslint-plugin/src/rules/prefer-regexp-exec.ts‎

Lines changed: 69 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import {
22
AST_NODE_TYPES,
33
TSESTree,
44
} from '@typescript-eslint/experimental-utils';
5+
import * as tsutils from 'tsutils';
6+
import * as ts from 'typescript';
57
import {
68
createRule,
79
getParserServices,
@@ -10,6 +12,13 @@ import {
1012
getWrappingFixer,
1113
} from '../util';
1214

15+
enum ArgumentType {
16+
Other = 0,
17+
String = 1 << 0,
18+
RegExp = 1 << 1,
19+
Both = String | RegExp,
20+
}
21+
1322
export default createRule({
1423
name: 'prefer-regexp-exec',
1524
defaultOptions: [],
@@ -37,25 +46,33 @@ export default createRule({
3746
const sourceCode = context.getSourceCode();
3847

3948
/**
40-
* Check if a given node is a string.
41-
* @param node The node to check.
49+
* Check if a given node type is a string.
50+
* @param node The node type to check.
4251
*/
43-
function isStringType(node: TSESTree.Node): boolean {
44-
const objectType = typeChecker.getTypeAtLocation(
45-
parserServices.esTreeNodeToTSNodeMap.get(node),
46-
);
47-
return getTypeName(typeChecker, objectType) === 'string';
52+
function isStringType(type: ts.Type): boolean {
53+
return getTypeName(typeChecker, type) === 'string';
4854
}
4955

5056
/**
51-
* Check if a given node is a RegExp.
52-
* @param node The node to check.
57+
* Check if a given node type is a RegExp.
58+
* @param node The node type to check.
5359
*/
54-
function isRegExpType(node: TSESTree.Node): boolean {
55-
const objectType = typeChecker.getTypeAtLocation(
56-
parserServices.esTreeNodeToTSNodeMap.get(node),
57-
);
58-
return getTypeName(typeChecker, objectType) === 'RegExp';
60+
function isRegExpType(type: ts.Type): boolean {
61+
return getTypeName(typeChecker, type) === 'RegExp';
62+
}
63+
64+
function collectArgumentTypes(types: ts.Type[]): ArgumentType {
65+
let result = ArgumentType.Other;
66+
67+
for (const type of types) {
68+
if (isRegExpType(type)) {
69+
result |= ArgumentType.RegExp;
70+
} else if (isStringType(type)) {
71+
result |= ArgumentType.String;
72+
}
73+
}
74+
75+
return result;
5976
}
6077

6178
return {
@@ -67,7 +84,13 @@ export default createRule({
6784
const argumentNode = callNode.arguments[0];
6885
const argumentValue = getStaticValue(argumentNode, globalScope);
6986

70-
if (!isStringType(objectNode)) {
87+
if (
88+
!isStringType(
89+
typeChecker.getTypeAtLocation(
90+
parserServices.esTreeNodeToTSNodeMap.get(objectNode),
91+
),
92+
)
93+
) {
7194
return;
7295
}
7396

@@ -97,38 +120,39 @@ export default createRule({
97120
});
98121
}
99122

100-
if (isRegExpType(argumentNode)) {
101-
return context.report({
102-
node: memberNode.property,
103-
messageId: 'regExpExecOverStringMatch',
104-
fix: getWrappingFixer({
105-
sourceCode,
106-
node: callNode,
107-
innerNode: [objectNode, argumentNode],
108-
wrap: (objectCode, argumentCode) =>
109-
`${argumentCode}.exec(${objectCode})`,
110-
}),
111-
});
112-
}
123+
const argumentType = typeChecker.getTypeAtLocation(
124+
parserServices.esTreeNodeToTSNodeMap.get(argumentNode),
125+
);
126+
const argumentTypes = collectArgumentTypes(
127+
tsutils.unionTypeParts(argumentType),
128+
);
129+
switch (argumentTypes) {
130+
case ArgumentType.RegExp:
131+
return context.report({
132+
node: memberNode.property,
133+
messageId: 'regExpExecOverStringMatch',
134+
fix: getWrappingFixer({
135+
sourceCode,
136+
node: callNode,
137+
innerNode: [objectNode, argumentNode],
138+
wrap: (objectCode, argumentCode) =>
139+
`${argumentCode}.exec(${objectCode})`,
140+
}),
141+
});
113142

114-
if (isStringType(argumentNode)) {
115-
return context.report({
116-
node: memberNode.property,
117-
messageId: 'regExpExecOverStringMatch',
118-
fix: getWrappingFixer({
119-
sourceCode,
120-
node: callNode,
121-
innerNode: [objectNode, argumentNode],
122-
wrap: (objectCode, argumentCode) =>
123-
`RegExp(${argumentCode}).exec(${objectCode})`,
124-
}),
125-
});
143+
case ArgumentType.String:
144+
return context.report({
145+
node: memberNode.property,
146+
messageId: 'regExpExecOverStringMatch',
147+
fix: getWrappingFixer({
148+
sourceCode,
149+
node: callNode,
150+
innerNode: [objectNode, argumentNode],
151+
wrap: (objectCode, argumentCode) =>
152+
`RegExp(${argumentCode}).exec(${objectCode})`,
153+
}),
154+
});
126155
}
127-
128-
return context.report({
129-
node: memberNode.property,
130-
messageId: 'regExpExecOverStringMatch',
131-
});
132156
},
133157
};
134158
},

‎packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts‎

Lines changed: 23 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,29 @@ function f(s: string | string[]) {
3333
s.match(/e/);
3434
}
3535
`,
36+
"(Math.random() > 0.5 ? 'abc' : 123).match(2);",
37+
"'212'.match(2);",
38+
"'212'.match(+2);",
39+
"'oNaNo'.match(NaN);",
40+
"'Infinity contains -Infinity and +Infinity in JavaScript.'.match(Infinity);",
41+
"'Infinity contains -Infinity and +Infinity in JavaScript.'.match(+Infinity);",
42+
"'Infinity contains -Infinity and +Infinity in JavaScript.'.match(-Infinity);",
43+
"'void and null'.match(null);",
44+
`
45+
const matchers = ['package-lock.json', /regexp/];
46+
const file = '';
47+
matchers.some(matcher => !!file.match(matcher));
48+
`,
49+
`
50+
const matchers = [/regexp/, 'package-lock.json'];
51+
const file = '';
52+
matchers.some(matcher => !!file.match(matcher));
53+
`,
54+
`
55+
const matchers = [{ match: (s: RegExp) => false }];
56+
const file = '';
57+
matchers.some(matcher => !!file.match(matcher));
58+
`,
3659
],
3760
invalid: [
3861
{
@@ -95,76 +118,6 @@ const search = 'thing';
95118
RegExp(search).exec(text);
96119
`,
97120
},
98-
{
99-
code: "'212'.match(2);",
100-
errors: [
101-
{
102-
messageId: 'regExpExecOverStringMatch',
103-
line: 1,
104-
column: 7,
105-
},
106-
],
107-
},
108-
{
109-
code: "'212'.match(+2);",
110-
errors: [
111-
{
112-
messageId: 'regExpExecOverStringMatch',
113-
line: 1,
114-
column: 7,
115-
},
116-
],
117-
},
118-
{
119-
code: "'oNaNo'.match(NaN);",
120-
errors: [
121-
{
122-
messageId: 'regExpExecOverStringMatch',
123-
line: 1,
124-
column: 9,
125-
},
126-
],
127-
},
128-
{
129-
code: "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(Infinity);",
130-
errors: [
131-
{
132-
messageId: 'regExpExecOverStringMatch',
133-
line: 1,
134-
column: 60,
135-
},
136-
],
137-
},
138-
{
139-
code: "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(+Infinity);",
140-
errors: [
141-
{
142-
messageId: 'regExpExecOverStringMatch',
143-
line: 1,
144-
column: 60,
145-
},
146-
],
147-
},
148-
{
149-
code: "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(-Infinity);",
150-
errors: [
151-
{
152-
messageId: 'regExpExecOverStringMatch',
153-
line: 1,
154-
column: 60,
155-
},
156-
],
157-
},
158-
{
159-
code: "'void and null'.match(null);",
160-
errors: [
161-
{
162-
messageId: 'regExpExecOverStringMatch',
163-
line: 1,
164-
column: 17,
165-
},
166-
],
167-
},
168121
{
169122
code: `
170123
function f(s: 'a' | 'b') {

0 commit comments

Comments
 (0)