Skip to content

Commit 45aa6f9

Browse files
Boombersindresorhus
Boomber
and
sindresorhus
authored
Detect more assertions in prefer-t-regex (#319)
Co-authored-by: Sindre Sorhus <[email protected]>
1 parent ae54195 commit 45aa6f9

File tree

2 files changed

+272
-50
lines changed

2 files changed

+272
-50
lines changed

‎rules/prefer-t-regex.js

Lines changed: 160 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,75 +13,186 @@ const create = context => {
1313
'falsy'
1414
]);
1515

16+
const equalityTests = new Set([
17+
'is',
18+
'deepEqual'
19+
]);
20+
21+
// Find the latest reference to the given identifier's name.
1622
const findReference = name => {
1723
const reference = context.getScope().references.find(reference => reference.identifier.name === name);
18-
const definitions = reference.resolved.defs;
19-
return definitions[definitions.length - 1].node;
20-
};
2124

22-
return ava.merge({
23-
CallExpression: visitIf([
24-
ava.isInTestFile,
25-
ava.isInTestNode
26-
])(node => {
27-
// Call a boolean assertion, for example, `t.true`, `t.false`, …
28-
const isBooleanAssertion = node.callee.type === 'MemberExpression' &&
29-
booleanTests.has(node.callee.property.name) &&
30-
util.getNameOfRootNodeObject(node.callee) === 't';
25+
if (reference && reference.resolved) {
26+
const definitions = reference.resolved.defs;
3127

32-
if (!isBooleanAssertion) {
28+
if (definitions.length === 0) {
3329
return;
3430
}
3531

36-
const firstArg = node.arguments[0];
32+
return definitions[definitions.length - 1].node;
33+
}
34+
};
3735

38-
// First argument is a call expression
39-
const isFunctionCall = firstArg.type === 'CallExpression';
40-
if (!isFunctionCall || !firstArg.callee.property) {
41-
return;
42-
}
36+
/*
37+
Recursively find the "origin" node of the given node.
4338
44-
const {name} = firstArg.callee.property;
45-
let lookup = {};
46-
let variable = {};
47-
48-
if (name === 'test') {
49-
// `lookup.test(variable)`
50-
lookup = firstArg.callee.object;
51-
variable = firstArg.arguments[0];
52-
} else if (['search', 'match'].includes(name)) {
53-
// `variable.match(lookup)`
54-
lookup = firstArg.arguments[0];
55-
variable = firstArg.callee.object;
56-
}
39+
Note: `context.getScope()` doesn't contain information about the outer scope so in most cases this function will only find the reference directly above the current scope. So the following code will only find the reference in this order: y -> x, and it will have no knowledge of the number `0`. (assuming we run this function on the identifier `y`)
5740
58-
let isRegExp = lookup.regex;
41+
```
42+
const test = require('ava');
5943
60-
// It's not a regexp but an identifier
61-
if (!isRegExp && lookup.type === 'Identifier') {
62-
const reference = findReference(lookup.name);
63-
isRegExp = reference.init.regex;
64-
}
44+
let x = 0;
45+
let y = x;
6546
66-
if (!isRegExp) {
67-
return;
47+
test(t => {
48+
t.is(y, 0);
49+
});
50+
```
51+
*/
52+
const findRootReference = node => {
53+
if (node.type === 'Identifier') {
54+
const reference = findReference(node.name);
55+
56+
if (reference && reference.init) {
57+
return findRootReference(reference.init);
6858
}
6959

70-
const assertion = ['true', 'truthy'].includes(node.callee.property.name) ? 'regex' : 'notRegex';
60+
return node;
61+
}
62+
63+
if (node.type === 'CallExpression' || node.type === 'NewExpression') {
64+
return findRootReference(node.callee);
65+
}
66+
67+
if (node.type === 'MemberExpression') {
68+
return findRootReference(node.object);
69+
}
70+
71+
return node;
72+
};
73+
74+
/*
75+
Determine if the given node is a regex expression.
76+
77+
There are two ways to create regex expressions in JavaScript: Regex literal and `RegExp` class.
78+
1. Regex literal can be easily looked up using the `.regex` property on the node.
79+
2. `RegExp` class can't be looked up so the function just checks for the name `RegExp`.
80+
*/
81+
const isRegExp = lookup => {
82+
if (lookup.regex) {
83+
return true;
84+
}
85+
86+
// Look up references in case it's a variable or RegExp declaration.
87+
const reference = findRootReference(lookup);
88+
return reference.regex || reference.name === 'RegExp';
89+
};
90+
91+
const booleanHandler = node => {
92+
const firstArg = node.arguments[0];
93+
94+
const isFunctionCall = firstArg.type === 'CallExpression';
95+
if (!isFunctionCall || !firstArg.callee.property) {
96+
return;
97+
}
98+
99+
const {name} = firstArg.callee.property;
100+
let lookup = {};
101+
let variable = {};
102+
103+
if (name === 'test') {
104+
// Represent: `lookup.test(variable)`
105+
lookup = firstArg.callee.object;
106+
variable = firstArg.arguments[0];
107+
} else if (['search', 'match'].includes(name)) {
108+
// Represent: `variable.match(lookup)`
109+
lookup = firstArg.arguments[0];
110+
variable = firstArg.callee.object;
111+
}
112+
113+
if (!isRegExp(lookup)) {
114+
return;
115+
}
116+
117+
const assertion = ['true', 'truthy'].includes(node.callee.property.name) ? 'regex' : 'notRegex';
118+
119+
const fix = fixer => {
120+
const source = context.getSourceCode();
121+
return [
122+
fixer.replaceText(node.callee.property, assertion),
123+
fixer.replaceText(firstArg, `${source.getText(variable)}, ${source.getText(lookup)}`)
124+
];
125+
};
126+
127+
context.report({
128+
node,
129+
message: `Prefer using the \`t.${assertion}()\` assertion.`,
130+
fix
131+
});
132+
};
71133

72-
const fix = fixer => {
73-
const source = context.getSourceCode();
74-
return [
75-
fixer.replaceText(node.callee.property, assertion),
76-
fixer.replaceText(firstArg, `${source.getText(variable)}, ${source.getText(lookup)}`)
77-
];
78-
};
134+
const equalityHandler = node => {
135+
const [firstArg, secondArg] = node.arguments;
136+
137+
const firstArgumentIsRegex = isRegExp(firstArg);
138+
const secondArgumentIsRegex = isRegExp(secondArg);
139+
140+
// If both are regex, or neither are, the expression is ok.
141+
if (firstArgumentIsRegex === secondArgumentIsRegex) {
142+
return;
143+
}
144+
145+
const matchee = secondArgumentIsRegex ? firstArg : secondArg;
146+
const regex = secondArgumentIsRegex ? secondArg : firstArg;
147+
148+
const booleanFixer = assertion => fixer => {
149+
const source = context.getSourceCode();
150+
return [
151+
fixer.replaceText(node.callee.property, assertion),
152+
fixer.replaceText(firstArg, `${source.getText(regex.arguments[0])}`),
153+
fixer.replaceText(secondArg, `${source.getText(regex.callee.object)}`)
154+
];
155+
};
156+
157+
// Only fix a statically verifiable equality.
158+
if (regex && matchee.type === 'Literal') {
159+
let assertion;
160+
161+
if (matchee.raw === 'true') {
162+
assertion = 'regex';
163+
} else if (matchee.raw === 'false') {
164+
assertion = 'notRegex';
165+
} else {
166+
return;
167+
}
79168

80169
context.report({
81170
node,
82171
message: `Prefer using the \`t.${assertion}()\` assertion.`,
83-
fix
172+
fix: booleanFixer(assertion)
84173
});
174+
}
175+
};
176+
177+
return ava.merge({
178+
CallExpression: visitIf([
179+
ava.isInTestFile,
180+
ava.isInTestNode
181+
])(node => {
182+
const isAssertion = node.callee.type === 'MemberExpression' &&
183+
util.getNameOfRootNodeObject(node.callee) === 't';
184+
185+
const isBooleanAssertion = isAssertion &&
186+
booleanTests.has(node.callee.property.name);
187+
188+
const isEqualityAssertion = isAssertion &&
189+
equalityTests.has(node.callee.property.name);
190+
191+
if (isBooleanAssertion) {
192+
booleanHandler(node);
193+
} else if (isEqualityAssertion) {
194+
equalityHandler(node);
195+
}
85196
})
86197
});
87198
};

‎test/prefer-t-regex.js

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,21 @@ ruleTester.run('prefer-t-regex', rule, {
1717
valid: [
1818
header + 'test(t => t.regex("foo", /\\d+/));',
1919
header + 'test(t => t.regex(foo(), /\\d+/));',
20-
header + 'test(t => t.is(/\\d+/.test("foo")), true);',
20+
header + 'test(t => t.is(/\\d+/.test("foo"), foo));',
21+
header + 'test(t => t.is(RegExp("\\d+").test("foo"), foo));',
22+
header + 'test(t => t.is(RegExp(/\\d+/).test("foo"), foo));',
23+
header + 'test(t => t.is(/\\d+/, /\\w+/));',
24+
header + 'test(t => t.is(123, /\\d+/));',
2125
header + 'test(t => t.true(1 === 1));',
2226
header + 'test(t => t.true(foo.bar()));',
27+
header + 'test(t => t.is(foo, true));',
2328
header + 'const a = /\\d+/;\ntest(t => t.truthy(a));',
2429
header + 'const a = "not a regexp";\ntest(t => t.true(a.test("foo")));',
2530
header + 'test("main", t => t.true(foo()));',
31+
header + 'test(t => t.regex(foo, new RegExp("\\d+")));',
32+
header + 'test(t => t.regex(foo, RegExp("\\d+")));',
33+
header + 'test(t => t.regex(foo, new RegExp(/\\d+/)));',
34+
header + 'test(t => t.regex(foo, RegExp(/\\d+/)));',
2635
// Shouldn't be triggered since it's not a test file
2736
'test(t => t.true(/\\d+/.test("foo")));'
2837
],
@@ -57,10 +66,112 @@ ruleTester.run('prefer-t-regex', rule, {
5766
output: header + 'test(t => t.regex(foo(), /\\d+/));',
5867
errors: errors('regex')
5968
},
69+
{
70+
code: header + 'test(t => t.is(/\\d+/.test(foo), true));',
71+
output: header + 'test(t => t.regex(foo, /\\d+/));',
72+
errors: errors('regex')
73+
},
74+
{
75+
code: header + 'test(t => t.is(/\\d+/.test(foo), false));',
76+
output: header + 'test(t => t.notRegex(foo, /\\d+/));',
77+
errors: errors('notRegex')
78+
},
6079
{
6180
code: header + 'const reg = /\\d+/;\ntest(t => t.true(reg.test(foo.bar())));',
6281
output: header + 'const reg = /\\d+/;\ntest(t => t.regex(foo.bar(), reg));',
6382
errors: errors('regex')
83+
},
84+
// The same as the above tests but with `RegExp()` object instead of a regex literal
85+
{
86+
code: header + 'test(t => t.true(new RegExp("\\d+").test("foo")));',
87+
output: header + 'test(t => t.regex("foo", new RegExp("\\d+")));',
88+
errors: errors('regex')
89+
},
90+
{
91+
code: header + 'test(t => t.false(foo.search(new RegExp("\\d+"))));',
92+
output: header + 'test(t => t.notRegex(foo, new RegExp("\\d+")));',
93+
errors: errors('notRegex')
94+
},
95+
{
96+
code: header + 'const regexp = RegExp("\\d+");\ntest(t => t.true(foo.search(regexp)));',
97+
output: header + 'const regexp = RegExp("\\d+");\ntest(t => t.regex(foo, regexp));',
98+
errors: errors('regex')
99+
},
100+
{
101+
code: header + 'test(t => t.truthy(foo.match(new RegExp("\\d+"))));',
102+
output: header + 'test(t => t.regex(foo, new RegExp("\\d+")));',
103+
errors: errors('regex')
104+
},
105+
{
106+
code: header + 'test(t => t.false(RegExp("\\d+").test("foo")));',
107+
output: header + 'test(t => t.notRegex("foo", RegExp("\\d+")));',
108+
errors: errors('notRegex')
109+
},
110+
{
111+
code: header + 'test(t => t.true(new RegExp("\\d+").test(foo())));',
112+
output: header + 'test(t => t.regex(foo(), new RegExp("\\d+")));',
113+
errors: errors('regex')
114+
},
115+
{
116+
code: header + 'test(t => t.is(new RegExp("\\d+").test(foo), true));',
117+
output: header + 'test(t => t.regex(foo, new RegExp("\\d+")));',
118+
errors: errors('regex')
119+
},
120+
{
121+
code: header + 'test(t => t.is(new RegExp("\\d+").test(foo), false));',
122+
output: header + 'test(t => t.notRegex(foo, new RegExp("\\d+")));',
123+
errors: errors('notRegex')
124+
},
125+
{
126+
code: header + 'const reg = RegExp("\\d+");\ntest(t => t.true(reg.test(foo.bar())));',
127+
output: header + 'const reg = RegExp("\\d+");\ntest(t => t.regex(foo.bar(), reg));',
128+
errors: errors('regex')
129+
},
130+
// The same as the above tests but with regex literal instead of string regex
131+
{
132+
code: header + 'test(t => t.true(new RegExp(/\\d+/).test("foo")));',
133+
output: header + 'test(t => t.regex("foo", new RegExp(/\\d+/)));',
134+
errors: errors('regex')
135+
},
136+
{
137+
code: header + 'test(t => t.false(foo.search(new RegExp(/\\d+/))));',
138+
output: header + 'test(t => t.notRegex(foo, new RegExp(/\\d+/)));',
139+
errors: errors('notRegex')
140+
},
141+
{
142+
code: header + 'const regexp = RegExp(/\\d+/);\ntest(t => t.true(foo.search(regexp)));',
143+
output: header + 'const regexp = RegExp(/\\d+/);\ntest(t => t.regex(foo, regexp));',
144+
errors: errors('regex')
145+
},
146+
{
147+
code: header + 'test(t => t.truthy(foo.match(new RegExp(/\\d+/))));',
148+
output: header + 'test(t => t.regex(foo, new RegExp(/\\d+/)));',
149+
errors: errors('regex')
150+
},
151+
{
152+
code: header + 'test(t => t.false(RegExp(/\\d+/).test("foo")));',
153+
output: header + 'test(t => t.notRegex("foo", RegExp(/\\d+/)));',
154+
errors: errors('notRegex')
155+
},
156+
{
157+
code: header + 'test(t => t.true(new RegExp(/\\d+/).test(foo())));',
158+
output: header + 'test(t => t.regex(foo(), new RegExp(/\\d+/)));',
159+
errors: errors('regex')
160+
},
161+
{
162+
code: header + 'test(t => t.is(new RegExp(/\\d+/).test(foo), true));',
163+
output: header + 'test(t => t.regex(foo, new RegExp(/\\d+/)));',
164+
errors: errors('regex')
165+
},
166+
{
167+
code: header + 'test(t => t.is(new RegExp(/\\d+/).test(foo), false));',
168+
output: header + 'test(t => t.notRegex(foo, new RegExp(/\\d+/)));',
169+
errors: errors('notRegex')
170+
},
171+
{
172+
code: header + 'const reg = RegExp(/\\d+/);\ntest(t => t.true(reg.test(foo.bar())));',
173+
output: header + 'const reg = RegExp(/\\d+/);\ntest(t => t.regex(foo.bar(), reg));',
174+
errors: errors('regex')
64175
}
65176
]
66177
});

0 commit comments

Comments
 (0)