Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(await-async-query): add auto-fix #917

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ module.exports = [
| Name                            | Description | 💼 | ⚠️ | 🔧 |
| :------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- | :------------------------------------------------------------------ | :-- |
| [await-async-events](docs/rules/await-async-events.md) | Enforce promises from async event methods are handled | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | 🔧 |
| [await-async-queries](docs/rules/await-async-queries.md) | Enforce promises from async queries to be handled | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | |
| [await-async-queries](docs/rules/await-async-queries.md) | Enforce promises from async queries to be handled | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | 🔧 |
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce promises from async utils to be awaited properly | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | |
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensures consistent usage of `data-testid` | | | |
| [no-await-sync-events](docs/rules/no-await-sync-events.md) | Disallow unnecessary `await` for sync events | ![badge-angular][] ![badge-dom][] ![badge-react][] | | |
Expand Down
2 changes: 2 additions & 0 deletions docs/rules/await-async-queries.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

💼 This rule is enabled in the following configs: `angular`, `dom`, `marko`, `react`, `vue`.

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->

Ensure that promises returned by async queries are handled properly.
Expand Down
81 changes: 75 additions & 6 deletions lib/rules/await-async-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import { ASTUtils, TSESTree } from '@typescript-eslint/utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';
import {
findClosestCallExpressionNode,
findClosestFunctionExpressionNode,
getDeepestIdentifierNode,
getFunctionName,
getInnermostReturningFunction,
getVariableReferences,
isMemberExpression,
isPromiseHandled,
} from '../node-utils';

Expand Down Expand Up @@ -34,6 +36,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
asyncQueryWrapper:
'promise returned from `{{ name }}` wrapper over async query must be handled',
},
fixable: 'code',
schema: [],
},
defaultOptions: [],
Expand Down Expand Up @@ -74,22 +77,39 @@ export default createTestingLibraryRule<Options, MessageIds>({
closestCallExpressionNode.parent
);

// check direct usage of async query:
// const element = await findByRole('button')
/**
* Check direct usage of async query:
* const element = await findByRole('button');
*/
if (references.length === 0) {
if (!isPromiseHandled(identifierNode)) {
context.report({
node: identifierNode,
messageId: 'awaitAsyncQuery',
data: { name: identifierNode.name },
fix: (fixer) => {
if (
isMemberExpression(identifierNode.parent) &&
ASTUtils.isIdentifier(identifierNode.parent.object) &&
identifierNode.parent.object.name === 'screen'
) {
return fixer.insertTextBefore(
identifierNode.parent,
'await '
);
}
return fixer.insertTextBefore(identifierNode, 'await ');
},
});
return;
}
}

// check references usages of async query:
// const promise = findByRole('button')
// const element = await promise
/**
* Check references usages of async query:
* const promise = findByRole('button');
* const element = await promise;
*/
for (const reference of references) {
if (
ASTUtils.isIdentifier(reference.identifier) &&
Expand All @@ -99,6 +119,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
node: identifierNode,
messageId: 'awaitAsyncQuery',
data: { name: identifierNode.name },
fix: (fixer) =>
references.map((ref) =>
fixer.insertTextBefore(ref.identifier, 'await ')
),
});
return;
}
Expand All @@ -107,11 +131,56 @@ export default createTestingLibraryRule<Options, MessageIds>({
functionWrappersNames.includes(identifierNode.name) &&
!isPromiseHandled(identifierNode)
) {
// check async queries used within a wrapper previously detected
// Check async queries used within a wrapper previously detected
context.report({
node: identifierNode,
messageId: 'asyncQueryWrapper',
data: { name: identifierNode.name },
fix: (fixer) => {
const functionExpression =
findClosestFunctionExpressionNode(node);

if (!functionExpression) return null;

let IdentifierNodeFixer;
if (isMemberExpression(identifierNode.parent)) {
/**
* If the wrapper is a property of an object,
* add 'await' before the object, e.g.:
* const obj = { wrapper: () => screen.findByText(/foo/i) };
* await obj.wrapper();
*/
IdentifierNodeFixer = fixer.insertTextBefore(
identifierNode.parent,
'await '
);
} else {
/**
* Add 'await' before the wrapper function, e.g.:
* const wrapper = () => screen.findByText(/foo/i);
* await wrapper();
*/
IdentifierNodeFixer = fixer.insertTextBefore(
identifierNode,
'await '
);
}

if (functionExpression.async) {
return IdentifierNodeFixer;
} else {
/**
* Mutate the actual node so if other nodes exist in this
* function expression body they don't also try to fix it.
*/
functionExpression.async = true;

return [
IdentifierNodeFixer,
fixer.insertTextBefore(functionExpression, 'async '),
];
}
},
});
}
},
Expand Down
115 changes: 112 additions & 3 deletions tests/lib/rules/await-async-queries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,14 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
errors: [{ messageId: 'awaitAsyncQuery', line: 6, column: 21 }],
output: `// async queries without await operator or then method are not valid
import { render } from '${testingFramework}'

test("An example test", async () => {
doSomething()
const foo = await ${query}('foo')
});
`,
}) as const
)
),
Expand All @@ -382,6 +390,13 @@ ruleTester.run(RULE_NAME, rule, {
data: { name: query },
},
],
output: `// async screen queries without await operator or then method are not valid
import { render } from '@testing-library/react'

test("An example test", async () => {
await screen.${query}('foo')
});
`,
}) as const
),
...ALL_ASYNC_COMBINATIONS_TO_TEST.map(
Expand All @@ -403,6 +418,14 @@ ruleTester.run(RULE_NAME, rule, {
data: { name: query },
},
],
output: `
import { render } from '@testing-library/react'

test("An example test", async () => {
doSomething()
const foo = await ${query}('foo')
});
`,
}) as const
),
...ALL_ASYNC_COMBINATIONS_TO_TEST.map(
Expand All @@ -425,6 +448,15 @@ ruleTester.run(RULE_NAME, rule, {
data: { name: query },
},
],
output: `
import { render } from '@testing-library/react'

test("An example test", async () => {
const foo = ${query}('foo')
expect(await foo).toBeInTheDocument()
expect(await foo).toHaveAttribute('src', 'bar');
});
`,
}) as const
),

Expand All @@ -440,6 +472,13 @@ ruleTester.run(RULE_NAME, rule, {
})
`,
errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }],
output: `
import { render } from "another-library"

test('An example test', async () => {
const example = await ${query}("my example")
})
`,
}) as const
),

Expand All @@ -458,11 +497,26 @@ ruleTester.run(RULE_NAME, rule, {
const element = queryWrapper()
})

test("An invalid example test", async () => {
test("A valid example test", async () => {
const element = await queryWrapper()
})
`,
errors: [{ messageId: 'asyncQueryWrapper', line: 9, column: 27 }],
output: `
function queryWrapper() {
doSomethingElse();

return screen.${query}('foo')
}

test("An invalid example test", async () => {
const element = await queryWrapper()
})

test("A valid example test", async () => {
const element = await queryWrapper()
})
`,
}) as const
),
// unhandled promise from async query arrow function wrapper is invalid
Expand All @@ -480,11 +534,26 @@ ruleTester.run(RULE_NAME, rule, {
const element = queryWrapper()
})

test("An invalid example test", async () => {
test("A valid example test", async () => {
const element = await queryWrapper()
})
`,
errors: [{ messageId: 'asyncQueryWrapper', line: 9, column: 27 }],
output: `
const queryWrapper = () => {
doSomethingElse();

return ${query}('foo')
}

test("An invalid example test", async () => {
const element = await queryWrapper()
})

test("A valid example test", async () => {
const element = await queryWrapper()
})
`,
}) as const
),
// unhandled promise implicitly returned from async query arrow function wrapper is invalid
Expand All @@ -498,11 +567,22 @@ ruleTester.run(RULE_NAME, rule, {
const element = queryWrapper()
})

test("An invalid example test", async () => {
test("A valid example test", async () => {
const element = await queryWrapper()
})
`,
errors: [{ messageId: 'asyncQueryWrapper', line: 5, column: 27 }],
output: `
const queryWrapper = () => screen.${query}('foo')

test("An invalid example test", async () => {
const element = await queryWrapper()
})

test("A valid example test", async () => {
const element = await queryWrapper()
})
`,
}) as const
),

Expand All @@ -517,6 +597,11 @@ ruleTester.run(RULE_NAME, rule, {
})
`,
errors: [{ messageId: 'awaitAsyncQuery', line: 3, column: 25 }],
output: `
test('An invalid example test', () => {
const element = await findByIcon('search')
})
`,
},

{
Expand Down Expand Up @@ -545,6 +630,30 @@ ruleTester.run(RULE_NAME, rule, {
})
`,
errors: [{ messageId: 'asyncQueryWrapper', line: 19, column: 34 }],
output: `// similar to issue #359 but forcing an error in no-awaited wrapper
import { render, screen } from 'mocks/test-utils'
import userEvent from '@testing-library/user-event'

const testData = {
name: 'John Doe',
email: '[email protected]',
password: 'extremeSecret',
}

const selectors = {
username: () => screen.findByRole('textbox', { name: /username/i }),
email: () => screen.findByRole('textbox', { name: /e-mail/i }),
password: () => screen.findByLabelText(/password/i),
}

test('this is a valid case', async () => {
render(<SomeComponent />)
userEvent.type(await selectors.username(), testData.name) // <-- unhandled here
userEvent.type(await selectors.email(), testData.email)
userEvent.type(await selectors.password(), testData.password)
// ...
})
`,
},
],
});