From 5ee3c4aaf9e65e3c3cc4b85f54f044324879e02c Mon Sep 17 00:00:00 2001 From: Josh Kelly Date: Mon, 3 Aug 2020 15:42:56 +0100 Subject: [PATCH 1/3] refactor(node-utils): use the `AST_NODE_TYPES` enum in type checks Signed-off-by: Josh Kelly --- lib/node-utils.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 64c8c028..d25c97a0 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -1,69 +1,69 @@ -import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/experimental-utils'; export function isCallExpression( node: TSESTree.Node ): node is TSESTree.CallExpression { - return node && node.type === 'CallExpression'; + return node && node.type === AST_NODE_TYPES.CallExpression; } export function isAwaitExpression( node: TSESTree.Node ): node is TSESTree.AwaitExpression { - return node && node.type === 'AwaitExpression'; + return node && node.type === AST_NODE_TYPES.AwaitExpression; } export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { - return node && node.type === 'Identifier'; + return node && node.type === AST_NODE_TYPES.Identifier; } export function isMemberExpression( node: TSESTree.Node ): node is TSESTree.MemberExpression { - return node && node.type === 'MemberExpression'; + return node && node.type === AST_NODE_TYPES.MemberExpression; } export function isLiteral(node: TSESTree.Node): node is TSESTree.Literal { - return node && node.type === 'Literal'; + return node && node.type === AST_NODE_TYPES.Literal; } export function isImportSpecifier( node: TSESTree.Node ): node is TSESTree.ImportSpecifier { - return node && node.type === 'ImportSpecifier'; + return node && node.type === AST_NODE_TYPES.ImportSpecifier; } export function isImportDefaultSpecifier( node: TSESTree.Node ): node is TSESTree.ImportDefaultSpecifier { - return node && node.type === 'ImportDefaultSpecifier'; + return node && node.type === AST_NODE_TYPES.ImportDefaultSpecifier; } export function isBlockStatement( node: TSESTree.Node ): node is TSESTree.BlockStatement { - return node && node.type === 'BlockStatement'; + return node && node.type === AST_NODE_TYPES.BlockStatement; } export function isVariableDeclarator( node: TSESTree.Node ): node is TSESTree.VariableDeclarator { - return node && node.type === 'VariableDeclarator'; + return node && node.type === AST_NODE_TYPES.VariableDeclarator; } export function isObjectPattern( node: TSESTree.Node ): node is TSESTree.ObjectPattern { - return node && node.type === 'ObjectPattern'; + return node && node.type === AST_NODE_TYPES.ObjectPattern; } export function isProperty(node: TSESTree.Node): node is TSESTree.Property { - return node && node.type === 'Property'; + return node && node.type === AST_NODE_TYPES.Property; } export function isJSXAttribute( node: TSESTree.Node ): node is TSESTree.JSXAttribute { - return node && node.type === 'JSXAttribute'; + return node && node.type === AST_NODE_TYPES.JSXAttribute; } export function findClosestCallExpressionNode( @@ -104,7 +104,7 @@ export function hasThenProperty(node: TSESTree.Node) { } export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression { - return node && node.type === 'ArrowFunctionExpression' + return node && node.type === AST_NODE_TYPES.ArrowFunctionExpression } export function isObjectExpression(node: TSESTree.Expression): node is TSESTree.ObjectExpression { From 36ec9f590479db54624379c9920af4fe26648a8a Mon Sep 17 00:00:00 2001 From: Josh Kelly Date: Mon, 3 Aug 2020 15:45:14 +0100 Subject: [PATCH 2/3] chore(formatting): prettier auto fixes Signed-off-by: Josh Kelly --- README.md | 3 +++ docs/rules/prefer-wait-for.md | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 83b05987..2eaf2b1c 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ [![Tweet][tweet-badge]][tweet-url] + [![All Contributors](https://img.shields.io/badge/all_contributors-28-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -216,6 +218,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d + This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! diff --git a/docs/rules/prefer-wait-for.md b/docs/rules/prefer-wait-for.md index a284c32a..ac32c7a5 100644 --- a/docs/rules/prefer-wait-for.md +++ b/docs/rules/prefer-wait-for.md @@ -23,7 +23,7 @@ const foo = async () => { await waitForElement(() => {}); await waitForDomChange(); await waitForDomChange(mutationObserverOptions); - await waitForDomChange({ timeout: 100}); + await waitForDomChange({ timeout: 100 }); }; ``` From 0d1114cc7e29a309227406fcd19a0bd06b3d6255 Mon Sep 17 00:00:00 2001 From: Josh Kelly Date: Mon, 3 Aug 2020 15:46:57 +0100 Subject: [PATCH 3/3] chore(eslint): fix outstanding lint errors Signed-off-by: Josh Kelly --- lib/rules/prefer-find-by.ts | 195 +++++++++++++++++++++++------------- 1 file changed, 125 insertions(+), 70 deletions(-) diff --git a/lib/rules/prefer-find-by.ts b/lib/rules/prefer-find-by.ts index fed0eab2..ee1ede38 100644 --- a/lib/rules/prefer-find-by.ts +++ b/lib/rules/prefer-find-by.ts @@ -1,10 +1,14 @@ import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { ReportFixFunction, Scope, RuleFix } from '@typescript-eslint/experimental-utils/dist/ts-eslint' import { - isIdentifier, + ReportFixFunction, + RuleFix, + Scope, +} from '@typescript-eslint/experimental-utils/dist/ts-eslint'; +import { + isArrowFunctionExpression, isCallExpression, + isIdentifier, isMemberExpression, - isArrowFunctionExpression, isObjectPattern, isProperty, } from '../node-utils'; @@ -15,30 +19,54 @@ export const RULE_NAME = 'prefer-find-by'; type Options = []; export type MessageIds = 'preferFindBy'; -export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait'] +export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait']; + +export function getFindByQueryVariant(queryMethod: string) { + return queryMethod.includes('All') ? 'findAllBy' : 'findBy'; +} + +function findRenderDefinitionDeclaration( + scope: Scope.Scope | null, + query: string +): TSESTree.Identifier | null { + if (!scope) { + return null; + } + + const variable = scope.variables.find( + (v: Scope.Variable) => v.name === query + ); + + if (variable) { + const def = variable.defs.find(({ name }) => name.name === query); + return def.name; + } + + return findRenderDefinitionDeclaration(scope.upper, query); +} export default ESLintUtils.RuleCreator(getDocsUrl)({ name: RULE_NAME, meta: { type: 'suggestion', docs: { - description: 'Suggest using find* instead of waitFor to wait for elements', + description: + 'Suggest using find* instead of waitFor to wait for elements', category: 'Best Practices', recommended: 'warn', }, messages: { - preferFindBy: 'Prefer {{queryVariant}}{{queryMethod}} method over using await {{fullQuery}}' + preferFindBy: + 'Prefer {{queryVariant}}{{queryMethod}} method over using await {{fullQuery}}', }, fixable: 'code', - schema: [] + schema: [], }, defaultOptions: [], create(context) { const sourceCode = context.getSourceCode(); - - /** * Reports the invalid usage of wait* plus getBy/QueryBy methods and automatically fixes the scenario * @param {TSESTree.CallExpression} node - The CallExpresion node that contains the wait* method @@ -46,107 +74,134 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ * @param {string} replacementParams.queryMethod - Suffix string to build the query method (the query-part that comes after the "By"): LabelText, Placeholder, Text, Role, Title, etc. * @param {ReportFixFunction} replacementParams.fix - Function that applies the fix to correct the code */ - function reportInvalidUsage(node: TSESTree.CallExpression, { queryVariant, queryMethod, fix }: { queryVariant: 'findBy' | 'findAllBy', queryMethod: string, fix: ReportFixFunction }) { - + function reportInvalidUsage( + node: TSESTree.CallExpression, + { + queryVariant, + queryMethod, + fix, + }: { + queryVariant: 'findBy' | 'findAllBy'; + queryMethod: string; + fix: ReportFixFunction; + } + ) { context.report({ node, - messageId: "preferFindBy", - data: { queryVariant, queryMethod, fullQuery: sourceCode.getText(node) }, + messageId: 'preferFindBy', + data: { + queryVariant, + queryMethod, + fullQuery: sourceCode.getText(node), + }, fix, }); } return { 'AwaitExpression > CallExpression'(node: TSESTree.CallExpression) { - if (!isIdentifier(node.callee) || !WAIT_METHODS.includes(node.callee.name)) { - return + if ( + !isIdentifier(node.callee) || + !WAIT_METHODS.includes(node.callee.name) + ) { + return; } // ensure the only argument is an arrow function expression - if the arrow function is a block // we skip it - const argument = node.arguments[0] + const argument = node.arguments[0]; if (!isArrowFunctionExpression(argument)) { - return + return; } if (!isCallExpression(argument.body)) { - return + return; } // ensure here it's one of the sync methods that we are calling - if (isMemberExpression(argument.body.callee) && isIdentifier(argument.body.callee.property) && isIdentifier(argument.body.callee.object) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)) { + if ( + isMemberExpression(argument.body.callee) && + isIdentifier(argument.body.callee.property) && + isIdentifier(argument.body.callee.object) && + SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name) + ) { // shape of () => screen.getByText - const fullQueryMethod = argument.body.callee.property.name - const caller = argument.body.callee.object.name - const queryVariant = getFindByQueryVariant(fullQueryMethod) - const callArguments = argument.body.arguments - const queryMethod = fullQueryMethod.split('By')[1] + const fullQueryMethod = argument.body.callee.property.name; + const caller = argument.body.callee.object.name; + const queryVariant = getFindByQueryVariant(fullQueryMethod); + const callArguments = argument.body.arguments; + const queryMethod = fullQueryMethod.split('By')[1]; reportInvalidUsage(node, { queryMethod, queryVariant, fix(fixer) { - const newCode = `${caller}.${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})` - return fixer.replaceText(node, newCode) - } - }) - return + const newCode = `${caller}.${queryVariant}${queryMethod}(${callArguments + .map(node => sourceCode.getText(node)) + .join(', ')})`; + return fixer.replaceText(node, newCode); + }, + }); + return; } - if (isIdentifier(argument.body.callee) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)) { + if ( + isIdentifier(argument.body.callee) && + SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name) + ) { // shape of () => getByText - const fullQueryMethod = argument.body.callee.name - const queryMethod = fullQueryMethod.split('By')[1] - const queryVariant = getFindByQueryVariant(fullQueryMethod) - const callArguments = argument.body.arguments + const fullQueryMethod = argument.body.callee.name; + const queryMethod = fullQueryMethod.split('By')[1]; + const queryVariant = getFindByQueryVariant(fullQueryMethod); + const callArguments = argument.body.arguments; reportInvalidUsage(node, { queryMethod, queryVariant, fix(fixer) { - const findByMethod = `${queryVariant}${queryMethod}` - const allFixes: RuleFix[] = [] + const findByMethod = `${queryVariant}${queryMethod}`; + const allFixes: RuleFix[] = []; // this updates waitFor with findBy* - const newCode = `${findByMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})` - allFixes.push(fixer.replaceText(node, newCode)) + const newCode = `${findByMethod}(${callArguments + .map(node => sourceCode.getText(node)) + .join(', ')})`; + allFixes.push(fixer.replaceText(node, newCode)); // this adds the findBy* declaration - adding it to the list of destructured variables { findBy* } = render() - const definition = findRenderDefinitionDeclaration(context.getScope(), fullQueryMethod) + const definition = findRenderDefinitionDeclaration( + context.getScope(), + fullQueryMethod + ); // I think it should always find it, otherwise code should not be valid (it'd be using undeclared variables) if (!definition) { - return allFixes + return allFixes; } // check the declaration is part of a destructuring if (isObjectPattern(definition.parent.parent)) { - const allVariableDeclarations = definition.parent.parent + const allVariableDeclarations = definition.parent.parent; // verify if the findBy* method was already declared - if (allVariableDeclarations.properties.some((p) => isProperty(p) && isIdentifier(p.key) && p.key.name === findByMethod)) { - return allFixes + if ( + allVariableDeclarations.properties.some( + p => + isProperty(p) && + isIdentifier(p.key) && + p.key.name === findByMethod + ) + ) { + return allFixes; } // the last character of a destructuring is always a "}", so we should replace it with the findBy* declaration - const textDestructuring = sourceCode.getText(allVariableDeclarations) - const text = textDestructuring.substring(0, textDestructuring.length - 2) + `, ${findByMethod} }` - allFixes.push(fixer.replaceText(allVariableDeclarations, text)) + const textDestructuring = sourceCode.getText( + allVariableDeclarations + ); + const text = + textDestructuring.substring(0, textDestructuring.length - 2) + + `, ${findByMethod} }`; + allFixes.push(fixer.replaceText(allVariableDeclarations, text)); } - return allFixes - } - }) - return + return allFixes; + }, + }); + return; } - } - } - } -}) - -export function getFindByQueryVariant(queryMethod: string) { - return queryMethod.includes('All') ? 'findAllBy' : 'findBy' -} - -function findRenderDefinitionDeclaration(scope: Scope.Scope | null, query: string): TSESTree.Identifier | null { - if (!scope) { - return null - } - let variable = scope.variables.find((v: Scope.Variable) => v.name === query) - if (variable) { - const def = variable.defs.find(({ name }) => name.name === query) - return def.name - } - return findRenderDefinitionDeclaration(scope.upper, query) -} \ No newline at end of file + }, + }; + }, +});