Skip to content

fix(await-async-utils): false positives #137

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 16, 2020
Merged

fix(await-async-utils): false positives #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cec1a8
refactor: move LIBRARY_MODULES to utils
dmitry-lobanov May 9, 2020
8422d53
fix: await-async-utils false positives
dmitry-lobanov May 16, 2020
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
48 changes: 40 additions & 8 deletions lib/rules/await-async-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';

import { getDocsUrl, ASYNC_UTILS } from '../utils';
import { getDocsUrl, ASYNC_UTILS, LIBRARY_MODULES } from '../utils';
import { isCallExpression, hasThenProperty } from '../node-utils';

export const RULE_NAME = 'await-async-utils';
Expand Down Expand Up @@ -49,17 +49,49 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
defaultOptions: [],

create(context) {
const testingLibraryUtilUsage: TSESTree.Identifier[] = [];
const asyncUtilsUsage: Array<{ node: TSESTree.Identifier | TSESTree.MemberExpression, name: string }> = [];
const importedAsyncUtils: string[] = [];

return {
'ImportDeclaration > ImportSpecifier,ImportNamespaceSpecifier'(node: TSESTree.Node) {
const parent = (node.parent as TSESTree.ImportDeclaration);

if (!LIBRARY_MODULES.includes(parent.source.value.toString())) return;

if (node.type === 'ImportSpecifier') {
importedAsyncUtils.push(node.imported.name);
}

if (node.type === 'ImportNamespaceSpecifier') {
importedAsyncUtils.push(node.local.name);
}
},
[`CallExpression > Identifier[name=${ASYNC_UTILS_REGEXP}]`](
node: TSESTree.Identifier
) {
if (!isAwaited(node.parent.parent) && !isPromiseResolved(node)) {
testingLibraryUtilUsage.push(node);
}
asyncUtilsUsage.push({ node, name: node.name });
},
[`CallExpression > MemberExpression > Identifier[name=${ASYNC_UTILS_REGEXP}]`](
node: TSESTree.Identifier
) {
const memberExpression = node.parent as TSESTree.MemberExpression;
const identifier = memberExpression.object as TSESTree.Identifier;
const memberExpressionName = identifier.name;

asyncUtilsUsage.push({ node: memberExpression, name: memberExpressionName });
},
'Program:exit'() {
testingLibraryUtilUsage.forEach(node => {
const testingLibraryUtilUsage = asyncUtilsUsage.filter(usage => {
if (usage.node.type === 'MemberExpression') {
const object = usage.node.object as TSESTree.Identifier;

return importedAsyncUtils.includes(object.name)
}

return importedAsyncUtils.includes(usage.name)
});

testingLibraryUtilUsage.forEach(({ node, name }) => {
const variableDeclaratorParent = node.parent.parent;

const references =
Expand All @@ -79,7 +111,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
node,
messageId: 'awaitAsyncUtil',
data: {
name: node.name,
name,
},
});
} else {
Expand All @@ -93,7 +125,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
node,
messageId: 'awaitAsyncUtil',
data: {
name: node.name,
name,
},
});

Expand Down
15 changes: 3 additions & 12 deletions lib/rules/no-debug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl } from '../utils';
import { getDocsUrl, LIBRARY_MODULES } from '../utils';
import {
isObjectPattern,
isProperty,
Expand All @@ -12,15 +12,6 @@ import {

export const RULE_NAME = 'no-debug';

const LIBRARY_MODULES_WITH_SCREEN = [
'@testing-library/dom',
'@testing-library/angular',
'@testing-library/react',
'@testing-library/preact',
'@testing-library/vue',
'@testing-library/svelte',
];

function isRenderFunction(
callNode: TSESTree.CallExpression,
renderFunctions: string[]
Expand Down Expand Up @@ -58,7 +49,7 @@ function hasTestingLibraryImportModule(
importDeclarationNode: TSESTree.ImportDeclaration
) {
const literal = importDeclarationNode.source;
return LIBRARY_MODULES_WITH_SCREEN.some(module => module === literal.value);
return LIBRARY_MODULES.some(module => module === literal.value);
}

export default ESLintUtils.RuleCreator(getDocsUrl)({
Expand Down Expand Up @@ -129,7 +120,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({
args =>
isLiteral(args) &&
typeof args.value === 'string' &&
LIBRARY_MODULES_WITH_SCREEN.includes(args.value)
LIBRARY_MODULES.includes(args.value)
);

if (!literalNodeScreenModuleName) {
Expand Down
10 changes: 10 additions & 0 deletions lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ const combineQueries = (variants: string[], methods: string[]) => {
const getDocsUrl = (ruleName: string) =>
`https://github.com/testing-library/eslint-plugin-testing-library/tree/master/docs/rules/${ruleName}.md`;

const LIBRARY_MODULES = [
'@testing-library/dom',
'@testing-library/angular',
'@testing-library/react',
'@testing-library/preact',
'@testing-library/vue',
'@testing-library/svelte',
];

const SYNC_QUERIES_VARIANTS = ['getBy', 'getAllBy', 'queryBy', 'queryAllBy'];
const ASYNC_QUERIES_VARIANTS = ['findBy', 'findAllBy'];
const ALL_QUERIES_VARIANTS = [
Expand Down Expand Up @@ -64,4 +73,5 @@ export {
ASYNC_QUERIES_COMBINATIONS,
ALL_QUERIES_COMBINATIONS,
ASYNC_UTILS,
LIBRARY_MODULES,
};
52 changes: 47 additions & 5 deletions tests/lib/rules/await-async-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ ruleTester.run(RULE_NAME, rule, {
valid: [
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util directly waited with await operator is valid', async () => {
doSomethingElse();
await ${asyncUtil}(() => getByLabelText('email'));
Expand All @@ -17,6 +18,7 @@ ruleTester.run(RULE_NAME, rule, {

...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util promise saved in var and waited with await operator is valid', async () => {
doSomethingElse();
const aPromise = ${asyncUtil}(() => getByLabelText('email'));
Expand All @@ -27,6 +29,7 @@ ruleTester.run(RULE_NAME, rule, {

...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util directly chained with then is valid', () => {
doSomethingElse();
${asyncUtil}(() => getByLabelText('email')).then(() => { console.log('done') });
Expand All @@ -36,6 +39,7 @@ ruleTester.run(RULE_NAME, rule, {

...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util promise saved in var and chained with then is valid', () => {
doSomethingElse();
const aPromise = ${asyncUtil}(() => getByLabelText('email'));
Expand All @@ -46,6 +50,7 @@ ruleTester.run(RULE_NAME, rule, {

...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util directly returned in arrow function is valid', async () => {
const makeCustomWait = () =>
${asyncUtil}(() =>
Expand All @@ -57,6 +62,7 @@ ruleTester.run(RULE_NAME, rule, {

...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util explicitly returned in arrow function is valid', async () => {
const makeCustomWait = () => {
return ${asyncUtil}(() =>
Expand All @@ -69,6 +75,7 @@ ruleTester.run(RULE_NAME, rule, {

...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util returned in regular function is valid', async () => {
function makeCustomWait() {
return ${asyncUtil}(() =>
Expand All @@ -81,6 +88,7 @@ ruleTester.run(RULE_NAME, rule, {

...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util promise saved in var and returned in function is valid', async () => {
const makeCustomWait = () => {
const aPromise = ${asyncUtil}(() =>
Expand All @@ -94,8 +102,29 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from 'some-other-library';
test('util "${asyncUtil}" which is not related to testing library is valid', async () => {
doSomethingElse();
${asyncUtil}();
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import * as asyncUtils from 'some-other-library';
test('util "asyncUtils.${asyncUtil}" which is not related to testing library is valid', async () => {
doSomethingElse();
asyncUtils.${asyncUtil}();
});
`,
})),

{
code: `test('waitForElementToBeRemoved receiving element rather than callback is valid', async () => {
code: `
import { waitForElementToBeRemoved } from '@testing-library/dom';
test('waitForElementToBeRemoved receiving element rather than callback is valid', async () => {
doSomethingElse();
const emailInput = getByLabelText('email');
await waitForElementToBeRemoved(emailInput);
Expand All @@ -114,33 +143,46 @@ ruleTester.run(RULE_NAME, rule, {
invalid: [
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util not waited', () => {
doSomethingElse();
${asyncUtil}(() => getByLabelText('email'));
});
`,
errors: [{ line: 4, messageId: 'awaitAsyncUtil' }],
errors: [{ line: 5, messageId: 'awaitAsyncUtil' }],
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import * as asyncUtil from '@testing-library/dom';
test('asyncUtil.${asyncUtil} util not waited', () => {
doSomethingElse();
asyncUtil.${asyncUtil}(() => getByLabelText('email'));
});
`,
errors: [{ line: 5, messageId: 'awaitAsyncUtil' }],
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util promise saved not waited', () => {
doSomethingElse();
const aPromise = ${asyncUtil}(() => getByLabelText('email'));
});
`,
errors: [{ line: 4, column: 28, messageId: 'awaitAsyncUtil' }],
errors: [{ line: 5, column: 28, messageId: 'awaitAsyncUtil' }],
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('several ${asyncUtil} utils not waited', () => {
const aPromise = ${asyncUtil}(() => getByLabelText('username'));
doSomethingElse(aPromise);
${asyncUtil}(() => getByLabelText('email'));
});
`,
errors: [
{ line: 3, column: 28, messageId: 'awaitAsyncUtil' },
{ line: 5, column: 11, messageId: 'awaitAsyncUtil' },
{ line: 4, column: 28, messageId: 'awaitAsyncUtil' },
{ line: 6, column: 11, messageId: 'awaitAsyncUtil' },
],
})),
],
Expand Down