Skip to content

Commit

Permalink
feat(valid-expect): supporting automatically fixing adding async in s…
Browse files Browse the repository at this point in the history
…ome cases (#1579)

* feat: add async

* test: tests for adding async

* feat: add await

* fix: valid-expect test

* Revert "fix: valid-expect test"

This reverts commit e652a25.

* fix: refactor to return an array

* fix: valid-expect logic

* Revert "fix: valid-expect logic"

This reverts commit ae8ecac.

* fix: valid-expect fixer logic

* refactor: fix import

* fix: write format
y-hsgw authored Jun 6, 2024
1 parent 0a14446 commit 5b9b47e
Showing 2 changed files with 188 additions and 26 deletions.
119 changes: 116 additions & 3 deletions src/rules/__tests__/valid-expect.test.ts
Original file line number Diff line number Diff line change
@@ -144,7 +144,6 @@ ruleTester.run('valid-expect', rule, {
},
],
},

{
code: 'expect().toBe(true);',
errors: [
@@ -417,7 +416,6 @@ ruleTester.run('valid-expect', rule, {
},
],
},

{
code: dedent`
expect.extend({
@@ -428,6 +426,15 @@ ruleTester.run('valid-expect', rule, {
}
});
`,
output: dedent`
expect.extend({
async toResolve(obj) {
this.isNot
? expect(obj).toBe(true)
: await expect(obj).resolves.not.toThrow();
}
});
`,
errors: [
{
column: 9,
@@ -446,6 +453,15 @@ ruleTester.run('valid-expect', rule, {
}
});
`,
output: dedent`
expect.extend({
async toResolve(obj) {
this.isNot
? await expect(obj).resolves.not.toThrow()
: expect(obj).toBe(true);
}
});
`,
errors: [
{
column: 9,
@@ -466,6 +482,17 @@ ruleTester.run('valid-expect', rule, {
}
});
`,
output: dedent`
expect.extend({
async toResolve(obj) {
this.isNot
? expect(obj).toBe(true)
: anotherCondition
? await expect(obj).resolves.not.toThrow()
: expect(obj).toBe(false)
}
});
`,
errors: [
{
column: 9,
@@ -478,6 +505,8 @@ ruleTester.run('valid-expect', rule, {
// expect().resolves
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).resolves.toBeDefined(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).resolves.toBeDefined(); });',
errors: [
{
column: 30,
@@ -489,6 +518,8 @@ ruleTester.run('valid-expect', rule, {
},
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).toResolve(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).toResolve(); });',
errors: [
{
messageId: 'asyncMustBeAwaited',
@@ -500,6 +531,8 @@ ruleTester.run('valid-expect', rule, {
},
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).toResolve(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).toResolve(); });',
options: [{ asyncMatchers: undefined }],
errors: [
{
@@ -512,6 +545,8 @@ ruleTester.run('valid-expect', rule, {
},
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).toReject(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).toReject(); });',
errors: [
{
messageId: 'asyncMustBeAwaited',
@@ -523,6 +558,8 @@ ruleTester.run('valid-expect', rule, {
},
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).not.toReject(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).not.toReject(); });',
errors: [
{
messageId: 'asyncMustBeAwaited',
@@ -535,6 +572,8 @@ ruleTester.run('valid-expect', rule, {
// expect().resolves.not
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).resolves.not.toBeDefined(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).resolves.not.toBeDefined(); });',
errors: [
{
column: 30,
@@ -547,6 +586,8 @@ ruleTester.run('valid-expect', rule, {
// expect().rejects
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).rejects.toBeDefined(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).rejects.toBeDefined(); });',
errors: [
{
column: 30,
@@ -559,6 +600,8 @@ ruleTester.run('valid-expect', rule, {
// expect().rejects.not
{
code: 'test("valid-expect", () => { expect(Promise.resolve(2)).rejects.not.toBeDefined(); });',
output:
'test("valid-expect", async () => { await expect(Promise.resolve(2)).rejects.not.toBeDefined(); });',
errors: [
{
column: 30,
@@ -597,6 +640,8 @@ ruleTester.run('valid-expect', rule, {
},
{
code: 'test("valid-expect", () => { expect(Promise.reject(2)).toRejectWith(2); });',
output:
'test("valid-expect", async () => { await expect(Promise.reject(2)).toRejectWith(2); });',
options: [{ asyncMatchers: ['toRejectWith'] }],
errors: [
{
@@ -608,6 +653,8 @@ ruleTester.run('valid-expect', rule, {
},
{
code: 'test("valid-expect", () => { expect(Promise.reject(2)).rejects.toBe(2); });',
output:
'test("valid-expect", async () => { await expect(Promise.reject(2)).rejects.toBe(2); });',
options: [{ asyncMatchers: ['toRejectWith'] }],
errors: [
{
@@ -785,6 +832,11 @@ ruleTester.run('valid-expect', rule, {
Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
output: dedent`
test("valid-expect", async () => {
await Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
errors: [
{
line: 2,
@@ -801,6 +853,11 @@ ruleTester.run('valid-expect', rule, {
Promise.reject(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
output: dedent`
test("valid-expect", async () => {
await Promise.reject(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
errors: [
{
line: 2,
@@ -838,6 +895,11 @@ ruleTester.run('valid-expect', rule, {
Promise.x(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
output: dedent`
test("valid-expect", async () => {
await Promise.x(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
errors: [
{
line: 2,
@@ -855,6 +917,11 @@ ruleTester.run('valid-expect', rule, {
Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
output: dedent`
test("valid-expect", async () => {
await Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined());
});
`,
options: [{ alwaysAwait: true }],
errors: [
{
@@ -875,6 +942,14 @@ ruleTester.run('valid-expect', rule, {
]);
});
`,
output: dedent`
test("valid-expect", async () => {
await Promise.all([
expect(Promise.resolve(2)).resolves.not.toBeDefined(),
expect(Promise.resolve(3)).resolves.not.toBeDefined(),
]);
});
`,
errors: [
{
line: 2,
@@ -896,6 +971,14 @@ ruleTester.run('valid-expect', rule, {
]);
});
`,
output: dedent`
test("valid-expect", async () => {
await Promise.x([
expect(Promise.resolve(2)).resolves.not.toBeDefined(),
expect(Promise.resolve(3)).resolves.not.toBeDefined(),
]);
});
`,
errors: [
{
line: 2,
@@ -907,7 +990,6 @@ ruleTester.run('valid-expect', rule, {
},
],
},
//
{
code: dedent`
test("valid-expect", () => {
@@ -917,6 +999,14 @@ ruleTester.run('valid-expect', rule, {
]
});
`,
output: dedent`
test("valid-expect", async () => {
const assertions = [
await expect(Promise.resolve(2)).resolves.not.toBeDefined(),
await expect(Promise.resolve(3)).resolves.not.toBeDefined(),
]
});
`,
errors: [
{
line: 3,
@@ -945,6 +1035,14 @@ ruleTester.run('valid-expect', rule, {
]
});
`,
output: dedent`
test("valid-expect", async () => {
const assertions = [
await expect(Promise.resolve(2)).toResolve(),
await expect(Promise.resolve(3)).toReject(),
]
});
`,
errors: [
{
messageId: 'asyncMustBeAwaited',
@@ -969,6 +1067,14 @@ ruleTester.run('valid-expect', rule, {
]
});
`,
output: dedent`
test("valid-expect", async () => {
const assertions = [
await expect(Promise.resolve(2)).not.toResolve(),
await expect(Promise.resolve(3)).resolves.toReject(),
]
});
`,
errors: [
{
messageId: 'asyncMustBeAwaited',
@@ -1002,6 +1108,13 @@ ruleTester.run('valid-expect', rule, {
});
});
`,
output: dedent`
test("valid-expect", () => {
return expect(functionReturningAPromise()).resolves.toEqual(1).then(async () => {
await expect(Promise.resolve(2)).resolves.toBe(1);
});
});
`,
errors: [
{
line: 3,
95 changes: 72 additions & 23 deletions src/rules/valid-expect.ts
Original file line number Diff line number Diff line change
@@ -3,8 +3,13 @@
* MIT license, Tom Vincent.
*/

import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils';
import {
AST_NODE_TYPES,
type TSESLint,
type TSESTree,
} from '@typescript-eslint/utils';
import {
type FunctionExpression,
ModifierName,
createRule,
getAccessorValue,
@@ -50,16 +55,30 @@ const findPromiseCallExpressionNode = (node: TSESTree.Node) =>
? getPromiseCallExpressionNode(node.parent)
: null;

const findFirstAsyncFunction = ({
const findFirstFunctionExpression = ({
parent,
}: TSESTree.Node): TSESTree.Node | null => {
}: TSESTree.Node): FunctionExpression | null => {
if (!parent) {
return null;
}

return isFunction(parent) && parent.async
? parent
: findFirstAsyncFunction(parent);
return isFunction(parent) ? parent : findFirstFunctionExpression(parent);
};

const getNormalizeFunctionExpression = (
functionExpression: FunctionExpression,
):
| TSESTree.PropertyComputedName
| TSESTree.PropertyNonComputedName
| FunctionExpression => {
if (
functionExpression.parent.type === AST_NODE_TYPES.Property &&
functionExpression.type === AST_NODE_TYPES.FunctionExpression
) {
return functionExpression.parent;
}

return functionExpression;
};

const getParentIfThenified = (node: TSESTree.Node): TSESTree.Node => {
@@ -189,6 +208,13 @@ export default createRule<[Options], MessageIds>({
) {
// Context state
const arrayExceptions = new Set<string>();
const descriptors: Array<{
node: TSESTree.Node;
messageId: Extract<
MessageIds,
'asyncMustBeAwaited' | 'promisesWithAsyncAssertionsMustBeAwaited'
>;
}> = [];

const pushPromiseArrayException = (loc: TSESTree.SourceLocation) =>
arrayExceptions.add(promiseArrayExceptionKey(loc));
@@ -320,7 +346,7 @@ export default createRule<[Options], MessageIds>({
jestFnCall.modifiers.some(nod => getAccessorValue(nod) !== 'not') ||
asyncMatchers.includes(getAccessorValue(matcher));

if (!parentNode?.parent || !shouldBeAwaited) {
if (!parentNode.parent || !shouldBeAwaited) {
return;
}
/**
@@ -329,7 +355,6 @@ export default createRule<[Options], MessageIds>({
*/
const isParentArrayExpression =
parentNode.parent.type === AST_NODE_TYPES.ArrayExpression;
const orReturned = alwaysAwait ? '' : ' or returned';
/**
* An async assertion can be chained with `then` or `catch` statements.
* In that case our target CallExpression node is the one with
@@ -346,39 +371,63 @@ export default createRule<[Options], MessageIds>({
// if we didn't warn user already
!promiseArrayExceptionExists(finalNode.loc)
) {
context.report({
loc: finalNode.loc,
data: { orReturned },
descriptors.push({
node: finalNode,
messageId:
finalNode === targetNode
targetNode === finalNode
? 'asyncMustBeAwaited'
: 'promisesWithAsyncAssertionsMustBeAwaited',
});
}
if (isParentArrayExpression) {
pushPromiseArrayException(finalNode.loc);
}
},
'Program:exit'() {
const fixes: TSESLint.RuleFix[] = [];

descriptors.forEach(({ node, messageId }, index) => {
const orReturned = alwaysAwait ? '' : ' or returned';

context.report({
loc: node.loc,
data: { orReturned },
messageId,
node,
fix(fixer) {
if (!findFirstAsyncFunction(finalNode)) {
return [];
const functionExpression = findFirstFunctionExpression(node);

if (!functionExpression) {
return null;
}
const foundAsyncFixer = fixes.some(fix => fix.text === 'async ');

if (!functionExpression.async && !foundAsyncFixer) {
const targetFunction =
getNormalizeFunctionExpression(functionExpression);

fixes.push(fixer.insertTextBefore(targetFunction, 'async '));
}

const returnStatement =
finalNode.parent?.type === AST_NODE_TYPES.ReturnStatement
? finalNode.parent
node.parent?.type === AST_NODE_TYPES.ReturnStatement
? node.parent
: null;

if (alwaysAwait && returnStatement) {
const sourceCodeText =
getSourceCode(context).getText(returnStatement);
const replacedText = sourceCodeText.replace('return', 'await');

return fixer.replaceText(returnStatement, replacedText);
fixes.push(fixer.replaceText(returnStatement, replacedText));
} else {
fixes.push(fixer.insertTextBefore(node, 'await '));
}

return fixer.insertTextBefore(finalNode, 'await ');
return index === descriptors.length - 1 ? fixes : null;
},
});

if (isParentArrayExpression) {
pushPromiseArrayException(finalNode.loc);
}
}
});
},
};
},

0 comments on commit 5b9b47e

Please sign in to comment.