Skip to content

Commit 6229b2b

Browse files
y-hsgwy-hsgw
authored
fix(prefer-called-exactly-once-with): incorrect auto-fix behavior (#778)
* fix: replace toHaveBeenCalledOnce/With pairs with toHaveBeenCalledExactlyOnceWith * test: add tests * fix: fix build error
1 parent c617878 commit 6229b2b

File tree

2 files changed

+272
-38
lines changed

2 files changed

+272
-38
lines changed

‎src/rules/prefer-called-exactly-once-with.ts‎

Lines changed: 151 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,69 @@
1+
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'
12
import { createEslintRule, getAccessorValue } from '../utils'
2-
import { parseVitestFnCall } from '../utils/parse-vitest-fn-call'
3+
import {
4+
ParsedExpectVitestFnCall,
5+
parseVitestFnCall,
6+
} from '../utils/parse-vitest-fn-call'
7+
import { SourceCode } from '@typescript-eslint/utils/ts-eslint'
38

49
type MESSAGE_IDS = 'preferCalledExactlyOnceWith'
510
export const RULE_NAME = 'prefer-called-exactly-once-with'
611
type Options = []
712

13+
const MATCHERS_TO_COMBINE = [
14+
'toHaveBeenCalledOnce',
15+
'toHaveBeenCalledWith',
16+
] as const
17+
18+
type CombinedMatcher = (typeof MATCHERS_TO_COMBINE)[number]
19+
20+
type MatcherReference = {
21+
matcherName: CombinedMatcher
22+
callExpression: TSESTree.CallExpression
23+
}
24+
25+
const hasMatchersToCombine = (target: string): target is CombinedMatcher =>
26+
MATCHERS_TO_COMBINE.some((matcher) => matcher === target)
27+
28+
const getExpectText = (
29+
expression: TSESTree.CallExpression,
30+
source: Readonly<SourceCode>,
31+
) => {
32+
if (expression.callee.type !== AST_NODE_TYPES.MemberExpression) return null
33+
34+
const { range } = expression.callee.object
35+
return source.text.slice(range[0], range[1])
36+
}
37+
38+
const getArgumentsText = (
39+
callExpression: TSESTree.CallExpression,
40+
source: Readonly<SourceCode>,
41+
) => callExpression.arguments.map((arg) => source.getText(arg)).join(', ')
42+
43+
const getValidExpectCall = (
44+
vitestFnCall: ReturnType<typeof parseVitestFnCall>,
45+
): ParsedExpectVitestFnCall | null => {
46+
if (vitestFnCall?.type !== 'expect') return null
47+
if (
48+
vitestFnCall.modifiers.some(
49+
(modifier) => getAccessorValue(modifier) === 'not',
50+
)
51+
)
52+
return null
53+
54+
return vitestFnCall
55+
}
56+
57+
const getMatcherName = (vitestFnCall: ReturnType<typeof parseVitestFnCall>) => {
58+
const validExpectCall = getValidExpectCall(vitestFnCall)
59+
return validExpectCall ? getAccessorValue(validExpectCall.matcher) : null
60+
}
61+
62+
const getMemberProperty = (expression: TSESTree.CallExpression) =>
63+
expression.callee.type === AST_NODE_TYPES.MemberExpression
64+
? expression.callee.property
65+
: null
66+
867
export default createEslintRule<Options, MESSAGE_IDS>({
968
name: RULE_NAME,
1069
meta: {
@@ -22,36 +81,100 @@ export default createEslintRule<Options, MESSAGE_IDS>({
2281
},
2382
defaultOptions: [],
2483
create(context) {
25-
return {
26-
CallExpression(node) {
27-
const vitestFnCall = parseVitestFnCall(node, context)
84+
const { sourceCode } = context
2885

29-
if (vitestFnCall?.type !== 'expect') return
86+
const getCallExpressions = (
87+
body: TSESTree.Statement[],
88+
): TSESTree.CallExpression[] =>
89+
body
90+
.filter((node) => node.type === AST_NODE_TYPES.ExpressionStatement)
91+
.flatMap((node) =>
92+
node.expression.type === AST_NODE_TYPES.CallExpression
93+
? node.expression
94+
: [],
95+
)
96+
97+
const checkBlockBody = (body: TSESTree.Statement[]) => {
98+
const callExpressions = getCallExpressions(body)
99+
const expectMatcherMap = new Map<string, Readonly<MatcherReference>[]>()
30100

31-
if (
32-
vitestFnCall.modifiers.some(
33-
(node) => getAccessorValue(node) === 'not',
34-
)
101+
for (const callExpression of callExpressions) {
102+
const matcherName = getMatcherName(
103+
parseVitestFnCall(callExpression, context),
35104
)
36-
return
37-
38-
const { matcher } = vitestFnCall
39-
const matcherName = getAccessorValue(matcher)
40-
41-
if (
42-
['toHaveBeenCalledOnce', 'toHaveBeenCalledWith'].includes(matcherName)
43-
) {
44-
context.report({
45-
data: {
46-
matcherName,
47-
},
48-
messageId: 'preferCalledExactlyOnceWith',
49-
node: matcher,
50-
fix: (fixer) => [
51-
fixer.replaceText(matcher, `toHaveBeenCalledExactlyOnceWith`),
52-
],
53-
})
54-
}
105+
const expectedText = getExpectText(callExpression, sourceCode)
106+
if (!matcherName || !hasMatchersToCombine(matcherName) || !expectedText)
107+
continue
108+
109+
const existingNodes = expectMatcherMap.get(expectedText) ?? []
110+
const newTargetNodes = [
111+
...existingNodes,
112+
{ matcherName, callExpression },
113+
] as const satisfies MatcherReference[]
114+
expectMatcherMap.set(expectedText, newTargetNodes)
115+
}
116+
117+
for (const [
118+
expectedText,
119+
matcherReferences,
120+
] of expectMatcherMap.entries()) {
121+
if (matcherReferences.length !== 2) continue
122+
123+
const targetArgNode = matcherReferences.find(
124+
(reference) => reference.matcherName === 'toHaveBeenCalledWith',
125+
)
126+
if (!targetArgNode) continue
127+
128+
const argsText = getArgumentsText(
129+
targetArgNode.callExpression,
130+
sourceCode,
131+
)
132+
133+
const [firstMatcherReference, secondMatcherReference] =
134+
matcherReferences
135+
const targetNode = getMemberProperty(
136+
secondMatcherReference.callExpression,
137+
)
138+
if (!targetNode) continue
139+
140+
const { callExpression: firstCallExpression } = firstMatcherReference
141+
const { callExpression: secondCallExpression, matcherName } =
142+
secondMatcherReference
143+
144+
context.report({
145+
messageId: 'preferCalledExactlyOnceWith',
146+
node: targetNode,
147+
data: { matcherName },
148+
fix(fixer) {
149+
const indentation = sourceCode.text.slice(
150+
firstCallExpression.parent.range[0],
151+
firstCallExpression.range[0],
152+
)
153+
const replacement = `${indentation}${expectedText}.toHaveBeenCalledExactlyOnceWith(${argsText})`
154+
155+
const lineStart = sourceCode.getIndexFromLoc({
156+
line: secondCallExpression.parent.loc.start.line,
157+
column: 0,
158+
})
159+
const lineEnd = sourceCode.getIndexFromLoc({
160+
line: secondCallExpression.parent.loc.end.line + 1,
161+
column: 0,
162+
})
163+
return [
164+
fixer.replaceText(firstCallExpression, replacement),
165+
fixer.removeRange([lineStart, lineEnd]),
166+
]
167+
},
168+
})
169+
}
170+
}
171+
172+
return {
173+
Program(node) {
174+
checkBlockBody(node.body)
175+
},
176+
BlockStatement(node) {
177+
checkBlockBody(node.body)
55178
},
56179
}
57180
},

‎tests/prefer-called-exactly-once-with.test.ts‎

Lines changed: 121 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,143 @@ import { ruleTester } from './ruleTester'
44
ruleTester.run(RULE_NAME, rule, {
55
valid: [
66
'expect(fn).toHaveBeenCalledExactlyOnceWith();',
7-
'expect(x). toHaveBeenCalledExactlyOnceWith(args);',
7+
'expect(x).toHaveBeenCalledExactlyOnceWith(args);',
8+
'expect(x).toHaveBeenCalledOnce();',
9+
`expect(x).toHaveBeenCalledWith('hoge');`,
10+
`
11+
expect(x).toHaveBeenCalledOnce();
12+
expect(y).toHaveBeenCalledWith('hoge');
13+
`,
14+
`
15+
expect(x).toHaveBeenCalledOnce();
16+
expect(x).not.toHaveBeenCalledWith('hoge');
17+
`,
18+
`
19+
expect(x).not.toHaveBeenCalledOnce();
20+
expect(x).toHaveBeenCalledWith('hoge');
21+
`,
22+
`
23+
expect(x).not.toHaveBeenCalledOnce();
24+
expect(x).not.toHaveBeenCalledWith('hoge');
25+
`,
826
],
927
invalid: [
1028
{
11-
code: 'expect(x).toHaveBeenCalledOnce();',
29+
code: `
30+
expect(x).toHaveBeenCalledOnce();
31+
expect(x).toHaveBeenCalledWith('hoge');
32+
`,
33+
errors: [
34+
{
35+
messageId: 'preferCalledExactlyOnceWith',
36+
data: { matcherName: 'toHaveBeenCalledWith' },
37+
column: 17,
38+
line: 3,
39+
},
40+
],
41+
output: `
42+
expect(x).toHaveBeenCalledExactlyOnceWith('hoge');
43+
`,
44+
},
45+
{
46+
code: `
47+
expect(x).toHaveBeenCalledWith('hoge');
48+
expect(x).toHaveBeenCalledOnce();
49+
`,
1250
errors: [
1351
{
1452
messageId: 'preferCalledExactlyOnceWith',
1553
data: { matcherName: 'toHaveBeenCalledOnce' },
16-
column: 11,
17-
line: 1,
54+
column: 17,
55+
line: 3,
1856
},
1957
],
20-
output: 'expect(x).toHaveBeenCalledExactlyOnceWith();',
58+
output: `
59+
expect(x).toHaveBeenCalledExactlyOnceWith('hoge');
60+
`,
2161
},
2262
{
23-
code: 'expect(x).toHaveBeenCalledWith();',
63+
code: `
64+
expect(x).toHaveBeenCalledWith('hoge', 123);
65+
expect(x).toHaveBeenCalledOnce();
66+
`,
2467
errors: [
2568
{
2669
messageId: 'preferCalledExactlyOnceWith',
27-
data: { matcherName: 'toHaveBeenCalledWith' },
28-
column: 11,
29-
line: 1,
70+
data: { matcherName: 'toHaveBeenCalledOnce' },
71+
column: 17,
72+
line: 3,
73+
},
74+
],
75+
output: `
76+
expect(x).toHaveBeenCalledExactlyOnceWith('hoge', 123);
77+
`,
78+
},
79+
{
80+
code: `
81+
test('example',() => {
82+
expect(x).toHaveBeenCalledWith('hoge', 123);
83+
expect(x).toHaveBeenCalledOnce();
84+
});
85+
`,
86+
errors: [
87+
{
88+
messageId: 'preferCalledExactlyOnceWith',
89+
data: { matcherName: 'toHaveBeenCalledOnce' },
90+
column: 19,
91+
line: 4,
92+
},
93+
],
94+
output: `
95+
test('example',() => {
96+
expect(x).toHaveBeenCalledExactlyOnceWith('hoge', 123);
97+
});
98+
`,
99+
},
100+
{
101+
code: `
102+
expect(x).toHaveBeenCalledWith('hoge', 123);
103+
expect(x).toHaveBeenCalledOnce();
104+
expect(y).toHaveBeenCalledWith('foo', 456);
105+
expect(y).toHaveBeenCalledOnce();
106+
`,
107+
errors: [
108+
{
109+
messageId: 'preferCalledExactlyOnceWith',
110+
data: { matcherName: 'toHaveBeenCalledOnce' },
111+
column: 17,
112+
line: 3,
113+
},
114+
{
115+
messageId: 'preferCalledExactlyOnceWith',
116+
data: { matcherName: 'toHaveBeenCalledOnce' },
117+
column: 17,
118+
line: 5,
119+
},
120+
],
121+
output: `
122+
expect(x).toHaveBeenCalledExactlyOnceWith('hoge', 123);
123+
expect(y).toHaveBeenCalledExactlyOnceWith('foo', 456);
124+
`,
125+
},
126+
{
127+
code: `
128+
expect(x).toHaveBeenCalledWith('hoge', 123);
129+
const hoge = 'foo';
130+
expect(x).toHaveBeenCalledOnce();
131+
`,
132+
errors: [
133+
{
134+
messageId: 'preferCalledExactlyOnceWith',
135+
data: { matcherName: 'toHaveBeenCalledOnce' },
136+
column: 17,
137+
line: 4,
30138
},
31139
],
32-
output: 'expect(x).toHaveBeenCalledExactlyOnceWith();',
140+
output: `
141+
expect(x).toHaveBeenCalledExactlyOnceWith('hoge', 123);
142+
const hoge = 'foo';
143+
`,
33144
},
34145
],
35146
})

0 commit comments

Comments
 (0)