refactor(render-result-naming-convention): migrate to v4 (#280)

* docs: add comments to main parts to be modified

* refactor(render-result-naming-convention): first approach for new helper

First implementation of isRenderUtil helper, and use it within this rule.

* refactor(aggressive-render): update criteria to consider valid renders

Before, it was checking if the name of the method started by "render". Now, it checks if the name of the method contains render.

* feat(aggressive-render): keep aggressive module reporting in mind

Depending on aggressive module reporting, isRenderUtil needs to check if node comes from valid Testing Library module or not.

* test(render-result-naming-convention): move valid to invalid tests

* docs(aggressive-reporting): improve jsdocs

* test(create-testing-library-rule): cases for render

Author: Belco90

lib/detect-testing-library-utils.ts

@@ -5,6 +5,7 @@ import {
 } from '@typescript-eslint/experimental-utils';
 import {
   getAssertNodeInfo,
+  getIdentifierNode,
   getImportModuleName,
   ImportModuleNode,
   isImportDeclaration,
@@ -24,6 +25,7 @@ import {
 export type TestingLibrarySettings = {
   'testing-library/module'?: string;
   'testing-library/filename-pattern'?: string;
+  'testing-library/custom-renders'?: string[];
 };
 
 export type TestingLibraryContext<
@@ -60,6 +62,7 @@ export type DetectionHelpers = {
   isCustomQuery: (node: TSESTree.Identifier) => boolean;
   isAsyncUtil: (node: TSESTree.Identifier) => boolean;
   isFireEventMethod: (node: TSESTree.Identifier) => boolean;
+  isRenderUtil: (node: TSESTree.Node) => boolean;
   isPresenceAssert: (node: TSESTree.MemberExpression) => boolean;
   isAbsenceAssert: (node: TSESTree.MemberExpression) => boolean;
   canReportErrors: () => boolean;
@@ -74,6 +77,7 @@ export type DetectionHelpers = {
 const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$';
 
 const FIRE_EVENT_NAME = 'fireEvent';
+const RENDER_NAME = 'render';
 
 /**
  * Enhances a given rule `create` with helpers to detect Testing Library utils.
@@ -95,15 +99,31 @@ export function detectTestingLibraryUtils<
     const filenamePattern =
       context.settings['testing-library/filename-pattern'] ??
       DEFAULT_FILENAME_PATTERN;
+    const customRenders = context.settings['testing-library/custom-renders'];
 
     /**
-     * Determines whether aggressive reporting is enabled or not.
+     * Determines whether aggressive module reporting is enabled or not.
      *
-     * Aggressive reporting is considered as enabled when:
-     * - custom module is not set (so we need to assume everything
-     *    matching TL utils is related to TL no matter where it was imported from)
+     * This aggressive reporting mechanism is considered as enabled when custom
+     * module is not set, so we need to assume everything matching Testing
+     * Library utils is related to Testing Library no matter from where module
+     * they are coming from. Otherwise, this aggressive reporting mechanism is
+     * opted-out in favour to report only those utils coming from Testing
+     * Library package or custom module set up on settings.
      */
-    const isAggressiveReportingEnabled = () => !customModule;
+    const isAggressiveModuleReportingEnabled = () => !customModule;
+
+    /**
+     * Determines whether aggressive render reporting is enabled or not.
+     *
+     * This aggressive reporting mechanism is considered as enabled when custom
+     * renders are not set, so we need to assume every method containing
+     * "render" is a valid Testing Library `render`. Otherwise, this aggressive
+     * reporting mechanism is opted-out in favour to report only `render` or
+     * names set up on custom renders setting.
+     */
+    const isAggressiveRenderReportingEnabled = () =>
+      !Array.isArray(customRenders) || customRenders.length === 0;
 
     // Helpers for Testing Library detection.
     const getTestingLibraryImportNode: DetectionHelpers['getTestingLibraryImportNode'] = () => {
@@ -135,7 +155,7 @@ export function detectTestingLibraryUtils<
      * or custom module are imported.
      */
     const isTestingLibraryImported: DetectionHelpers['isTestingLibraryImported'] = () => {
-      if (isAggressiveReportingEnabled()) {
+      if (isAggressiveModuleReportingEnabled()) {
         return true;
       }
 
@@ -215,7 +235,7 @@ export function detectTestingLibraryUtils<
         fireEventUtilName = ASTUtils.isIdentifier(fireEventUtil)
           ? fireEventUtil.name
           : fireEventUtil.local.name;
-      } else if (isAggressiveReportingEnabled()) {
+      } else if (isAggressiveModuleReportingEnabled()) {
         fireEventUtilName = FIRE_EVENT_NAME;
       }
 
@@ -256,6 +276,48 @@ export function detectTestingLibraryUtils<
       return regularCall || wildcardCall;
     };
 
+    /**
+     * Determines whether a given node is a valid render util or not.
+     *
+     * A node will be interpreted as a valid render based on two conditions:
+     * the name matches with a valid "render" option, and the node is coming
+     * from Testing Library module. This depends on:
+     *
+     * - Aggressive render reporting: if enabled, then every node name
+     * containing "render" will be assumed as Testing Library render util.
+     * Otherwise, it means `custom-modules` has been set up, so only those nodes
+     * named as "render" or some of the `custom-modules` options will be
+     * considered as Testing Library render util.
+     * - Aggressive module reporting: if enabled, then it doesn't matter from
+     * where the given node was imported from as it will be considered part of
+     * Testing Library. Otherwise, it means `custom-module` has been set up, so
+     * only those nodes coming from Testing Library will be considered as valid.
+     */
+    const isRenderUtil: DetectionHelpers['isRenderUtil'] = (node) => {
+      const identifier = getIdentifierNode(node);
+
+      if (!identifier) {
+        return false;
+      }
+
+      const isNameMatching = (function () {
+        if (isAggressiveRenderReportingEnabled()) {
+          return identifier.name.toLowerCase().includes(RENDER_NAME);
+        }
+
+        return [RENDER_NAME, ...customRenders].includes(identifier.name);
+      })();
+
+      if (!isNameMatching) {
+        return false;
+      }
+
+      return (
+        isAggressiveModuleReportingEnabled() ||
+        isNodeComingFromTestingLibrary(identifier)
+      );
+    };
+
     /**
      * Determines whether a given MemberExpression node is a presence assert
      *
@@ -376,6 +438,7 @@ export function detectTestingLibraryUtils<
       isCustomQuery,
       isAsyncUtil,
       isFireEventMethod,
+      isRenderUtil,
       isPresenceAssert,
       isAbsenceAssert,
       canReportErrors,

lib/rules/render-result-naming-convention.ts

@@ -1,27 +1,18 @@
-import {
-  ESLintUtils,
-  TSESTree,
-  ASTUtils,
-} from '@typescript-eslint/experimental-utils';
-import { getDocsUrl, hasTestingLibraryImportModule } from '../utils';
-import {
-  isCallExpression,
-  isImportSpecifier,
-  isMemberExpression,
-  isObjectPattern,
-  isRenderVariableDeclarator,
-} from '../node-utils';
+import { createTestingLibraryRule } from '../create-testing-library-rule';
+import { isObjectPattern } from '../node-utils';
+import { ASTUtils } from '@typescript-eslint/experimental-utils';
 
 export const RULE_NAME = 'render-result-naming-convention';
 export type MessageIds = 'renderResultNamingConvention';
-type Options = [{ renderFunctions?: string[] }];
+
+type Options = [];
 
 const ALLOWED_VAR_NAMES = ['view', 'utils'];
 const ALLOWED_VAR_NAMES_TEXT = ALLOWED_VAR_NAMES.map(
   (name) => `\`${name}\``
 ).join(', ');
 
-export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
+export default createTestingLibraryRule<Options, MessageIds>({
   name: RULE_NAME,
   meta: {
     type: 'suggestion',
@@ -31,103 +22,27 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
       recommended: false,
     },
     messages: {
-      renderResultNamingConvention: `\`{{ varName }}\` is not a recommended name for \`render\` returned value. Instead, you should destructure it, or call it using one of the valid choices: ${ALLOWED_VAR_NAMES_TEXT}`,
+      renderResultNamingConvention: `\`{{ renderResultName }}\` is not a recommended name for \`render\` returned value. Instead, you should destructure it, or name it using one of: ${ALLOWED_VAR_NAMES_TEXT}`,
     },
     fixable: null,
-    schema: [
-      {
-        type: 'object',
-        properties: {
-          renderFunctions: {
-            type: 'array',
-          },
-        },
-      },
-    ],
+    schema: [],
   },
-  defaultOptions: [
-    {
-      renderFunctions: [],
-    },
-  ],
-
-  create(context, [options]) {
-    const { renderFunctions } = options;
-    let renderAlias: string | undefined;
-    let wildcardImportName: string | undefined;
+  defaultOptions: [],
 
+  create(context, _, helpers) {
     return {
-      // check named imports
-      ImportDeclaration(node: TSESTree.ImportDeclaration) {
-        if (!hasTestingLibraryImportModule(node)) {
-          return;
-        }
-        const renderImport = node.specifiers.find(
-          (node) => isImportSpecifier(node) && node.imported.name === 'render'
-        );
-
-        if (!renderImport) {
-          return;
-        }
-
-        renderAlias = renderImport.local.name;
-      },
-      // check wildcard imports
-      'ImportDeclaration ImportNamespaceSpecifier'(
-        node: TSESTree.ImportNamespaceSpecifier
-      ) {
-        if (
-          !hasTestingLibraryImportModule(
-            node.parent as TSESTree.ImportDeclaration
-          )
-        ) {
+      VariableDeclarator(node) {
+        if (!helpers.isRenderUtil(node.init)) {
           return;
         }
 
-        wildcardImportName = node.local.name;
-      },
-      VariableDeclarator(node: TSESTree.VariableDeclarator) {
         // check if destructuring return value from render
         if (isObjectPattern(node.id)) {
           return;
         }
 
-        const isValidRenderDeclarator = isRenderVariableDeclarator(node, [
-          ...renderFunctions,
-          renderAlias,
-        ]);
-        const isValidWildcardImport = !!wildcardImportName;
-
-        // check if is a Testing Library related import
-        if (!isValidRenderDeclarator && !isValidWildcardImport) {
-          return;
-        }
-
-        const renderFunctionName =
-          isCallExpression(node.init) &&
-          ASTUtils.isIdentifier(node.init.callee) &&
-          node.init.callee.name;
-
-        const renderFunctionObjectName =
-          isCallExpression(node.init) &&
-          isMemberExpression(node.init.callee) &&
-          ASTUtils.isIdentifier(node.init.callee.property) &&
-          ASTUtils.isIdentifier(node.init.callee.object) &&
-          node.init.callee.property.name === 'render' &&
-          node.init.callee.object.name;
-
-        const isRenderAlias = !!renderAlias;
-        const isCustomRender = renderFunctions.includes(renderFunctionName);
-        const isWildCardRender =
-          renderFunctionObjectName &&
-          renderFunctionObjectName === wildcardImportName;
-
-        // check if is a qualified render function
-        if (!isRenderAlias && !isCustomRender && !isWildCardRender) {
-          return;
-        }
-
         const renderResultName = ASTUtils.isIdentifier(node.id) && node.id.name;
+
         const isAllowedRenderResultName = ALLOWED_VAR_NAMES.includes(
           renderResultName
         );
@@ -141,7 +56,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
           node,
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: renderResultName,
+            renderResultName,
           },
         });
       },