From 89caf8cfec7ff8b0b55f070b125ec1a9582f63a6 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Tue, 29 Mar 2022 16:56:37 +0200 Subject: [PATCH 1/7] feat: add no-global-regex-flag-in-query --- README.md | 1 + docs/rules/no-global-regexp-flag-in-query.md | 31 ++++++ lib/configs/angular.ts | 1 + lib/configs/dom.ts | 1 + lib/configs/react.ts | 1 + lib/configs/vue.ts | 1 + lib/node-utils/is-node-of-type.ts | 1 + lib/rules/no-global-regexp-flag-in-query.ts | 83 ++++++++++++++++ tests/__snapshots__/index.test.ts.snap | 4 + tests/index.test.ts | 2 +- .../no-global-regexp-flag-in-query.test.ts | 96 +++++++++++++++++++ 11 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-global-regexp-flag-in-query.md create mode 100644 lib/rules/no-global-regexp-flag-in-query.ts create mode 100644 tests/lib/rules/no-global-regexp-flag-in-query.test.ts diff --git a/README.md b/README.md index 2a9aff48..d84f49e2 100644 --- 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 | | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | [`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 new file mode 100644 index 00000000..61ebed20 --- /dev/null +++ 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/configs/angular.ts b/lib/configs/angular.ts index af9b277c..4c92b62d 100644 --- a/lib/configs/angular.ts +++ b/lib/configs/angular.ts @@ -11,6 +11,7 @@ export = { 'testing-library/no-container': 'error', 'testing-library/no-debugging-utils': 'error', 'testing-library/no-dom-import': ['error', 'angular'], + 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-node-access': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-render-in-setup': 'error', diff --git a/lib/configs/dom.ts b/lib/configs/dom.ts index ce153733..1f6d8ed4 100644 --- a/lib/configs/dom.ts +++ b/lib/configs/dom.ts @@ -8,6 +8,7 @@ export = { 'testing-library/await-async-query': 'error', 'testing-library/await-async-utils': 'error', 'testing-library/no-await-sync-query': 'error', + 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-wait-for-empty-callback': 'error', 'testing-library/no-wait-for-multiple-assertions': 'error', diff --git a/lib/configs/react.ts b/lib/configs/react.ts index 5e2bc966..c7787bc1 100644 --- a/lib/configs/react.ts +++ b/lib/configs/react.ts @@ -11,6 +11,7 @@ export = { 'testing-library/no-container': 'error', 'testing-library/no-debugging-utils': 'error', 'testing-library/no-dom-import': ['error', 'react'], + 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-node-access': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-render-in-setup': 'error', diff --git a/lib/configs/vue.ts b/lib/configs/vue.ts index cf9e42bb..c5d1b764 100644 --- a/lib/configs/vue.ts +++ b/lib/configs/vue.ts @@ -12,6 +12,7 @@ export = { 'testing-library/no-container': 'error', 'testing-library/no-debugging-utils': 'error', 'testing-library/no-dom-import': ['error', 'vue'], + 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-node-access': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-render-in-setup': 'error', diff --git a/lib/node-utils/is-node-of-type.ts b/lib/node-utils/is-node-of-type.ts index a7f7597c..a75004a4 100644 --- a/lib/node-utils/is-node-of-type.ts +++ b/lib/node-utils/is-node-of-type.ts @@ -59,3 +59,4 @@ export const isReturnStatement = ASTUtils.isNodeOfType( export const isFunctionExpression = ASTUtils.isNodeOfType( AST_NODE_TYPES.FunctionExpression ); +export const isIdentifier = ASTUtils.isNodeOfType(AST_NODE_TYPES.Identifier); diff --git a/lib/rules/no-global-regexp-flag-in-query.ts b/lib/rules/no-global-regexp-flag-in-query.ts new file mode 100644 index 00000000..d31b5a51 --- /dev/null +++ b/lib/rules/no-global-regexp-flag-in-query.ts @@ -0,0 +1,83 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +import { createTestingLibraryRule } from '../create-testing-library-rule'; +import { + isMemberExpression, + isIdentifier, + isCallExpression, + isProperty, + isObjectExpression, +} from '../node-utils'; + +export const RULE_NAME = 'no-global-regexp-flag-in-query'; +export type MessageIds = 'noGlobalRegExpFlagInQuery'; +type Options = []; + +export default createTestingLibraryRule({ + name: RULE_NAME, + meta: { + type: 'suggestion', + docs: { + description: 'Disallow the use of the global RegExp flag (/g) in queries', + recommendedConfig: { + dom: 'error', + angular: 'error', + react: 'error', + vue: 'error', + }, + }, + messages: { + noGlobalRegExpFlagInQuery: + 'Avoid using the global RegExp flag (/g) in queries', + }, + schema: [], + }, + defaultOptions: [], + create(context, _, helpers) { + function lint( + regexpNode: TSESTree.Literal, + identifier: TSESTree.Identifier + ) { + if (helpers.isQuery(identifier)) { + context.report({ + node: regexpNode, + messageId: 'noGlobalRegExpFlagInQuery', + }); + } + } + + return { + [`CallExpression[callee.type=MemberExpression] > Literal[regex.flags=/g/].arguments`]( + node: TSESTree.Literal + ) { + if ( + isCallExpression(node.parent) && + isMemberExpression(node.parent.callee) && + isIdentifier(node.parent.callee.property) + ) { + lint(node, node.parent.callee.property); + } + }, + [`CallExpression[callee.type=Identifier] > Literal[regex.flags=/g/].arguments`]( + node: TSESTree.Literal + ) { + if (isCallExpression(node.parent) && isIdentifier(node.parent.callee)) { + lint(node, node.parent.callee); + } + }, + [`ObjectExpression:has(Property>[name="name"]) Literal[regex.flags=/g/]`]( + node: TSESTree.Literal + ) { + if ( + isProperty(node.parent) && + isObjectExpression(node.parent.parent) && + isCallExpression(node.parent.parent.parent) && + isMemberExpression(node.parent.parent.parent.callee) && + isIdentifier(node.parent.parent.parent.callee.property) + ) { + lint(node, node.parent.parent.parent.callee.property); + } + }, + }; + }, +}); diff --git a/tests/__snapshots__/index.test.ts.snap b/tests/__snapshots__/index.test.ts.snap index d800dee4..b2035f7a 100644 --- a/tests/__snapshots__/index.test.ts.snap +++ b/tests/__snapshots__/index.test.ts.snap @@ -16,6 +16,7 @@ Object { "error", "angular", ], + "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-node-access": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-render-in-setup": "error", @@ -38,6 +39,7 @@ Object { "testing-library/await-async-query": "error", "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", + "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-wait-for-empty-callback": "error", "testing-library/no-wait-for-multiple-assertions": "error", @@ -63,6 +65,7 @@ Object { "error", "react", ], + "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-node-access": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-render-in-setup": "error", @@ -93,6 +96,7 @@ Object { "error", "vue", ], + "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-node-access": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-render-in-setup": "error", diff --git a/tests/index.test.ts b/tests/index.test.ts index e317945a..8bb777fc 100644 --- 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 new file mode 100644 index 00000000..0ed019d6 --- /dev/null +++ b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts @@ -0,0 +1,96 @@ +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/i}) + `, + ` + import { screen } from '@testing-library/dom' + screen.findByRole('button', {name: 'hello'}) + `, + ` + const utils = render() + utils.findByRole('button', {name: /hello/i}) + `, + ` + const {queryAllByPlaceholderText} = render() + queryAllByPlaceholderText(/hello/i) + `, + ], + invalid: [ + { + code: ` + import { screen } from '@testing-library/dom' + screen.getByText(/hello/g)`, + errors: [ + { + messageId: 'noGlobalRegExpFlagInQuery', + }, + ], + }, + { + code: ` + import { screen } from '@testing-library/dom' + screen.findByRole('button', {name: /hello/g})`, + errors: [ + { + messageId: 'noGlobalRegExpFlagInQuery', + }, + ], + }, + { + code: ` + import { screen } from '@testing-library/dom' + screen.findByRole('button', {otherProp: true, name: /hello/g})`, + errors: [ + { + messageId: 'noGlobalRegExpFlagInQuery', + }, + ], + }, + { + code: ` + const utils = render() + utils.findByRole('button', {name: /hello/ig})`, + errors: [ + { + messageId: 'noGlobalRegExpFlagInQuery', + }, + ], + }, + { + code: ` + const {queryAllByLabelText} = render() + queryAllByLabelText(/hello/ig)`, + errors: [ + { + messageId: 'noGlobalRegExpFlagInQuery', + }, + ], + }, + ], +}); From c1953a9e600a1058ac173721e42d79b83d351303 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 30 Mar 2022 07:43:21 +0200 Subject: [PATCH 2/7] refactor: review feedback --- lib/configs/angular.ts | 1 - lib/configs/dom.ts | 1 - lib/configs/react.ts | 1 - lib/configs/vue.ts | 1 - lib/node-utils/is-node-of-type.ts | 1 - lib/rules/no-global-regexp-flag-in-query.ts | 20 +++++---- tests/__snapshots__/index.test.ts.snap | 4 -- .../no-global-regexp-flag-in-query.test.ts | 42 ++++++++++++++++++- 8 files changed, 51 insertions(+), 20 deletions(-) diff --git a/lib/configs/angular.ts b/lib/configs/angular.ts index 4c92b62d..af9b277c 100644 --- a/lib/configs/angular.ts +++ b/lib/configs/angular.ts @@ -11,7 +11,6 @@ export = { 'testing-library/no-container': 'error', 'testing-library/no-debugging-utils': 'error', 'testing-library/no-dom-import': ['error', 'angular'], - 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-node-access': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-render-in-setup': 'error', diff --git a/lib/configs/dom.ts b/lib/configs/dom.ts index 1f6d8ed4..ce153733 100644 --- a/lib/configs/dom.ts +++ b/lib/configs/dom.ts @@ -8,7 +8,6 @@ export = { 'testing-library/await-async-query': 'error', 'testing-library/await-async-utils': 'error', 'testing-library/no-await-sync-query': 'error', - 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-wait-for-empty-callback': 'error', 'testing-library/no-wait-for-multiple-assertions': 'error', diff --git a/lib/configs/react.ts b/lib/configs/react.ts index c7787bc1..5e2bc966 100644 --- a/lib/configs/react.ts +++ b/lib/configs/react.ts @@ -11,7 +11,6 @@ export = { 'testing-library/no-container': 'error', 'testing-library/no-debugging-utils': 'error', 'testing-library/no-dom-import': ['error', 'react'], - 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-node-access': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-render-in-setup': 'error', diff --git a/lib/configs/vue.ts b/lib/configs/vue.ts index c5d1b764..cf9e42bb 100644 --- a/lib/configs/vue.ts +++ b/lib/configs/vue.ts @@ -12,7 +12,6 @@ export = { 'testing-library/no-container': 'error', 'testing-library/no-debugging-utils': 'error', 'testing-library/no-dom-import': ['error', 'vue'], - 'testing-library/no-global-regexp-flag-in-query': 'error', 'testing-library/no-node-access': 'error', 'testing-library/no-promise-in-fire-event': 'error', 'testing-library/no-render-in-setup': 'error', diff --git a/lib/node-utils/is-node-of-type.ts b/lib/node-utils/is-node-of-type.ts index a75004a4..a7f7597c 100644 --- a/lib/node-utils/is-node-of-type.ts +++ b/lib/node-utils/is-node-of-type.ts @@ -59,4 +59,3 @@ export const isReturnStatement = ASTUtils.isNodeOfType( export const isFunctionExpression = ASTUtils.isNodeOfType( AST_NODE_TYPES.FunctionExpression ); -export const isIdentifier = ASTUtils.isNodeOfType(AST_NODE_TYPES.Identifier); diff --git a/lib/rules/no-global-regexp-flag-in-query.ts b/lib/rules/no-global-regexp-flag-in-query.ts index d31b5a51..657213e9 100644 --- a/lib/rules/no-global-regexp-flag-in-query.ts +++ b/lib/rules/no-global-regexp-flag-in-query.ts @@ -1,9 +1,8 @@ -import { TSESTree } from '@typescript-eslint/utils'; +import { ASTUtils, TSESTree } from '@typescript-eslint/utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; import { isMemberExpression, - isIdentifier, isCallExpression, isProperty, isObjectExpression, @@ -20,10 +19,10 @@ export default createTestingLibraryRule({ docs: { description: 'Disallow the use of the global RegExp flag (/g) in queries', recommendedConfig: { - dom: 'error', - angular: 'error', - react: 'error', - vue: 'error', + dom: false, + angular: false, + react: false, + vue: false, }, }, messages: { @@ -53,7 +52,7 @@ export default createTestingLibraryRule({ if ( isCallExpression(node.parent) && isMemberExpression(node.parent.callee) && - isIdentifier(node.parent.callee.property) + ASTUtils.isIdentifier(node.parent.callee.property) ) { lint(node, node.parent.callee.property); } @@ -61,7 +60,10 @@ export default createTestingLibraryRule({ [`CallExpression[callee.type=Identifier] > Literal[regex.flags=/g/].arguments`]( node: TSESTree.Literal ) { - if (isCallExpression(node.parent) && isIdentifier(node.parent.callee)) { + if ( + isCallExpression(node.parent) && + ASTUtils.isIdentifier(node.parent.callee) + ) { lint(node, node.parent.callee); } }, @@ -73,7 +75,7 @@ export default createTestingLibraryRule({ isObjectExpression(node.parent.parent) && isCallExpression(node.parent.parent.parent) && isMemberExpression(node.parent.parent.parent.callee) && - isIdentifier(node.parent.parent.parent.callee.property) + ASTUtils.isIdentifier(node.parent.parent.parent.callee.property) ) { lint(node, node.parent.parent.parent.callee.property); } diff --git a/tests/__snapshots__/index.test.ts.snap b/tests/__snapshots__/index.test.ts.snap index b2035f7a..d800dee4 100644 --- a/tests/__snapshots__/index.test.ts.snap +++ b/tests/__snapshots__/index.test.ts.snap @@ -16,7 +16,6 @@ Object { "error", "angular", ], - "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-node-access": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-render-in-setup": "error", @@ -39,7 +38,6 @@ Object { "testing-library/await-async-query": "error", "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", - "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-wait-for-empty-callback": "error", "testing-library/no-wait-for-multiple-assertions": "error", @@ -65,7 +63,6 @@ Object { "error", "react", ], - "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-node-access": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-render-in-setup": "error", @@ -96,7 +93,6 @@ Object { "error", "vue", ], - "testing-library/no-global-regexp-flag-in-query": "error", "testing-library/no-node-access": "error", "testing-library/no-promise-in-fire-event": "error", "testing-library/no-render-in-setup": "error", 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 index 0ed019d6..84f65e67 100644 --- a/tests/lib/rules/no-global-regexp-flag-in-query.test.ts +++ b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts @@ -37,8 +37,38 @@ ruleTester.run(RULE_NAME, rule, { utils.findByRole('button', {name: /hello/i}) `, ` - const {queryAllByPlaceholderText} = render() - queryAllByPlaceholderText(/hello/i) + const {queryAllByPlaceholderText} = render() + queryAllByPlaceholderText(/hello/i) + `, + ` + 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() + utils.notAQuery('button', {name: /hello/i}) + `, + ` + const utils = render() + utils.notAQuery(/hello/i) `, ], invalid: [ @@ -59,6 +89,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'noGlobalRegExpFlagInQuery', + line: 3, + column: 46, }, ], }, @@ -69,6 +101,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'noGlobalRegExpFlagInQuery', + line: 3, + column: 65, }, ], }, @@ -79,6 +113,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'noGlobalRegExpFlagInQuery', + line: 3, + column: 47, }, ], }, @@ -89,6 +125,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'noGlobalRegExpFlagInQuery', + line: 3, + column: 33, }, ], }, From 274cccf07ef5ee99b9a0876cd03d9fa170c1ad71 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 30 Mar 2022 08:00:09 +0200 Subject: [PATCH 3/7] feat: add fixer --- lib/rules/no-global-regexp-flag-in-query.ts | 12 ++++++ .../no-global-regexp-flag-in-query.test.ts | 39 +++++++++++++------ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/lib/rules/no-global-regexp-flag-in-query.ts b/lib/rules/no-global-regexp-flag-in-query.ts index 657213e9..ec6b6ce5 100644 --- a/lib/rules/no-global-regexp-flag-in-query.ts +++ b/lib/rules/no-global-regexp-flag-in-query.ts @@ -29,6 +29,7 @@ export default createTestingLibraryRule({ noGlobalRegExpFlagInQuery: 'Avoid using the global RegExp flag (/g) in queries', }, + fixable: 'code', schema: [], }, defaultOptions: [], @@ -41,6 +42,17 @@ export default createTestingLibraryRule({ context.report({ node: regexpNode, messageId: 'noGlobalRegExpFlagInQuery', + fix(fixer) { + const splitter = regexpNode.raw.lastIndexOf('/'); + const raw = regexpNode.raw.substring(0, splitter); + const flags = regexpNode.raw.substring(splitter + 1); + const flagsWithoutGlobal = flags.replace('g', ''); + + return fixer.replaceText( + regexpNode, + `${raw}/${flagsWithoutGlobal}` + ); + }, }); } } 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 index 84f65e67..739afedc 100644 --- a/tests/lib/rules/no-global-regexp-flag-in-query.test.ts +++ b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts @@ -81,54 +81,69 @@ ruleTester.run(RULE_NAME, rule, { messageId: 'noGlobalRegExpFlagInQuery', }, ], + output: ` + import { screen } from '@testing-library/dom' + screen.getByText(/hello/)`, }, { code: ` - import { screen } from '@testing-library/dom' - screen.findByRole('button', {name: /hello/g})`, + import { screen } from '@testing-library/dom' + screen.findByRole('button', {name: /hellogg/g})`, errors: [ { messageId: 'noGlobalRegExpFlagInQuery', line: 3, - column: 46, + 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})`, + import { screen } from '@testing-library/dom' + screen.findByRole('button', {otherProp: true, name: /hello/g})`, errors: [ { messageId: 'noGlobalRegExpFlagInQuery', line: 3, - column: 65, + column: 61, }, ], + output: ` + import { screen } from '@testing-library/dom' + screen.findByRole('button', {otherProp: true, name: /hello/})`, }, { code: ` - const utils = render() - utils.findByRole('button', {name: /hello/ig})`, + const utils = render() + utils.findByRole('button', {name: /hello/ig})`, errors: [ { messageId: 'noGlobalRegExpFlagInQuery', line: 3, - column: 47, + column: 43, }, ], + output: ` + const utils = render() + utils.findByRole('button', {name: /hello/i})`, }, { code: ` - const {queryAllByLabelText} = render() - queryAllByLabelText(/hello/ig)`, + const {queryAllByLabelText} = render() + queryAllByLabelText(/hello/gi)`, errors: [ { messageId: 'noGlobalRegExpFlagInQuery', line: 3, - column: 33, + column: 29, }, ], + output: ` + const {queryAllByLabelText} = render() + queryAllByLabelText(/hello/i)`, }, ], }); From 6d606b2cbdd4ed9869da222276e71b56d3bf1cc4 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 30 Mar 2022 10:29:14 +0200 Subject: [PATCH 4/7] test: add error details --- README.md | 2 +- tests/lib/rules/no-global-regexp-flag-in-query.test.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d84f49e2..d9ea34cc 100644 --- a/README.md +++ b/README.md @@ -198,7 +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 | | ![dom-badge][] ![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/tests/lib/rules/no-global-regexp-flag-in-query.test.ts b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts index 739afedc..5edefa8c 100644 --- a/tests/lib/rules/no-global-regexp-flag-in-query.test.ts +++ b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts @@ -79,6 +79,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'noGlobalRegExpFlagInQuery', + line: 3, + column: 26, }, ], output: ` From 25e5038d4b76510d797c923ef76201d4b549d144 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 30 Mar 2022 16:47:47 +0200 Subject: [PATCH 5/7] test: add within cases --- .../no-global-regexp-flag-in-query.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) 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 index 5edefa8c..d33e4db3 100644 --- a/tests/lib/rules/no-global-regexp-flag-in-query.test.ts +++ b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts @@ -40,6 +40,14 @@ ruleTester.run(RULE_NAME, rule, { const {queryAllByPlaceholderText} = render() 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) @@ -147,5 +155,35 @@ ruleTester.run(RULE_NAME, rule, { const {queryAllByLabelText} = render() queryAllByLabelText(/hello/i)`, }, + { + code: ` + import { within } from '@testing-library/dom' + within(element).findByRole('button', {name: /hello/ig})`, + errors: [ + { + messageId: 'noGlobalRegExpFlagInQuery', + line: 3, + column: 53, + }, + ], + output: ` + import { within } from '@testing-library/dom' + within(element).findByRole('button', {name: /hello/i})`, + }, + { + 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)`, + }, ], }); From d662a2f53befbdd0e433584f421c90f71bc392a4 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 30 Mar 2022 17:08:42 +0200 Subject: [PATCH 6/7] refactor: use getDeepestIdentifierNode --- lib/rules/no-global-regexp-flag-in-query.ts | 79 ++++++++++----------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/lib/rules/no-global-regexp-flag-in-query.ts b/lib/rules/no-global-regexp-flag-in-query.ts index ec6b6ce5..eb9b176a 100644 --- a/lib/rules/no-global-regexp-flag-in-query.ts +++ b/lib/rules/no-global-regexp-flag-in-query.ts @@ -6,6 +6,8 @@ import { isCallExpression, isProperty, isObjectExpression, + getDeepestIdentifierNode, + isLiteral, } from '../node-utils'; export const RULE_NAME = 'no-global-regexp-flag-in-query'; @@ -34,62 +36,57 @@ export default createTestingLibraryRule({ }, defaultOptions: [], create(context, _, helpers) { - function lint( - regexpNode: TSESTree.Literal, - identifier: TSESTree.Identifier - ) { - if (helpers.isQuery(identifier)) { + function report(literalNode: TSESTree.Node) { + if ( + isLiteral(literalNode) && + 'regex' in literalNode && + literalNode.regex.flags.includes('g') + ) { context.report({ - node: regexpNode, + node: literalNode, messageId: 'noGlobalRegExpFlagInQuery', fix(fixer) { - const splitter = regexpNode.raw.lastIndexOf('/'); - const raw = regexpNode.raw.substring(0, splitter); - const flags = regexpNode.raw.substring(splitter + 1); + 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( - regexpNode, + literalNode, `${raw}/${flagsWithoutGlobal}` ); }, }); + return true; } + return false; } return { - [`CallExpression[callee.type=MemberExpression] > Literal[regex.flags=/g/].arguments`]( - node: TSESTree.Literal - ) { - if ( - isCallExpression(node.parent) && - isMemberExpression(node.parent.callee) && - ASTUtils.isIdentifier(node.parent.callee.property) - ) { - lint(node, node.parent.callee.property); - } - }, - [`CallExpression[callee.type=Identifier] > Literal[regex.flags=/g/].arguments`]( - node: TSESTree.Literal - ) { - if ( - isCallExpression(node.parent) && - ASTUtils.isIdentifier(node.parent.callee) - ) { - lint(node, node.parent.callee); + CallExpression(node) { + const identifierNode = getDeepestIdentifierNode(node); + if (!identifierNode || !helpers.isQuery(identifierNode)) { + return; } - }, - [`ObjectExpression:has(Property>[name="name"]) Literal[regex.flags=/g/]`]( - node: TSESTree.Literal - ) { - if ( - isProperty(node.parent) && - isObjectExpression(node.parent.parent) && - isCallExpression(node.parent.parent.parent) && - isMemberExpression(node.parent.parent.parent.callee) && - ASTUtils.isIdentifier(node.parent.parent.parent.callee.property) - ) { - lint(node, node.parent.parent.parent.callee.property); + + const [firstArg, secondArg] = isCallExpression(identifierNode.parent) + ? identifierNode.parent.arguments + : isMemberExpression(identifierNode.parent) && + isCallExpression(identifierNode.parent.parent) + ? identifierNode.parent.parent.arguments + : []; + + if (!report(firstArg)) { + 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); + } } }, }; From 1a7bf1235f97dab78aafd36cc8e0dffa2e01a925 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 30 Mar 2022 18:02:20 +0200 Subject: [PATCH 7/7] refactor: review feedback --- lib/rules/no-global-regexp-flag-in-query.ts | 45 ++++++++++++------- .../no-global-regexp-flag-in-query.test.ts | 8 ++-- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/lib/rules/no-global-regexp-flag-in-query.ts b/lib/rules/no-global-regexp-flag-in-query.ts index eb9b176a..6f8683f8 100644 --- a/lib/rules/no-global-regexp-flag-in-query.ts +++ b/lib/rules/no-global-regexp-flag-in-query.ts @@ -62,6 +62,19 @@ export default createTestingLibraryRule({ 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); @@ -69,24 +82,22 @@ export default createTestingLibraryRule({ return; } - const [firstArg, secondArg] = isCallExpression(identifierNode.parent) - ? identifierNode.parent.arguments - : isMemberExpression(identifierNode.parent) && - isCallExpression(identifierNode.parent.parent) - ? identifierNode.parent.parent.arguments - : []; + const [firstArg, secondArg] = getArguments(identifierNode); + + const firstArgumentHasError = report(firstArg); + if (firstArgumentHasError) { + return; + } - if (!report(firstArg)) { - 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); - } + 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/lib/rules/no-global-regexp-flag-in-query.test.ts b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts index d33e4db3..df26b365 100644 --- a/tests/lib/rules/no-global-regexp-flag-in-query.test.ts +++ b/tests/lib/rules/no-global-regexp-flag-in-query.test.ts @@ -26,7 +26,7 @@ ruleTester.run(RULE_NAME, rule, { `, ` import { screen } from '@testing-library/dom' - screen.findByRole('button', {name: /hello/i}) + screen.findByRole('button', {name: /hello/im}) `, ` import { screen } from '@testing-library/dom' @@ -34,7 +34,7 @@ ruleTester.run(RULE_NAME, rule, { `, ` const utils = render() - utils.findByRole('button', {name: /hello/i}) + utils.findByRole('button', {name: /hello/m}) `, ` const {queryAllByPlaceholderText} = render() @@ -158,7 +158,7 @@ ruleTester.run(RULE_NAME, rule, { { code: ` import { within } from '@testing-library/dom' - within(element).findByRole('button', {name: /hello/ig})`, + within(element).findByRole('button', {name: /hello/igm})`, errors: [ { messageId: 'noGlobalRegExpFlagInQuery', @@ -168,7 +168,7 @@ ruleTester.run(RULE_NAME, rule, { ], output: ` import { within } from '@testing-library/dom' - within(element).findByRole('button', {name: /hello/i})`, + within(element).findByRole('button', {name: /hello/im})`, }, { code: `