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

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

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;
@@ -95,6 +98,7 @@ 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.
@@ -105,6 +109,12 @@ export function detectTestingLibraryUtils<
      */
     const isAggressiveReportingEnabled = () => !customModule;
 
+    /**
+     * TODO
+     */
+    const isAggressiveRenderEnabled = () =>
+      !Array.isArray(customRenders) || customRenders.length === 0;
+
     // Helpers for Testing Library detection.
     const getTestingLibraryImportNode: DetectionHelpers['getTestingLibraryImportNode'] = () => {
       return importedTestingLibraryNode;
@@ -256,6 +266,24 @@ export function detectTestingLibraryUtils<
       return regularCall || wildcardCall;
     };
 
+    /**
+     * TODO
+     */
+    const isRenderUtil: DetectionHelpers['isRenderUtil'] = (node) => {
+      const identifier = getIdentifierNode(node);
+
+      if (!identifier) {
+        return false;
+      }
+
+      if (isAggressiveRenderEnabled()) {
+        // TODO: should this be always true?
+        return identifier.name.toLowerCase().startsWith('render');
+      }
+
+      return ['render', ...customRenders].includes(identifier.name);
+    };
+
     /**
      * Determines whether a given MemberExpression node is a presence assert
      *
@@ -376,6 +404,7 @@ export function detectTestingLibraryUtils<
       isCustomQuery,
       isAsyncUtil,
       isFireEventMethod,
+      isRenderUtil,
       isPresenceAssert,
       isAbsenceAssert,
       canReportErrors,

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

@@ -1,29 +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';
 
-// TODO: remove renderFunctions option first, and then move it to ESLint settings
-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',
@@ -33,109 +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 {
-      // TODO: this can be removed
-      // check named imports
-      ImportDeclaration(node: TSESTree.ImportDeclaration) {
-        if (!hasTestingLibraryImportModule(node)) {
+      VariableDeclarator(node) {
+        if (!helpers.isRenderUtil(node.init)) {
           return;
         }
-        const renderImport = node.specifiers.find(
-          (node) => isImportSpecifier(node) && node.imported.name === 'render'
-        );
 
-        if (!renderImport) {
-          return;
-        }
-
-        renderAlias = renderImport.local.name;
-      },
-      // TODO: this can be removed
-      // check wildcard imports
-      'ImportDeclaration ImportNamespaceSpecifier'(
-        node: TSESTree.ImportNamespaceSpecifier
-      ) {
-        if (
-          !hasTestingLibraryImportModule(
-            node.parent as TSESTree.ImportDeclaration
-          )
-        ) {
-          return;
-        }
-
-        wildcardImportName = node.local.name;
-      },
-      VariableDeclarator(node: TSESTree.VariableDeclarator) {
         // check if destructuring return value from render
         if (isObjectPattern(node.id)) {
           return;
         }
 
-        // TODO: call `helpers.isRender` with the node.init
-        //  this ini could be Identifier (render) or MemberExpression (rtl.render)
-        const isValidRenderDeclarator = isRenderVariableDeclarator(node, [
-          ...renderFunctions,
-          renderAlias,
-        ]);
-
-        // TODO: After this point, most of the checks should be removed
-        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
         );
@@ -149,7 +56,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
           node,
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: renderResultName,
+            renderResultName,
           },
         });
       },

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

@@ -93,11 +93,7 @@ ruleTester.run(RULE_NAME, rule, {
           const button = screen.getByText('some button');
         });
       `,
-      options: [
-        {
-          renderFunctions: ['customRender'],
-        },
-      ],
+      settings: { 'testing-library/custom-renders': ['customRender'] },
     },
     {
       code: `
@@ -108,11 +104,7 @@ ruleTester.run(RULE_NAME, rule, {
           await view.findByRole('button');
         });
       `,
-      options: [
-        {
-          renderFunctions: ['customRender'],
-        },
-      ],
+      settings: { 'testing-library/custom-renders': ['customRender'] },
     },
     {
       code: `
@@ -123,11 +115,7 @@ ruleTester.run(RULE_NAME, rule, {
           await utils.findByRole('button');
         });
       `,
-      options: [
-        {
-          renderFunctions: ['customRender'],
-        },
-      ],
+      settings: { 'testing-library/custom-renders': ['customRender'] },
     },
     {
       code: `
@@ -203,7 +191,7 @@ ruleTester.run(RULE_NAME, rule, {
         {
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: 'wrapper',
+            renderResultName: 'wrapper',
           },
           line: 5,
           column: 17,
@@ -223,7 +211,7 @@ ruleTester.run(RULE_NAME, rule, {
         {
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: 'wrapper',
+            renderResultName: 'wrapper',
           },
           line: 5,
           column: 17,
@@ -243,7 +231,7 @@ ruleTester.run(RULE_NAME, rule, {
         {
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: 'component',
+            renderResultName: 'component',
           },
           line: 5,
           column: 17,
@@ -280,7 +268,7 @@ ruleTester.run(RULE_NAME, rule, {
         {
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: 'wrapper',
+            renderResultName: 'wrapper',
           },
           line: 5,
           column: 17,
@@ -307,7 +295,7 @@ ruleTester.run(RULE_NAME, rule, {
         {
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: 'wrapper',
+            renderResultName: 'wrapper',
           },
           line: 6,
           column: 17,
@@ -323,16 +311,12 @@ ruleTester.run(RULE_NAME, rule, {
           const button = wrapper.getByText('some button');
         });
       `,
-      options: [
-        {
-          renderFunctions: ['customRender'],
-        },
-      ],
+      settings: { 'testing-library/custom-renders': ['customRender'] },
       errors: [
         {
           messageId: 'renderResultNamingConvention',
           data: {
-            varName: 'wrapper',
+            renderResultName: 'wrapper',
           },
           line: 5,
           column: 17,