feat: add no-global-regex-flag-in-query rule (#560)

* feat: add no-global-regex-flag-in-query * refactor: review feedback * feat: add fixer * test: add error details * test: add within cases * refactor: use getDeepestIdentifierNode * refactor: review feedback Closes #559
testing-library · Mar 31, 2022 · 6e645e6 · 6e645e6
1 parent 7bc2b9c
commit 6e645e6
Show file tree

Hide file tree

Showing 5 changed files with 327 additions and 1 deletion.
diff --git a/README.md b/README.md
@@ -198,6 +198,7 @@ To enable this configuration use the `extends` property in your
 | [`testing-library/no-container`](./docs/rules/no-container.md)                                       | Disallow the use of `container` methods                                                      |     | ![angular-badge][] ![react-badge][] ![vue-badge][]                |
 | [`testing-library/no-debugging-utils`](./docs/rules/no-debugging-utils.md)                           | Disallow the use of debugging utilities like `debug`                                         |     | ![angular-badge][] ![react-badge][] ![vue-badge][]                |
 | [`testing-library/no-dom-import`](./docs/rules/no-dom-import.md)                                     | Disallow importing from DOM Testing Library                                                  | 🔧  | ![angular-badge][] ![react-badge][] ![vue-badge][]                |
+| [`testing-library/no-global-regexp-flag-in-query`](./docs/rules/no-global-regexp-flag-in-query.md)   | Disallow the use of the global RegExp flag (/g) in queries                                   | 🔧  |                                                                   |
 | [`testing-library/no-manual-cleanup`](./docs/rules/no-manual-cleanup.md)                             | Disallow the use of `cleanup`                                                                |     |                                                                   |
 | [`testing-library/no-node-access`](./docs/rules/no-node-access.md)                                   | Disallow direct Node access                                                                  |     | ![angular-badge][] ![react-badge][] ![vue-badge][]                |
 | [`testing-library/no-promise-in-fire-event`](./docs/rules/no-promise-in-fire-event.md)               | Disallow the use of promises passed to a `fireEvent` method                                  |     | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] |

diff --git a/docs/rules/no-global-regexp-flag-in-query.md b/docs/rules/no-global-regexp-flag-in-query.md
@@ -0,0 +1,31 @@
+# Disallow the use of the global RegExp flag (/g) in queries (`testing-library/no-global-regexp-flag-in-query`)
+
+Ensure that there are no global RegExp flags used when using queries.
+
+## Rule Details
+
+A RegExp instance that's using the global flag `/g` holds state and this might cause false-positives while querying for elements.
+
+Examples of **incorrect** code for this rule:
+
+```js
+screen.getByText(/hello/gi);
+```
+
+```js
+await screen.findByRole('button', { otherProp: true, name: /hello/g });
+```
+
+Examples of **correct** code for this rule:
+
+```js
+screen.getByText(/hello/i);
+```
+
+```js
+await screen.findByRole('button', { otherProp: true, name: /hello/ });
+```
+
+## Further Reading
+
+- [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/lastIndex)
diff --git a/lib/rules/no-global-regexp-flag-in-query.ts b/lib/rules/no-global-regexp-flag-in-query.ts
@@ -0,0 +1,105 @@
+import { ASTUtils, TSESTree } from '@typescript-eslint/utils';
+
+import { createTestingLibraryRule } from '../create-testing-library-rule';
+import {
+  isMemberExpression,
+  isCallExpression,
+  isProperty,
+  isObjectExpression,
+  getDeepestIdentifierNode,
+  isLiteral,
+} from '../node-utils';
+
+export const RULE_NAME = 'no-global-regexp-flag-in-query';
+export type MessageIds = 'noGlobalRegExpFlagInQuery';
+type Options = [];
+
+export default createTestingLibraryRule<Options, MessageIds>({
+  name: RULE_NAME,
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'Disallow the use of the global RegExp flag (/g) in queries',
+      recommendedConfig: {
+        dom: false,
+        angular: false,
+        react: false,
+        vue: false,
+      },
+    },
+    messages: {
+      noGlobalRegExpFlagInQuery:
+        'Avoid using the global RegExp flag (/g) in queries',
+    },
+    fixable: 'code',
+    schema: [],
+  },
+  defaultOptions: [],
+  create(context, _, helpers) {
+    function report(literalNode: TSESTree.Node) {
+      if (
+        isLiteral(literalNode) &&
+        'regex' in literalNode &&
+        literalNode.regex.flags.includes('g')
+      ) {
+        context.report({
+          node: literalNode,
+          messageId: 'noGlobalRegExpFlagInQuery',
+          fix(fixer) {
+            const splitter = literalNode.raw.lastIndexOf('/');
+            const raw = literalNode.raw.substring(0, splitter);
+            const flags = literalNode.raw.substring(splitter + 1);
+            const flagsWithoutGlobal = flags.replace('g', '');
+
+            return fixer.replaceText(
+              literalNode,
+              `${raw}/${flagsWithoutGlobal}`
+            );
+          },
+        });
+        return true;
+      }
+      return false;
+    }
+
+    function getArguments(identifierNode: TSESTree.Identifier) {
+      if (isCallExpression(identifierNode.parent)) {
+        return identifierNode.parent.arguments;
+      } else if (
+        isMemberExpression(identifierNode.parent) &&
+        isCallExpression(identifierNode.parent.parent)
+      ) {
+        return identifierNode.parent.parent.arguments;
+      }
+
+      return [];
+    }
+
+    return {
+      CallExpression(node) {
+        const identifierNode = getDeepestIdentifierNode(node);
+        if (!identifierNode || !helpers.isQuery(identifierNode)) {
+          return;
+        }
+
+        const [firstArg, secondArg] = getArguments(identifierNode);
+
+        const firstArgumentHasError = report(firstArg);
+        if (firstArgumentHasError) {
+          return;
+        }
+
+        if (isObjectExpression(secondArg)) {
+          const namePropertyNode = secondArg.properties.find(
+            (p) =>
+              isProperty(p) &&
+              ASTUtils.isIdentifier(p.key) &&
+              p.key.name === 'name' &&
+              isLiteral(p.value)
+          ) as TSESTree.ObjectLiteralElement & { value: TSESTree.Literal };
+          report(namePropertyNode.value);
+        }
+      },
+    };
+  },
+});
diff --git a/tests/index.test.ts b/tests/index.test.ts
@@ -8,7 +8,7 @@ import plugin from '../lib';
 const execAsync = util.promisify(exec);
 const generateConfigs = () => execAsync(`npm run generate:configs`);
 
-const numberOfRules = 26;
+const numberOfRules = 27;
 const ruleNames = Object.keys(plugin.rules);
 
 // eslint-disable-next-line jest/expect-expect

diff --git a/tests/lib/rules/no-global-regexp-flag-in-query.test.ts b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts
@@ -0,0 +1,189 @@
+import rule, {
+  RULE_NAME,
+} from '../../../lib/rules/no-global-regexp-flag-in-query';
+import { createRuleTester } from '../test-utils';
+
+const ruleTester = createRuleTester();
+
+ruleTester.run(RULE_NAME, rule, {
+  valid: [
+    `
+        import { screen } from '@testing-library/dom'
+        screen.getByText(/hello/)
+    `,
+    `
+        import { screen } from '@testing-library/dom'
+        screen.getByText(/hello/i)
+    `,
+    `
+        import { screen } from '@testing-library/dom'
+        screen.getByText('hello')
+    `,
+
+    `
+        import { screen } from '@testing-library/dom'
+        screen.findByRole('button', {name: /hello/})
+    `,
+    `
+        import { screen } from '@testing-library/dom'
+        screen.findByRole('button', {name: /hello/im})
+    `,
+    `
+        import { screen } from '@testing-library/dom'
+        screen.findByRole('button', {name: 'hello'})
+    `,
+    `
+        const utils = render(<Component/>)
+        utils.findByRole('button', {name: /hello/m})
+    `,
+    `
+      const {queryAllByPlaceholderText} = render(<Component/>)
+      queryAllByPlaceholderText(/hello/i)
+    `,
+    `
+        import { within } from '@testing-library/dom'
+        within(element).findByRole('button', {name: /hello/i})
+    `,
+    `
+        import { within } from '@testing-library/dom'
+        within(element).queryByText('Hello')
+    `,
+    `
+        const text = 'hello';
+        /hello/g.test(text)
+        text.match(/hello/g)
+    `,
+    `
+        const text = somethingElse()
+        /hello/g.test(text)
+        text.match(/hello/g)
+    `,
+    `
+        import somethingElse from 'somethingElse'
+        somethingElse.lookup(/hello/g)
+    `,
+    `
+        import { screen } from '@testing-library/dom'
+        screen.notAQuery(/hello/g)
+    `,
+    `
+        import { screen } from '@testing-library/dom'
+        screen.notAQuery('button', {name: /hello/g})
+    `,
+    `
+        const utils = render(<Component/>)
+        utils.notAQuery('button', {name: /hello/i})
+    `,
+    `
+        const utils = render(<Component/>)
+        utils.notAQuery(/hello/i)
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+        import { screen } from '@testing-library/dom'
+        screen.getByText(/hello/g)`,
+      errors: [
+        {
+          messageId: 'noGlobalRegExpFlagInQuery',
+          line: 3,
+          column: 26,
+        },
+      ],
+      output: `
+        import { screen } from '@testing-library/dom'
+        screen.getByText(/hello/)`,
+    },
+    {
+      code: `
+        import { screen } from '@testing-library/dom'
+        screen.findByRole('button', {name: /hellogg/g})`,
+      errors: [
+        {
+          messageId: 'noGlobalRegExpFlagInQuery',
+          line: 3,
+          column: 44,
+        },
+      ],
+      output: `
+        import { screen } from '@testing-library/dom'
+        screen.findByRole('button', {name: /hellogg/})`,
+    },
+    {
+      code: `
+        import { screen } from '@testing-library/dom'
+        screen.findByRole('button', {otherProp: true, name: /hello/g})`,
+      errors: [
+        {
+          messageId: 'noGlobalRegExpFlagInQuery',
+          line: 3,
+          column: 61,
+        },
+      ],
+      output: `
+        import { screen } from '@testing-library/dom'
+        screen.findByRole('button', {otherProp: true, name: /hello/})`,
+    },
+    {
+      code: `
+        const utils = render(<Component/>)
+        utils.findByRole('button', {name: /hello/ig})`,
+      errors: [
+        {
+          messageId: 'noGlobalRegExpFlagInQuery',
+          line: 3,
+          column: 43,
+        },
+      ],
+      output: `
+        const utils = render(<Component/>)
+        utils.findByRole('button', {name: /hello/i})`,
+    },
+    {
+      code: `
+        const {queryAllByLabelText} = render(<Component/>)
+        queryAllByLabelText(/hello/gi)`,
+      errors: [
+        {
+          messageId: 'noGlobalRegExpFlagInQuery',
+          line: 3,
+          column: 29,
+        },
+      ],
+      output: `
+        const {queryAllByLabelText} = render(<Component/>)
+        queryAllByLabelText(/hello/i)`,
+    },
+    {
+      code: `
+        import { within } from '@testing-library/dom'
+        within(element).findByRole('button', {name: /hello/igm})`,
+      errors: [
+        {
+          messageId: 'noGlobalRegExpFlagInQuery',
+          line: 3,
+          column: 53,
+        },
+      ],
+      output: `
+        import { within } from '@testing-library/dom'
+        within(element).findByRole('button', {name: /hello/im})`,
+    },
+    {
+      code: `
+        import { within } from '@testing-library/dom'
+        within(element).queryAllByText(/hello/ig)`,
+      errors: [
+        {
+          messageId: 'noGlobalRegExpFlagInQuery',
+          line: 3,
+          column: 40,
+        },
+      ],
+      output: `
+        import { within } from '@testing-library/dom'
+        within(element).queryAllByText(/hello/i)`,
+    },
+  ],
+});