refacto(no-wait-for-side-effects): migrate to v4 (#300)

* test: improve errors location asserts

* refactor: use new rule creator

* refactor: improve error reported location

* refactor: use new helpers for detection

* test: add more cases

* feat: detect properly if fireEvent and userEvent should be reported

* test: add cases for increasing coverage up to 100%

* refactor: rename rule for consistency

* docs: remove duplicated no-wait-for-snapshot row

* fix: get identifier node simpler

README.md

@@ -23,7 +23,9 @@
 [![Tweet][tweet-badge]][tweet-url]
 
 <!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
+
 [![All Contributors](https://img.shields.io/badge/all_contributors-36-orange.svg?style=flat-square)](#contributors-)
+
 <!-- ALL-CONTRIBUTORS-BADGE:END -->
 
 ## Installation
@@ -139,9 +141,8 @@ To enable this configuration use the `extends` property in your
 | [no-node-access](docs/rules/no-node-access.md)                                   | Disallow direct Node access                                                              | ![angular-badge][] ![react-badge][] ![vue-badge][]                |                    |
 | [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md)               | Disallow the use of promises passed to a `fireEvent` method                              |                                                                   |                    |
 | [no-render-in-setup](docs/rules/no-render-in-setup.md)                           | Disallow the use of `render` in setup functions                                          |                                                                   |                    |
-| [no-side-effects-wait-for](docs/rules/no-side-effects-wait-for.md)               | Disallow the use of side effects inside `waitFor`                                        |                                                                   |                    |
 | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md)           | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved`                   | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] |                    |
-| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md)                       | Ensures no snapshot is generated inside of a `waitFor` call                              |                                                                   |                    |
+| [no-wait-for-side-effects](docs/rules/no-wait-for-side-effects.md)               | Disallow the use of side effects inside `waitFor`                                        |                                                                   |                    |
 | [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md)                       | Ensures no snapshot is generated inside of a `waitFor` call                              |                                                                   |                    |
 | [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md)                   | Suggest using explicit assertions rather than just `getBy*` queries                      |                                                                   |                    |
 | [prefer-find-by](docs/rules/prefer-find-by.md)                                   | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries               | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |

docs/rules/no-side-effects-wait-for.md → docs/rules/no-wait-for-side-effects.md

@@ -1,4 +1,4 @@
-# Side effects inside `waitFor` are not preferred (no-side-effects-wait-for)
+# Side effects inside `waitFor` are not preferred (no-wait-for-side-effects)
 
 ## Rule Details
 

lib/detect-testing-library-utils.ts

@@ -96,6 +96,8 @@ export interface DetectionHelpers {
   isQuery: IsQueryFn;
   isCustomQuery: IsCustomQueryFn;
   isAsyncUtil: IsAsyncUtilFn;
+  isFireEventUtil: (node: TSESTree.Identifier) => boolean;
+  isUserEventUtil: (node: TSESTree.Identifier) => boolean;
   isFireEventMethod: IsFireEventMethodFn;
   isRenderUtil: IsRenderUtilFn;
   isRenderVariableDeclarator: IsRenderVariableDeclaratorFn;
@@ -107,7 +109,6 @@ export interface DetectionHelpers {
   isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn;
 }
 
-const FIRE_EVENT_NAME = 'fireEvent';
 const RENDER_NAME = 'render';
 
 /**
@@ -173,6 +174,69 @@ export function detectTestingLibraryUtils<
       return isNodeComingFromTestingLibrary(referenceNodeIdentifier);
     }
 
+    /**
+     * Determines whether a given node is a simulate event util related to
+     * Testing Library or not.
+     *
+     * In order to determine this, the node must match:
+     * - indicated simulate event name: fireEvent or userEvent
+     * - imported from valid Testing Library module (depends on Aggressive
+     * Reporting)
+     *
+     */
+    function isTestingLibrarySimulateEventUtil(
+      node: TSESTree.Identifier,
+      utilName: 'fireEvent' | 'userEvent'
+    ): boolean {
+      const simulateEventUtil = findImportedUtilSpecifier(utilName);
+      let simulateEventUtilName: string | undefined;
+
+      if (simulateEventUtil) {
+        simulateEventUtilName = ASTUtils.isIdentifier(simulateEventUtil)
+          ? simulateEventUtil.name
+          : simulateEventUtil.local.name;
+      } else if (isAggressiveModuleReportingEnabled()) {
+        simulateEventUtilName = utilName;
+      }
+
+      if (!simulateEventUtilName) {
+        return false;
+      }
+
+      const parentMemberExpression:
+        | TSESTree.MemberExpression
+        | undefined = isMemberExpression(node.parent) ? node.parent : undefined;
+
+      if (!parentMemberExpression) {
+        return false;
+      }
+
+      // make sure that given node it's not fireEvent/userEvent object itself
+      if (
+        [simulateEventUtilName, utilName].includes(node.name) ||
+        (ASTUtils.isIdentifier(parentMemberExpression.object) &&
+          parentMemberExpression.object.name === node.name)
+      ) {
+        return false;
+      }
+
+      // check fireEvent.click()/userEvent.click() usage
+      const regularCall =
+        ASTUtils.isIdentifier(parentMemberExpression.object) &&
+        parentMemberExpression.object.name === simulateEventUtilName;
+
+      // check testingLibraryUtils.fireEvent.click() or
+      // testingLibraryUtils.userEvent.click() usage
+      const wildcardCall =
+        isMemberExpression(parentMemberExpression.object) &&
+        ASTUtils.isIdentifier(parentMemberExpression.object.object) &&
+        parentMemberExpression.object.object.name === simulateEventUtilName &&
+        ASTUtils.isIdentifier(parentMemberExpression.object.property) &&
+        parentMemberExpression.object.property.name === utilName;
+
+      return regularCall || wildcardCall;
+    }
+
     /**
      * Determines whether aggressive module reporting is enabled or not.
      *
@@ -308,55 +372,38 @@ export function detectTestingLibraryUtils<
     };
 
     /**
-     * Determines whether a given node is fireEvent method or not
+     * Determines whether a given node is fireEvent util itself or not.
+     *
+     * Not to be confused with {@link isFireEventMethod}
      */
-    const isFireEventMethod: IsFireEventMethodFn = (node) => {
-      const fireEventUtil = findImportedUtilSpecifier(FIRE_EVENT_NAME);
-      let fireEventUtilName: string | undefined;
-
-      if (fireEventUtil) {
-        fireEventUtilName = ASTUtils.isIdentifier(fireEventUtil)
-          ? fireEventUtil.name
-          : fireEventUtil.local.name;
-      } else if (isAggressiveModuleReportingEnabled()) {
-        fireEventUtilName = FIRE_EVENT_NAME;
-      }
-
-      if (!fireEventUtilName) {
-        return false;
-      }
-
-      const parentMemberExpression:
-        | TSESTree.MemberExpression
-        | undefined = isMemberExpression(node.parent) ? node.parent : undefined;
-
-      if (!parentMemberExpression) {
-        return false;
-      }
-
-      // make sure that given node it's not fireEvent object itself
-      if (
-        [fireEventUtilName, FIRE_EVENT_NAME].includes(node.name) ||
-        (ASTUtils.isIdentifier(parentMemberExpression.object) &&
-          parentMemberExpression.object.name === node.name)
-      ) {
-        return false;
-      }
-
-      // check fireEvent.click() usage
-      const regularCall =
-        ASTUtils.isIdentifier(parentMemberExpression.object) &&
-        parentMemberExpression.object.name === fireEventUtilName;
+    const isFireEventUtil = (node: TSESTree.Identifier): boolean => {
+      return isTestingLibraryUtil(
+        node,
+        (identifierNodeName, originalNodeName) => {
+          return [identifierNodeName, originalNodeName].includes('fireEvent');
+        }
+      );
+    };
 
-      // check testingLibraryUtils.fireEvent.click() usage
-      const wildcardCall =
-        isMemberExpression(parentMemberExpression.object) &&
-        ASTUtils.isIdentifier(parentMemberExpression.object.object) &&
-        parentMemberExpression.object.object.name === fireEventUtilName &&
-        ASTUtils.isIdentifier(parentMemberExpression.object.property) &&
-        parentMemberExpression.object.property.name === FIRE_EVENT_NAME;
+    /**
+     * Determines whether a given node is userEvent util itself or not.
+     *
+     * Not to be confused with {@link isUserEventMethod}
+     */
+    const isUserEventUtil = (node: TSESTree.Identifier): boolean => {
+      return isTestingLibraryUtil(
+        node,
+        (identifierNodeName, originalNodeName) => {
+          return [identifierNodeName, originalNodeName].includes('userEvent');
+        }
+      );
+    };
 
-      return regularCall || wildcardCall;
+    /**
+     * Determines whether a given node is fireEvent method or not
+     */
+    const isFireEventMethod: IsFireEventMethodFn = (node) => {
+      return isTestingLibrarySimulateEventUtil(node, 'fireEvent');
     };
 
     /**
@@ -557,6 +604,8 @@ export function detectTestingLibraryUtils<
       isQuery,
       isCustomQuery,
       isAsyncUtil,
+      isFireEventUtil,
+      isUserEventUtil,
       isFireEventMethod,
       isRenderUtil,
       isRenderVariableDeclarator,

lib/index.ts

@@ -20,7 +20,7 @@ import preferUserEvent from './rules/prefer-user-event';
 import preferWaitFor from './rules/prefer-wait-for';
 import noMultipleAssertionsWaitFor from './rules/no-multiple-assertions-wait-for';
 import preferFindBy from './rules/prefer-find-by';
-import noSideEffectsWaitFor from './rules/no-side-effects-wait-for';
+import noWaitForSideEffects from './rules/no-wait-for-side-effects';
 import renderResultNamingConvention from './rules/render-result-naming-convention';
 
 const rules = {
@@ -38,8 +38,8 @@ const rules = {
   'no-node-access': noNodeAccess,
   'no-promise-in-fire-event': noPromiseInFireEvent,
   'no-render-in-setup': noRenderInSetup,
-  'no-side-effects-wait-for': noSideEffectsWaitFor,
   'no-wait-for-empty-callback': noWaitForEmptyCallback,
+  'no-wait-for-side-effects': noWaitForSideEffects,
   'no-wait-for-snapshot': noWaitForSnapshot,
   'prefer-explicit-assert': preferExplicitAssert,
   'prefer-find-by': preferFindBy,

lib/node-utils.ts

@@ -101,6 +101,12 @@ export function isJSXAttribute(
   return node?.type === AST_NODE_TYPES.JSXAttribute;
 }
 
+export function isExpressionStatement(
+  node: TSESTree.Node
+): node is TSESTree.ExpressionStatement {
+  return node?.type === AST_NODE_TYPES.ExpressionStatement;
+}
+
 /**
  * Finds the closest CallExpression node for a given node.
  * @param node
@@ -388,6 +394,10 @@ export function getPropertyIdentifierNode(
     return getPropertyIdentifierNode(node.callee);
   }
 
+  if (isExpressionStatement(node)) {
+    return getPropertyIdentifierNode(node.expression);
+  }
+
   return null;
 }
 

lib/rules/no-wait-for-side-effects.ts

@@ -0,0 +1,67 @@
+import { TSESTree } from '@typescript-eslint/experimental-utils';
+import { getPropertyIdentifierNode } from '../node-utils';
+import { createTestingLibraryRule } from '../create-testing-library-rule';
+
+export const RULE_NAME = 'no-wait-for-side-effects';
+export type MessageIds = 'noSideEffectsWaitFor';
+type Options = [];
+
+export default createTestingLibraryRule<Options, MessageIds>({
+  name: RULE_NAME,
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: "It's preferred to avoid side effects in `waitFor`",
+      category: 'Best Practices',
+      recommended: false,
+    },
+    messages: {
+      noSideEffectsWaitFor:
+        'Avoid using side effects within `waitFor` callback',
+    },
+    fixable: null,
+    schema: [],
+  },
+  defaultOptions: [],
+  create: function (context, _, helpers) {
+    function hasSideEffects(body: Array<TSESTree.Node>): boolean {
+      return body.some((node: TSESTree.ExpressionStatement) => {
+        const expressionIdentifier = getPropertyIdentifierNode(node);
+
+        if (!expressionIdentifier) {
+          return false;
+        }
+
+        return (
+          helpers.isFireEventUtil(expressionIdentifier) ||
+          helpers.isUserEventUtil(expressionIdentifier)
+        );
+      });
+    }
+
+    function reportSideEffects(node: TSESTree.BlockStatement) {
+      const callExpressionNode = node.parent.parent as TSESTree.CallExpression;
+      const callExpressionIdentifier = getPropertyIdentifierNode(
+        callExpressionNode
+      );
+
+      if (!helpers.isAsyncUtil(callExpressionIdentifier, ['waitFor'])) {
+        return;
+      }
+
+      if (!hasSideEffects(node.body)) {
+        return;
+      }
+
+      context.report({
+        node: callExpressionNode,
+        messageId: 'noSideEffectsWaitFor',
+      });
+    }
+
+    return {
+      'CallExpression > ArrowFunctionExpression > BlockStatement': reportSideEffects,
+      'CallExpression > FunctionExpression > BlockStatement': reportSideEffects,
+    };
+  },
+});