Skip to content

Commit

Permalink
fix: include findBy variable declaration to prefer-find-by when auto …
Browse files Browse the repository at this point in the history 
…fixing (#197)
  • Loading branch information
@gndelia
gndelia committed Jul 20, 2020
1 parent f9f499b commit 993fd8c
Show file tree
Hide file tree
Showing 2 changed files with 198 additions and 51 deletions.
85 changes: 67 additions & 18 deletions lib/rules/prefer-find-by.ts
View file
@@ -1,9 +1,12 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { ReportFixFunction, Scope, RuleFix } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
import {
isIdentifier,
isCallExpression,
isMemberExpression,
isArrowFunctionExpression,
isObjectPattern,
isProperty,
} from '../node-utils';
import { getDocsUrl, SYNC_QUERIES_COMBINATIONS } from '../utils';

Expand Down Expand Up @@ -34,24 +37,22 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
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
* @param {'findBy' | 'findAllBy'} replacementParams.queryVariant - The variant method used to query: findBy/findByAll.
* @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 {Array<TSESTree.Expression>} replacementParams.callArguments - Array of argument nodes which contain the parameters of the query inside the wait* method.
* @param {string=} replacementParams.caller - the variable name that targets screen or the value returned from `render` function.
* @param {ReportFixFunction} replacementParams.fix - Function that applies the fix to correct the code
*/
function reportInvalidUsage(node: TSESTree.CallExpression, { queryVariant, queryMethod, callArguments, caller }: { queryVariant: 'findBy' | 'findAllBy', queryMethod: string, callArguments: TSESTree.Expression[], caller?: string }) {
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) },
fix(fixer) {
const newCode = `${caller ? `${caller}.` : ''}${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})`
return fixer.replaceText(node, newCode)
}
fix,
});
}

Expand All @@ -72,24 +73,60 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
// 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)) {
// shape of () => screen.getByText
const queryMethod = argument.body.callee.property.name
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: queryMethod.split('By')[1],
queryVariant: getFindByQueryVariant(queryMethod),
callArguments: argument.body.arguments,
caller,
queryMethod,
queryVariant,
fix(fixer) {
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)) {
// shape of () => getByText
const queryMethod = argument.body.callee.name
const fullQueryMethod = argument.body.callee.name
const queryMethod = fullQueryMethod.split('By')[1]
const queryVariant = getFindByQueryVariant(fullQueryMethod)
const callArguments = argument.body.arguments

reportInvalidUsage(node, {
queryMethod: queryMethod.split('By')[1],
queryVariant: getFindByQueryVariant(queryMethod),
callArguments: argument.body.arguments,
queryMethod,
queryVariant,
fix(fixer) {
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))

// this adds the findBy* declaration - adding it to the list of destructured variables { findBy* } = render()
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
}
// check the declaration is part of a destructuring
if (isObjectPattern(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
}
// 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))
}

return allFixes
}
})
return
}
Expand All @@ -98,6 +135,18 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
}
})

function getFindByQueryVariant(queryMethod: string) {
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)
}
164 changes: 131 additions & 33 deletions tests/lib/rules/prefer-find-by.test.ts
View file
@@ -1,18 +1,34 @@
import { InvalidTestCase } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
import { InvalidTestCase, ValidTestCase } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
import { createRuleTester } from '../test-utils';
import { ASYNC_QUERIES_COMBINATIONS, SYNC_QUERIES_COMBINATIONS } from '../../../lib/utils';
import rule, { WAIT_METHODS, RULE_NAME } from '../../../lib/rules/prefer-find-by';
import rule, { WAIT_METHODS, RULE_NAME, getFindByQueryVariant, MessageIds } from '../../../lib/rules/prefer-find-by';

const ruleTester = createRuleTester({
ecmaFeatures: {
jsx: true,
},
});

function buildFindByMethod(queryMethod: string) {
return `${getFindByQueryVariant(queryMethod)}${queryMethod.split('By')[1]}`
}

function createScenario<T extends ValidTestCase<[]> | InvalidTestCase<MessageIds, []>>(callback: (waitMethod: string, queryMethod: string) => T) {
return WAIT_METHODS.reduce((acc: T[], waitMethod) =>
acc.concat(
SYNC_QUERIES_COMBINATIONS
.map((queryMethod) => callback(waitMethod, queryMethod))
)
, [])
}

ruleTester.run(RULE_NAME, rule, {
valid: [
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
code: `const submitButton = await ${queryMethod}('foo')`
code: `
const { ${queryMethod} } = setup()
const submitButton = await ${queryMethod}('foo')
`
})),
...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({
code: `const submitButton = await screen.${queryMethod}('foo')`
Expand Down Expand Up @@ -60,35 +76,117 @@ ruleTester.run(RULE_NAME, rule, {
}
],
invalid: [
// using reduce + concat 'cause flatMap is not available in node10.x
...WAIT_METHODS.reduce((acc: InvalidTestCase<'preferFindBy', []>[], waitMethod) => acc
.concat(
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `const submitButton = await ${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
queryMethod: queryMethod.split('By')[1],
fullQuery: `${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
},
}],
output: `const submitButton = await ${queryMethod.includes('All') ? 'findAllBy': 'findBy'}${queryMethod.split('By')[1]}('foo', { name: 'baz' })`
}))
).concat(
SYNC_QUERIES_COMBINATIONS.map((queryMethod: string) => ({
code: `const submitButton = await ${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: queryMethod.includes('All') ? 'findAllBy': 'findBy',
queryMethod: queryMethod.split('By')[1],
fullQuery: `${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
}
}],
output: `const submitButton = await screen.${queryMethod.includes('All') ? 'findAllBy': 'findBy'}${queryMethod.split('By')[1]}('foo', { name: 'baz' })`
}))
),
[])
...createScenario((waitMethod: string, queryMethod: string) => ({
code: `
const { ${queryMethod} } = render()
const submitButton = await ${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))
`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: getFindByQueryVariant(queryMethod),
queryMethod: queryMethod.split('By')[1],
fullQuery: `${waitMethod}(() => ${queryMethod}('foo', { name: 'baz' }))`,
},
}],
output: `
const { ${queryMethod}, ${buildFindByMethod(queryMethod)} } = render()
const submitButton = await ${buildFindByMethod(queryMethod)}('foo', { name: 'baz' })
`
})),
...createScenario((waitMethod: string, queryMethod: string) => ({
code: `const submitButton = await ${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: getFindByQueryVariant(queryMethod),
queryMethod: queryMethod.split('By')[1],
fullQuery: `${waitMethod}(() => screen.${queryMethod}('foo', { name: 'baz' }))`,
}
}],
output: `const submitButton = await screen.${buildFindByMethod(queryMethod)}('foo', { name: 'baz' })`
})),
// // this scenario verifies it works when the render function is defined in another scope
...WAIT_METHODS.map((waitMethod: string) => ({
code: `
const { getByText, queryByLabelText, findAllByRole } = customRender()
it('foo', async () => {
const submitButton = await ${waitMethod}(() => getByText('baz', { name: 'button' }))
})
`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: 'findBy',
queryMethod: 'Text',
fullQuery: `${waitMethod}(() => getByText('baz', { name: 'button' }))`,
}
}],
output: `
const { getByText, queryByLabelText, findAllByRole, findByText } = customRender()
it('foo', async () => {
const submitButton = await findByText('baz', { name: 'button' })
})
`
})),
// // this scenario verifies when findBy* were already defined (because it was used elsewhere)
...WAIT_METHODS.map((waitMethod: string) => ({
code: `
const { getAllByRole, findAllByRole } = customRender()
describe('some scenario', () => {
it('foo', async () => {
const submitButton = await ${waitMethod}(() => getAllByRole('baz', { name: 'button' }))
})
})
`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: 'findAllBy',
queryMethod: 'Role',
fullQuery: `${waitMethod}(() => getAllByRole('baz', { name: 'button' }))`,
}
}],
output: `
const { getAllByRole, findAllByRole } = customRender()
describe('some scenario', () => {
it('foo', async () => {
const submitButton = await findAllByRole('baz', { name: 'button' })
})
})
`
})),
// invalid code, as we need findBy* to be defined somewhere, but required for getting 100% coverage
{
code: `const submitButton = await waitFor(() => getByText('baz', { name: 'button' }))`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: 'findBy',
queryMethod: 'Text',
fullQuery: `waitFor(() => getByText('baz', { name: 'button' }))`
}
}],
output: `const submitButton = await findByText('baz', { name: 'button' })`
},
// this code would be invalid too, as findByRole is not defined anywhere.
{
code: `
const getByRole = render().getByRole
const submitButton = await waitFor(() => getByRole('baz', { name: 'button' }))
`,
errors: [{
messageId: 'preferFindBy',
data: {
queryVariant: 'findBy',
queryMethod: 'Role',
fullQuery: `waitFor(() => getByRole('baz', { name: 'button' }))`
}
}],
output: `
const getByRole = render().getByRole
const submitButton = await findByRole('baz', { name: 'button' })
`
}
],
})

0 comments on commit 993fd8c

Please sign in to comment.