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

* fix: false positive for await-async-query * refactor: async utils used in different async rules * test: fix coverage by including rejects
testing-library · Aug 5, 2020 · cbdfd5f · cbdfd5f
1 parent 6005415
commit cbdfd5f
Show file tree

Hide file tree

Showing 5 changed files with 224 additions and 246 deletions.
diff --git a/lib/node-utils.ts b/lib/node-utils.ts
@@ -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;
 }
@@ -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) &&
@@ -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)) || [];
 }
diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts
@@ -1,41 +1,21 @@
 import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
-import { getDocsUrl } from '../utils';
+import { getDocsUrl, LIBRARY_MODULES } from '../utils';
 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;
@@ -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
       ) {
@@ -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: {
@@ -129,7 +121,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
                 !isAwaited(referenceNode.parent) &&
                 !isPromiseResolved(referenceNode)
               ) {
-                context.report({
+                report({
                   node,
                   messageId: 'awaitAsyncQuery',
                   data: {

diff --git a/lib/rules/await-async-utils.ts b/lib/rules/await-async-utils.ts
@@ -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: {
@@ -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;
 
@@ -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 &&

diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts
@@ -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: {
@@ -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,