refactor(no-promise-in-fire-event): use custom rule creator (#266)

Belco90 · web-flow · commit 6f506ee86ca9 · 2020-12-07T22:31:13.000+01:00
* refactor(await-async-utils): create rule with custom creator

* test(await-async-utils): remove outdated comment

* docs(no-promise-in-fire-event): improve description and examples

* docs(no-promise-in-fire-event): improve invalid errors checks

* refactor(no-promise-in-fire-event): use custom rule creator and helpers

* feat(no-promise-in-fire-event): detect promise in variable references

* docs(no-promise-in-fire-event): update examples

* test(no-promise-in-fire-event): increase coverage up to 100%
diff --git a/docs/rules/no-promise-in-fire-event.md b/docs/rules/no-promise-in-fire-event.md
@@ -1,33 +1,45 @@
 # Disallow the use of promises passed to a `fireEvent` method (no-promise-in-fire-event)
 
-The `fireEvent` method expects that a DOM element is passed.
+Methods from `fireEvent` expect to receive a DOM element. Passing a promise will end up in an error, so it must be prevented.
 
 Examples of **incorrect** code for this rule:
 
 ```js
 import { screen, fireEvent } from '@testing-library/react';
 
-// usage of findBy queries
+// usage of unhandled findBy queries
 fireEvent.click(screen.findByRole('button'));
 
-// usage of promises
-fireEvent.click(new Promise(jest.fn())
+// usage of unhandled promises
+fireEvent.click(new Promise(jest.fn()));
+
+// usage of references to unhandled promises
+const promise = new Promise();
+fireEvent.click(promise);
+
+const anotherPromise = screen.findByRole('button');
+fireEvent.click(anotherPromise);
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
 import { screen, fireEvent } from '@testing-library/react';
 
-// use getBy queries
+// usage of getBy queries
 fireEvent.click(screen.getByRole('button'));
 
-// use awaited findBy queries
+// usage of awaited findBy queries
 fireEvent.click(await screen.findByRole('button'));
 
-// this won't give a linting error, but it will throw a runtime error
+// usage of references to handled promises
 const promise = new Promise();
-fireEvent.click(promise)`,
+const element = await promise;
+fireEvent.click(element);
+
+const anotherPromise = screen.findByRole('button');
+const button = await anotherPromise;
+fireEvent.click(button);
 ```
 
 ## Further Reading
diff --git a/lib/node-utils.ts b/lib/node-utils.ts
@@ -83,12 +83,6 @@ export function isBlockStatement(
   return node?.type === AST_NODE_TYPES.BlockStatement;
 }
 
-export function isVariableDeclarator(
-  node: TSESTree.Node
-): node is TSESTree.VariableDeclarator {
-  return node?.type === AST_NODE_TYPES.VariableDeclarator;
-}
-
 export function isObjectPattern(
   node: TSESTree.Node
 ): node is TSESTree.ObjectPattern {
@@ -191,14 +185,6 @@ export function isImportDeclaration(
   return node?.type === AST_NODE_TYPES.ImportDeclaration;
 }
 
-export function isAwaited(node: TSESTree.Node): boolean {
-  return (
-    ASTUtils.isAwaitExpression(node) ||
-    isArrowFunctionExpression(node) ||
-    isReturnStatement(node)
-  );
-}
-
 export function hasChainedThen(node: TSESTree.Node): boolean {
   const parent = node.parent;
 
@@ -211,11 +197,16 @@ export function hasChainedThen(node: TSESTree.Node): boolean {
   return hasThenProperty(parent);
 }
 
+export function isPromiseIdentifier(
+  node: TSESTree.Node
+): node is TSESTree.Identifier & { name: 'Promise' } {
+  return ASTUtils.isIdentifier(node) && node.name === 'Promise';
+}
+
 export function isPromiseAll(node: TSESTree.CallExpression): boolean {
   return (
     isMemberExpression(node.callee) &&
-    ASTUtils.isIdentifier(node.callee.object) &&
-    node.callee.object.name === 'Promise' &&
+    isPromiseIdentifier(node.callee.object) &&
     ASTUtils.isIdentifier(node.callee.property) &&
     node.callee.property.name === 'all'
   );
@@ -224,8 +215,7 @@ export function isPromiseAll(node: TSESTree.CallExpression): boolean {
 export function isPromiseAllSettled(node: TSESTree.CallExpression): boolean {
   return (
     isMemberExpression(node.callee) &&
-    ASTUtils.isIdentifier(node.callee.object) &&
-    node.callee.object.name === 'Promise' &&
+    isPromiseIdentifier(node.callee.object) &&
     ASTUtils.isIdentifier(node.callee.property) &&
     node.callee.property.name === 'allSettled'
   );
@@ -304,7 +294,7 @@ export function getVariableReferences(
   node: TSESTree.Node
 ): TSESLint.Scope.Reference[] {
   return (
-    (isVariableDeclarator(node) &&
+    (ASTUtils.isVariableDeclarator(node) &&
       context.getDeclaredVariables(node)[0]?.references?.slice(1)) ||
     []
   );
diff --git a/lib/rules/no-promise-in-fire-event.ts b/lib/rules/no-promise-in-fire-event.ts
@@ -1,20 +1,18 @@
+import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils';
+import { createTestingLibraryRule } from '../create-testing-library-rule';
 import {
-  TSESTree,
-  ESLintUtils,
-  ASTUtils,
-} from '@typescript-eslint/experimental-utils';
-import { getDocsUrl, ASYNC_QUERIES_VARIANTS } from '../utils';
-import {
-  isNewExpression,
-  isImportSpecifier,
+  findClosestCallExpressionNode,
+  getIdentifierNode,
   isCallExpression,
+  isNewExpression,
+  isPromiseIdentifier,
 } from '../node-utils';
 
 export const RULE_NAME = 'no-promise-in-fire-event';
 export type MessageIds = 'noPromiseInFireEvent';
 type Options = [];
 
-export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
+export default createTestingLibraryRule<Options, MessageIds>({
   name: RULE_NAME,
   meta: {
     type: 'problem',
@@ -28,49 +26,74 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
       noPromiseInFireEvent:
         "A promise shouldn't be passed to a `fireEvent` method, instead pass the DOM element",
     },
-    fixable: 'code',
+    fixable: null,
     schema: [],
   },
   defaultOptions: [],
 
-  create(context) {
-    return {
-      'ImportDeclaration[source.value=/testing-library/]'(
-        node: TSESTree.ImportDeclaration
-      ) {
-        const fireEventImportNode = node.specifiers.find(
-          (specifier) =>
-            isImportSpecifier(specifier) &&
-            specifier.imported &&
-            'fireEvent' === specifier.imported.name
-        ) as TSESTree.ImportSpecifier;
+  create(context, _, helpers) {
+    function checkSuspiciousNode(
+      node: TSESTree.Node,
+      originalNode?: TSESTree.Node
+    ): void {
+      if (ASTUtils.isAwaitExpression(node)) {
+        return;
+      }
 
-        const { references } = context.getDeclaredVariables(
-          fireEventImportNode
-        )[0];
+      if (isNewExpression(node)) {
+        if (isPromiseIdentifier(node.callee)) {
+          return context.report({
+            node: originalNode ?? node,
+            messageId: 'noPromiseInFireEvent',
+          });
+        }
+      }
 
-        for (const reference of references) {
-          const referenceNode = reference.identifier;
-          const callExpression = referenceNode.parent
-            .parent as TSESTree.CallExpression;
-          const [element] = callExpression.arguments as TSESTree.Node[];
-          if (isCallExpression(element) || isNewExpression(element)) {
-            const methodName = ASTUtils.isIdentifier(element.callee)
-              ? element.callee.name
-              : ((element.callee as TSESTree.MemberExpression)
-                  .property as TSESTree.Identifier).name;
+      if (isCallExpression(node)) {
+        const domElementIdentifier = getIdentifierNode(node);
+
+        if (
+          helpers.isAsyncQuery(domElementIdentifier) ||
+          isPromiseIdentifier(domElementIdentifier)
+        ) {
+          return context.report({
+            node: originalNode ?? node,
+            messageId: 'noPromiseInFireEvent',
+          });
+        }
+      }
+
+      if (ASTUtils.isIdentifier(node)) {
+        const nodeVariable = ASTUtils.findVariable(
+          context.getScope(),
+          node.name
+        );
+        if (!nodeVariable || !nodeVariable.defs) {
+          return;
+        }
 
-            if (
-              ASYNC_QUERIES_VARIANTS.some((q) => methodName.startsWith(q)) ||
-              methodName === 'Promise'
-            ) {
-              context.report({
-                node: element,
-                messageId: 'noPromiseInFireEvent',
-              });
-            }
-          }
+        for (const definition of nodeVariable.defs) {
+          const variableDeclarator = definition.node as TSESTree.VariableDeclarator;
+          checkSuspiciousNode(variableDeclarator.init, node);
         }
+      }
+    }
+
+    return {
+      'CallExpression Identifier'(node: TSESTree.Identifier) {
+        if (!helpers.isFireEventMethod(node)) {
+          return;
+        }
+
+        const closestCallExpression = findClosestCallExpressionNode(node, true);
+
+        if (!closestCallExpression) {
+          return;
+        }
+
+        const domElementArgument = closestCallExpression.arguments[0];
+
+        checkSuspiciousNode(domElementArgument);
       },
     };
   },
diff --git a/tests/lib/rules/no-promise-in-fire-event.test.ts b/tests/lib/rules/no-promise-in-fire-event.test.ts
