Skip to content

fix(await-async-query): false positives for await-async-query #208

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 3 commits into from
Aug 5, 2020
Merged

fix(await-async-query): false positives for await-async-query #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a8e7110
fix: false positive for await-async-query
thomaslombart Aug 4, 2020
f447291
refactor: async utils used in different async rules
thomaslombart Aug 4, 2020
08f7452
test: fix coverage by including rejects
thomaslombart Aug 4, 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
41 changes: 33 additions & 8 deletions lib/node-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/experimental-utils';
import { RuleContext } from '@typescript-eslint/experimental-utils/dist/ts-eslint';

export function isCallExpression(
node: TSESTree.Node
): node is TSESTree.CallExpression {
return node && node.type === AST_NODE_TYPES.CallExpression;
}

export function isAwaitExpression(
node: TSESTree.Node
): node is TSESTree.AwaitExpression {
return node && node.type === AST_NODE_TYPES.AwaitExpression;
}

export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier {
return node && node.type === AST_NODE_TYPES.Identifier;
}
Expand Down Expand Up @@ -95,6 +90,10 @@ export function findClosestCallNode(
}
}

export function isObjectExpression(node: TSESTree.Expression): node is TSESTree.ObjectExpression {
return node?.type === AST_NODE_TYPES.ObjectExpression
}

export function hasThenProperty(node: TSESTree.Node) {
return (
isMemberExpression(node) &&
Expand All @@ -103,10 +102,36 @@ export function hasThenProperty(node: TSESTree.Node) {
);
}

export function isAwaitExpression(
node: TSESTree.Node
): node is TSESTree.AwaitExpression {
return node && node.type === AST_NODE_TYPES.AwaitExpression;
}

export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression {
return node && node.type === AST_NODE_TYPES.ArrowFunctionExpression
}

export function isObjectExpression(node: TSESTree.Expression): node is TSESTree.ObjectExpression {
return node?.type === AST_NODE_TYPES.ObjectExpression
export function isReturnStatement(node: TSESTree.Node): node is TSESTree.ReturnStatement {
return node && node.type === AST_NODE_TYPES.ReturnStatement
}

export function isAwaited(node: TSESTree.Node) {
return isAwaitExpression(node) || isArrowFunctionExpression(node) || isReturnStatement(node)
}

export function isPromiseResolved(node: TSESTree.Node) {
const parent = node.parent;

// wait(...).then(...)
if (isCallExpression(parent)) {
return hasThenProperty(parent.parent);
}

// promise.then(...)
return hasThenProperty(parent);
}

export function getVariableReferences(context: RuleContext<string, []>, node: TSESTree.Node) {
return (isVariableDeclarator(node) && context.getDeclaredVariables(node)[0].references.slice(1)) || [];
}
62 changes: 27 additions & 35 deletions lib/rules/await-async-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,21 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl } from '../utils';
import { getDocsUrl, LIBRARY_MODULES } from '../utils';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's an improvement to do on the LIBRARY_MODULES naming. Indeed, it doesn't make sense to make the rule fire on the async queries for @testing-library/cypress. Right now it works because it's not included in the LIBRARY_MODULES constants, however it's still a library module.

import {
isVariableDeclarator,
hasThenProperty,
isCallExpression,
isIdentifier,
isMemberExpression,
isAwaited,
isPromiseResolved,
getVariableReferences,
} from '../node-utils';
import { ReportDescriptor } from '@typescript-eslint/experimental-utils/dist/ts-eslint';

export const RULE_NAME = 'await-async-query';
export type MessageIds = 'awaitAsyncQuery';
type Options = [];

const VALID_PARENTS = [
'AwaitExpression',
'ArrowFunctionExpression',
'ReturnStatement',
];

const ASYNC_QUERIES_REGEXP = /^find(All)?By(LabelText|PlaceholderText|Text|AltText|Title|DisplayValue|Role|TestId)$/;

function isAwaited(node: TSESTree.Node) {
return VALID_PARENTS.includes(node.type);
}

function isPromiseResolved(node: TSESTree.Node) {
const parent = node.parent;

// findByText("foo").then(...)
if (isCallExpression(parent)) {
return hasThenProperty(parent.parent);
}

// promise.then(...)
return hasThenProperty(parent);
}

function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean {
if (!node.parent) {
return false;
Expand Down Expand Up @@ -79,14 +59,33 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
node: TSESTree.Identifier | TSESTree.MemberExpression;
queryName: string;
}[] = [];

const isQueryUsage = (
node: TSESTree.Identifier | TSESTree.MemberExpression
) =>
!isAwaited(node.parent.parent) &&
!isPromiseResolved(node) &&
!hasClosestExpectResolvesRejects(node);

let hasImportedFromTestingLibraryModule = false;

function report(params: ReportDescriptor<'awaitAsyncQuery'>) {
if (hasImportedFromTestingLibraryModule) {
context.report(params);
}
}

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

if (LIBRARY_MODULES.includes(module)) {
hasImportedFromTestingLibraryModule = true;
}
},
[`CallExpression > Identifier[name=${ASYNC_QUERIES_REGEXP}]`](
node: TSESTree.Identifier
) {
Expand All @@ -105,17 +104,10 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
},
'Program:exit'() {
testingLibraryQueryUsage.forEach(({ node, queryName }) => {
const variableDeclaratorParent = node.parent.parent;

const references =
(isVariableDeclarator(variableDeclaratorParent) &&
context
.getDeclaredVariables(variableDeclaratorParent)[0]
.references.slice(1)) ||
[];
const references = getVariableReferences(context, node.parent.parent);

if (references && references.length === 0) {
context.report({
report({
node,
messageId: 'awaitAsyncQuery',
data: {
Expand All @@ -129,7 +121,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
!isAwaited(referenceNode.parent) &&
!isPromiseResolved(referenceNode)
) {
context.report({
report({
node,
messageId: 'awaitAsyncQuery',
data: {
Expand Down
57 changes: 20 additions & 37 deletions lib/rules/await-async-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,18 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';

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

export const RULE_NAME = 'await-async-utils';
export type MessageIds = 'awaitAsyncUtil';
type Options = [];

const VALID_PARENTS = [
'AwaitExpression',
'ArrowFunctionExpression',
'ReturnStatement',
];

const ASYNC_UTILS_REGEXP = new RegExp(`^(${ASYNC_UTILS.join('|')})$`);

function isAwaited(node: TSESTree.Node) {
return VALID_PARENTS.includes(node.type);
}

function isPromiseResolved(node: TSESTree.Node) {
const parent = node.parent;

// wait(...).then(...)
if (isCallExpression(parent)) {
return hasThenProperty(parent.parent);
}

// promise.then(...)
return hasThenProperty(parent);
}

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
name: RULE_NAME,
meta: {
Expand All @@ -49,12 +31,17 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
defaultOptions: [],

create(context) {
const asyncUtilsUsage: Array<{ node: TSESTree.Identifier | TSESTree.MemberExpression, name: string }> = [];
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);
'ImportDeclaration > ImportSpecifier,ImportNamespaceSpecifier'(
node: TSESTree.Node
) {
const parent = node.parent as TSESTree.ImportDeclaration;

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

Expand All @@ -78,28 +65,24 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
const identifier = memberExpression.object as TSESTree.Identifier;
const memberExpressionName = identifier.name;

asyncUtilsUsage.push({ node: memberExpression, name: memberExpressionName });
asyncUtilsUsage.push({
node: memberExpression,
name: memberExpressionName,
});
},
'Program:exit'() {
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(object.name);
}

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

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

const references =
(variableDeclaratorParent.type === 'VariableDeclarator' &&
context
.getDeclaredVariables(variableDeclaratorParent)[0]
.references.slice(1)) ||
[];
const references = getVariableReferences(context, node.parent.parent);

if (
references &&
Expand Down
22 changes: 2 additions & 20 deletions lib/rules/await-fire-event.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,10 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl } from '../utils';
import { isIdentifier, isCallExpression, hasThenProperty } from '../node-utils';
import { isIdentifier, isAwaited, isPromiseResolved } from '../node-utils';

export const RULE_NAME = 'await-fire-event';
export type MessageIds = 'awaitFireEvent';
type Options = [];

const VALID_PARENTS = [
'AwaitExpression',
'ArrowFunctionExpression',
'ReturnStatement',
];

function isAwaited(node: TSESTree.Node) {
return VALID_PARENTS.includes(node.type);
}

function isPromiseResolved(node: TSESTree.Node) {
const parent = node.parent.parent;

// fireEvent.click().then(...)
return isCallExpression(parent) && hasThenProperty(parent.parent);
}

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
name: RULE_NAME,
meta: {
Expand Down Expand Up @@ -51,7 +33,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
if (
isIdentifier(fireEventMethodNode) &&
!isAwaited(node.parent.parent.parent) &&
!isPromiseResolved(fireEventMethodNode)
!isPromiseResolved(fireEventMethodNode.parent)
) {
context.report({
node: fireEventMethodNode,
Expand Down
Loading