Skip to content

Commit 2e8d2a6

Browse files
G-RathG-Rath
committed
fix(no-test-callback): provide suggestion instead of autofix
1 parent 361b1bc commit 2e8d2a6

File tree

2 files changed

+207
-98
lines changed

2 files changed

+207
-98
lines changed

‎src/rules/__tests__/no-test-callback.test.ts

+136-33
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { TSESLint } from '@typescript-eslint/experimental-utils';
2+
import dedent from 'dedent';
23
import resolveFrom from 'resolve-from';
34
import rule from '../no-test-callback';
45

@@ -21,37 +22,127 @@ ruleTester.run('no-test-callback', rule, {
2122
invalid: [
2223
{
2324
code: 'test("something", done => {done();})',
24-
errors: [{ messageId: 'illegalTestCallback', line: 1, column: 19 }],
25-
output:
26-
'test("something", () => {return new Promise(done => {done();})})',
25+
errors: [
26+
{
27+
messageId: 'illegalTestCallback',
28+
line: 1,
29+
column: 19,
30+
suggestions: [
31+
{
32+
messageId: 'suggestWrappingInPromise',
33+
data: { callback: 'done' },
34+
output:
35+
'test("something", () => {return new Promise(done => {done();})})',
36+
},
37+
],
38+
},
39+
],
40+
},
41+
{
42+
code: 'test("something", finished => {finished();})',
43+
errors: [
44+
{
45+
messageId: 'illegalTestCallback',
46+
line: 1,
47+
column: 19,
48+
suggestions: [
49+
{
50+
messageId: 'suggestWrappingInPromise',
51+
data: { callback: 'finished' },
52+
output:
53+
'test("something", () => {return new Promise(finished => {finished();})})',
54+
},
55+
],
56+
},
57+
],
2758
},
2859
{
2960
code: 'test("something", (done) => {done();})',
30-
errors: [{ messageId: 'illegalTestCallback', line: 1, column: 20 }],
31-
output:
32-
'test("something", () => {return new Promise((done) => {done();})})',
61+
errors: [
62+
{
63+
messageId: 'illegalTestCallback',
64+
line: 1,
65+
column: 20,
66+
suggestions: [
67+
{
68+
messageId: 'suggestWrappingInPromise',
69+
data: { callback: 'done' },
70+
output:
71+
'test("something", () => {return new Promise((done) => {done();})})',
72+
},
73+
],
74+
},
75+
],
3376
},
3477
{
3578
code: 'test("something", done => done())',
36-
errors: [{ messageId: 'illegalTestCallback', line: 1, column: 19 }],
37-
output: 'test("something", () => new Promise(done => done()))',
79+
errors: [
80+
{
81+
messageId: 'illegalTestCallback',
82+
line: 1,
83+
column: 19,
84+
suggestions: [
85+
{
86+
messageId: 'suggestWrappingInPromise',
87+
data: { callback: 'done' },
88+
output: 'test("something", () => new Promise(done => done()))',
89+
},
90+
],
91+
},
92+
],
3893
},
3994
{
4095
code: 'test("something", (done) => done())',
41-
errors: [{ messageId: 'illegalTestCallback', line: 1, column: 20 }],
42-
output: 'test("something", () => new Promise((done) => done()))',
96+
errors: [
97+
{
98+
messageId: 'illegalTestCallback',
99+
line: 1,
100+
column: 20,
101+
suggestions: [
102+
{
103+
messageId: 'suggestWrappingInPromise',
104+
data: { callback: 'done' },
105+
output: 'test("something", () => new Promise((done) => done()))',
106+
},
107+
],
108+
},
109+
],
43110
},
44111
{
45112
code: 'test("something", function(done) {done();})',
46-
errors: [{ messageId: 'illegalTestCallback', line: 1, column: 28 }],
47-
output:
48-
'test("something", function() {return new Promise((done) => {done();})})',
113+
errors: [
114+
{
115+
messageId: 'illegalTestCallback',
116+
line: 1,
117+
column: 28,
118+
suggestions: [
119+
{
120+
messageId: 'suggestWrappingInPromise',
121+
data: { callback: 'done' },
122+
output:
123+
'test("something", function() {return new Promise((done) => {done();})})',
124+
},
125+
],
126+
},
127+
],
49128
},
50129
{
51130
code: 'test("something", function (done) {done();})',
52-
errors: [{ messageId: 'illegalTestCallback', line: 1, column: 29 }],
53-
output:
54-
'test("something", function () {return new Promise((done) => {done();})})',
131+
errors: [
132+
{
133+
messageId: 'illegalTestCallback',
134+
line: 1,
135+
column: 29,
136+
suggestions: [
137+
{
138+
messageId: 'suggestWrappingInPromise',
139+
data: { callback: 'done' },
140+
output:
141+
'test("something", function () {return new Promise((done) => {done();})})',
142+
},
143+
],
144+
},
145+
],
55146
},
56147
{
57148
code: 'test("something", async done => {done();})',
@@ -66,27 +157,39 @@ ruleTester.run('no-test-callback', rule, {
66157
errors: [{ messageId: 'useAwaitInsteadOfCallback', line: 1, column: 35 }],
67158
},
68159
{
69-
code: `
70-
test('my test', async (done) => {
71-
await myAsyncTask();
72-
expect(true).toBe(false);
73-
done();
74-
});
160+
code: dedent`
161+
test('my test', async (done) => {
162+
await myAsyncTask();
163+
expect(true).toBe(false);
164+
done();
165+
});
75166
`,
76-
errors: [{ messageId: 'useAwaitInsteadOfCallback', line: 2, column: 30 }],
167+
errors: [{ messageId: 'useAwaitInsteadOfCallback', line: 1, column: 24 }],
77168
},
78169
{
79-
code: `
80-
test('something', (done) => {
81-
done();
82-
});
83-
`,
84-
errors: [{ messageId: 'illegalTestCallback', line: 2, column: 26 }],
85-
output: `
86-
test('something', () => {return new Promise((done) => {
87-
done();
88-
})});
170+
code: dedent`
171+
test('something', (done) => {
172+
done();
173+
});
89174
`,
175+
errors: [
176+
{
177+
messageId: 'illegalTestCallback',
178+
line: 1,
179+
column: 20,
180+
suggestions: [
181+
{
182+
messageId: 'suggestWrappingInPromise',
183+
data: { callback: 'done' },
184+
output: dedent`
185+
test('something', () => {return new Promise((done) => {
186+
done();
187+
})});
188+
`,
189+
},
190+
],
191+
},
192+
],
90193
},
91194
],
92195
});

‎src/rules/no-test-callback.ts

+71-65
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export default createRule({
1111
},
1212
messages: {
1313
illegalTestCallback: 'Illegal usage of test callback',
14+
suggestWrappingInPromise: 'Wrap in `new Promise({{ callback }} => ...`',
1415
useAwaitInsteadOfCallback:
1516
'Use await instead of callback in async functions',
1617
},
@@ -50,71 +51,76 @@ export default createRule({
5051
context.report({
5152
node: argument,
5253
messageId: 'illegalTestCallback',
53-
fix(fixer) {
54-
const { body } = callback;
55-
56-
/* istanbul ignore if https://github.com/typescript-eslint/typescript-eslint/issues/734 */
57-
if (!body) {
58-
throw new Error(
59-
`Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
60-
);
61-
}
62-
63-
const sourceCode = context.getSourceCode();
64-
const firstBodyToken = sourceCode.getFirstToken(body);
65-
const lastBodyToken = sourceCode.getLastToken(body);
66-
const tokenBeforeArgument = sourceCode.getTokenBefore(argument);
67-
const tokenAfterArgument = sourceCode.getTokenAfter(argument);
68-
69-
/* istanbul ignore if */
70-
if (
71-
!('name' in argument) ||
72-
!firstBodyToken ||
73-
!lastBodyToken ||
74-
!tokenBeforeArgument ||
75-
!tokenAfterArgument
76-
) {
77-
throw new Error(
78-
`Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
79-
);
80-
}
81-
82-
const argumentInParens =
83-
tokenBeforeArgument.value === '(' &&
84-
tokenAfterArgument.value === ')';
85-
86-
let argumentFix = fixer.replaceText(argument, '()');
87-
88-
if (argumentInParens) {
89-
argumentFix = fixer.remove(argument);
90-
}
91-
92-
let newCallback = argument.name;
93-
94-
if (argumentInParens) {
95-
newCallback = `(${newCallback})`;
96-
}
97-
98-
let beforeReplacement = `new Promise(${newCallback} => `;
99-
let afterReplacement = ')';
100-
let replaceBefore = true;
101-
102-
if (body.type === AST_NODE_TYPES.BlockStatement) {
103-
const keyword = 'return';
104-
105-
beforeReplacement = `${keyword} ${beforeReplacement}{`;
106-
afterReplacement += '}';
107-
replaceBefore = false;
108-
}
109-
110-
return [
111-
argumentFix,
112-
replaceBefore
113-
? fixer.insertTextBefore(firstBodyToken, beforeReplacement)
114-
: fixer.insertTextAfter(firstBodyToken, beforeReplacement),
115-
fixer.insertTextAfter(lastBodyToken, afterReplacement),
116-
];
117-
},
54+
suggest: [
55+
{
56+
messageId: 'suggestWrappingInPromise',
57+
data: { callback: argument.name },
58+
fix(fixer) {
59+
const { body } = callback;
60+
61+
/* istanbul ignore if https://github.com/typescript-eslint/typescript-eslint/issues/734 */
62+
if (!body) {
63+
throw new Error(
64+
`Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
65+
);
66+
}
67+
68+
const sourceCode = context.getSourceCode();
69+
const firstBodyToken = sourceCode.getFirstToken(body);
70+
const lastBodyToken = sourceCode.getLastToken(body);
71+
const tokenBeforeArgument = sourceCode.getTokenBefore(argument);
72+
const tokenAfterArgument = sourceCode.getTokenAfter(argument);
73+
74+
/* istanbul ignore if */
75+
if (
76+
!firstBodyToken ||
77+
!lastBodyToken ||
78+
!tokenBeforeArgument ||
79+
!tokenAfterArgument
80+
) {
81+
throw new Error(
82+
`Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
83+
);
84+
}
85+
86+
const argumentInParens =
87+
tokenBeforeArgument.value === '(' &&
88+
tokenAfterArgument.value === ')';
89+
90+
let argumentFix = fixer.replaceText(argument, '()');
91+
92+
if (argumentInParens) {
93+
argumentFix = fixer.remove(argument);
94+
}
95+
96+
let newCallback = argument.name;
97+
98+
if (argumentInParens) {
99+
newCallback = `(${newCallback})`;
100+
}
101+
102+
let beforeReplacement = `new Promise(${newCallback} => `;
103+
let afterReplacement = ')';
104+
let replaceBefore = true;
105+
106+
if (body.type === AST_NODE_TYPES.BlockStatement) {
107+
const keyword = 'return';
108+
109+
beforeReplacement = `${keyword} ${beforeReplacement}{`;
110+
afterReplacement += '}';
111+
replaceBefore = false;
112+
}
113+
114+
return [
115+
argumentFix,
116+
replaceBefore
117+
? fixer.insertTextBefore(firstBodyToken, beforeReplacement)
118+
: fixer.insertTextAfter(firstBodyToken, beforeReplacement),
119+
fixer.insertTextAfter(lastBodyToken, afterReplacement),
120+
];
121+
},
122+
},
123+
],
118124
});
119125
},
120126
};

0 commit comments

Comments
 (0)