feat(prefer-presence-queries): use aggressive query reporting

Author: Belco90

.lintstagedrc

@@ -1,6 +1,6 @@
 {
   "*.{js,ts}": [
-    "eslint --fix",
+    "eslint --max-warnings 0 --fix",
     "prettier --write",
     "jest --findRelatedTests"
   ],

lib/detect-testing-library-utils.ts

@@ -1,12 +1,11 @@
-import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
+import { ASTUtils, TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
 import {
   getImportModuleName,
   isLiteral,
   ImportModuleNode,
   isImportDeclaration,
   isImportNamespaceSpecifier,
   isImportSpecifier,
-  isIdentifier,
   isProperty,
 } from './node-utils';
 
@@ -41,6 +40,9 @@ export type DetectionHelpers = {
   getCustomModuleImportName: () => string | undefined;
   getIsTestingLibraryImported: () => boolean;
   getIsValidFilename: () => boolean;
+  isGetByQuery: (node: TSESTree.Identifier) => boolean;
+  isQueryByQuery: (node: TSESTree.Identifier) => boolean;
+  isSyncQuery: (node: TSESTree.Identifier) => boolean;
   canReportErrors: () => boolean;
   findImportedUtilSpecifier: (
     specifierName: string
@@ -85,7 +87,8 @@ export function detectTestingLibraryUtils<
         return getImportModuleName(importedCustomModuleNode);
       },
       /**
-       * Gets if Testing Library is considered as imported or not.
+       * Determines whether Testing Library utils are imported or not for
+       * current file being analyzed.
        *
        * By default, it is ALWAYS considered as imported. This is what we call
        * "aggressive reporting" so we don't miss TL utils reexported from
@@ -105,17 +108,37 @@ export function detectTestingLibraryUtils<
       },
 
       /**
-       * Gets if filename being analyzed is valid or not.
-       *
-       * This is based on "testing-library/filename-pattern" setting.
+       * Determines whether filename is valid or not for current file
+       * being analyzed based on "testing-library/filename-pattern" setting.
        */
       getIsValidFilename() {
         const fileName = context.getFilename();
         return !!fileName.match(filenamePattern);
       },
 
       /**
-       * Wraps all conditions that must be met to report rules.
+       * Determines whether a given node is `getBy*` or `getAllBy*` query variant or not.
+       */
+      isGetByQuery(node) {
+        return !!node.name.match(/^get(All)?By.+$/);
+      },
+
+      /**
+       * Determines whether a given node is `queryBy*` or `queryAllBy*` query variant or not.
+       */
+      isQueryByQuery(node) {
+        return !!node.name.match(/^query(All)?By.+$/);
+      },
+
+      /**
+       * Determines whether a given node is sync query.
+       */
+      isSyncQuery(node) {
+        return this.isGetByQuery(node) || this.isQueryByQuery(node);
+      },
+
+      /**
+       * Determines if file inspected meets all conditions to be reported by rules or not.
        */
       canReportErrors() {
         return (
@@ -144,7 +167,7 @@ export function detectTestingLibraryUtils<
           return node.specifiers.find((n) => isImportNamespaceSpecifier(n));
         } else {
           const requireNode = node.parent as TSESTree.VariableDeclarator;
-          if (isIdentifier(requireNode.id)) {
+          if (ASTUtils.isIdentifier(requireNode.id)) {
             // this is const rtl = require('foo')
             return requireNode.id;
           }
@@ -153,7 +176,7 @@ export function detectTestingLibraryUtils<
           const property = destructuring.properties.find(
             (n) =>
               isProperty(n) &&
-              isIdentifier(n.key) &&
+              ASTUtils.isIdentifier(n.key) &&
               n.key.name === specifierName
           );
           return (property as TSESTree.Property).key as TSESTree.Identifier;

lib/rules/prefer-presence-queries.ts

@@ -1,28 +1,12 @@
-import { TSESTree } from '@typescript-eslint/experimental-utils';
-import {
-  ABSENCE_MATCHERS,
-  ALL_QUERIES_METHODS,
-  PRESENCE_MATCHERS,
-} from '../utils';
-import {
-  findClosestCallNode,
-  isIdentifier,
-  isMemberExpression,
-} from '../node-utils';
+import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils';
+import { ABSENCE_MATCHERS, PRESENCE_MATCHERS } from '../utils';
+import { findClosestCallNode, isMemberExpression } from '../node-utils';
 import { createTestingLibraryRule } from '../create-testing-library-rule';
 
 export const RULE_NAME = 'prefer-presence-queries';
 export type MessageIds = 'presenceQuery' | 'absenceQuery';
 type Options = [];
 
-const QUERIES_REGEXP = new RegExp(
-  `^(get|query)(All)?(${ALL_QUERIES_METHODS.join('|')})$`
-);
-
-function isThrowingQuery(node: TSESTree.Identifier) {
-  return node.name.startsWith('get');
-}
-
 export default createTestingLibraryRule<Options, MessageIds>({
   name: RULE_NAME,
   meta: {
@@ -44,49 +28,53 @@ export default createTestingLibraryRule<Options, MessageIds>({
   },
   defaultOptions: [],
 
-  create(context) {
+  create(context, _, helpers) {
     return {
-      [`CallExpression Identifier[name=${QUERIES_REGEXP}]`](
-        node: TSESTree.Identifier
-      ) {
+      'CallExpression Identifier'(node: TSESTree.Identifier) {
         const expectCallNode = findClosestCallNode(node, 'expect');
 
-        if (expectCallNode && isMemberExpression(expectCallNode.parent)) {
-          const expectStatement = expectCallNode.parent;
-          const property = expectStatement.property as TSESTree.Identifier;
-          let matcher = property.name;
-          let isNegatedMatcher = false;
+        if (!expectCallNode || !isMemberExpression(expectCallNode.parent)) {
+          return;
+        }
 
-          if (
-            matcher === 'not' &&
-            isMemberExpression(expectStatement.parent) &&
-            isIdentifier(expectStatement.parent.property)
-          ) {
-            isNegatedMatcher = true;
-            matcher = expectStatement.parent.property.name;
-          }
+        if (!helpers.isSyncQuery(node)) {
+          return;
+        }
+
+        const isPresenceQuery = helpers.isGetByQuery(node);
+        const expectStatement = expectCallNode.parent;
+        let matcher =
+          ASTUtils.isIdentifier(expectStatement.property) &&
+          expectStatement.property.name;
+        let isNegatedMatcher = false;
+
+        if (
+          matcher === 'not' &&
+          isMemberExpression(expectStatement.parent) &&
+          ASTUtils.isIdentifier(expectStatement.parent.property)
+        ) {
+          isNegatedMatcher = true;
+          matcher = expectStatement.parent.property.name;
+        }
 
-          const validMatchers = isThrowingQuery(node)
-            ? PRESENCE_MATCHERS
-            : ABSENCE_MATCHERS;
+        const validMatchers = isPresenceQuery
+          ? PRESENCE_MATCHERS
+          : ABSENCE_MATCHERS;
 
-          const invalidMatchers = isThrowingQuery(node)
-            ? ABSENCE_MATCHERS
-            : PRESENCE_MATCHERS;
+        const invalidMatchers = isPresenceQuery
+          ? ABSENCE_MATCHERS
+          : PRESENCE_MATCHERS;
 
-          const messageId = isThrowingQuery(node)
-            ? 'absenceQuery'
-            : 'presenceQuery';
+        const messageId = isPresenceQuery ? 'absenceQuery' : 'presenceQuery';
 
-          if (
-            (!isNegatedMatcher && invalidMatchers.includes(matcher)) ||
-            (isNegatedMatcher && validMatchers.includes(matcher))
-          ) {
-            return context.report({
-              node,
-              messageId,
-            });
-          }
+        if (
+          (!isNegatedMatcher && invalidMatchers.includes(matcher)) ||
+          (isNegatedMatcher && validMatchers.includes(matcher))
+        ) {
+          context.report({
+            node,
+            messageId,
+          });
         }
       },
     };

lib/utils.ts

@@ -24,6 +24,7 @@ const LIBRARY_MODULES = [
   '@testing-library/svelte',
 ];
 
+// TODO: should be deleted after all rules are migrated to v4
 const hasTestingLibraryImportModule = (
   node: TSESTree.ImportDeclaration
 ): boolean => {

package.json

@@ -27,7 +27,7 @@
   "scripts": {
     "build": "tsc",
     "postbuild": "cpy README.md ./dist && cpy package.json ./dist && cpy LICENSE ./dist",
-    "lint": "eslint . --ext .js,.ts",
+    "lint": "eslint . --max-warnings 0 --ext .js,.ts",
     "lint:fix": "npm run lint -- --fix",
     "format": "prettier --write README.md \"{lib,docs,tests}/**/*.{js,ts,md}\"",
     "format:check": "prettier --check README.md \"{lib,docs,tests}/**/*.{js,json,yml,ts,md}\"",

tests/lib/rules/prefer-presence-queries.test.ts

@@ -346,29 +346,25 @@ ruleTester.run(RULE_NAME, rule, {
     {
       code: 'const el = queryByText("button")',
     },
-    {
-      // TODO: this one is gonna be reported by aggressive query reporting
-      code:
-        'expect(getByNonTestingLibraryQuery("button")).not.toBeInTheDocument()',
-    },
-    {
-      // TODO: this one is gonna be reported by aggressive query reporting
-      code:
-        'expect(queryByNonTestingLibraryQuery("button")).toBeInTheDocument()',
-    },
     {
       code: `async () => {
         const el = await findByText('button')
         expect(el).toBeInTheDocument()
       }`,
     },
+    `// case: query an element with getBy but then check its absence after doing
+     // some action which makes it disappear.
+
+     // submit button exists
+     const submitButton = screen.getByRole('button')
+     fireEvent.click(submitButton)
+    
+     // right after clicking submit button it disappears
+     expect(submitButton).not.toBeInTheDocument()
+    `,
     // some weird examples after here to check guard against parent nodes
-    {
-      code: 'expect(getByText("button")).not()',
-    },
-    {
-      code: 'expect(queryByText("button")).not()',
-    },
+    'expect(getByText("button")).not()',
+    'expect(queryByText("button")).not()',
   ],
   invalid: [
     // cases: asserting absence incorrectly with `getBy*` queries
@@ -655,7 +651,20 @@ ruleTester.run(RULE_NAME, rule, {
       code: 'expect(screen.queryAllByText("button")[1]).toBeInTheDocument()',
       errors: [{ messageId: 'presenceQuery', line: 1, column: 15 }],
     },
-    // TODO: add more tests for using custom queries
+    {
+      code: `
+      // case: asserting presence incorrectly with custom queryBy* query
+        expect(queryByCustomQuery("button")).toBeInTheDocument()
+      `,
+      errors: [{ messageId: 'presenceQuery', line: 3, column: 16 }],
+    },
+    {
+      code: `
+        // case: asserting absence incorrectly with custom getBy* query
+        expect(getByCustomQuery("button")).not.toBeInTheDocument()
+      `,
+      errors: [{ messageId: 'absenceQuery', line: 3, column: 16 }],
+    },
     // TODO: add more tests for importing custom module
   ],
 });