Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: migrate prefer-find-by to v4 #270

Merged
merged 2 commits into from
Dec 13, 2020
Merged

refactor: migrate prefer-find-by to v4 #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ed8788
refactor: migrate prefer-find-by to v4
gndelia Dec 11, 2020
28e08b5
refactor: applied pr suggestions
gndelia Dec 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion lib/detect-testing-library-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ import {
isMemberExpression,
isProperty,
} from './node-utils';
import { ABSENCE_MATCHERS, ASYNC_UTILS, PRESENCE_MATCHERS } from './utils';
import {
ABSENCE_MATCHERS,
ASYNC_UTILS,
PRESENCE_MATCHERS,
ALL_QUERIES_COMBINATIONS,
} from './utils';

export type TestingLibrarySettings = {
'testing-library/module'?: string;
Expand Down Expand Up @@ -52,6 +57,7 @@ export type DetectionHelpers = {
isFindByQuery: (node: TSESTree.Identifier) => boolean;
isSyncQuery: (node: TSESTree.Identifier) => boolean;
isAsyncQuery: (node: TSESTree.Identifier) => boolean;
isCustomQuery: (node: TSESTree.Identifier) => boolean;
isAsyncUtil: (node: TSESTree.Identifier) => boolean;
isFireEventMethod: (node: TSESTree.Identifier) => boolean;
isPresenceAssert: (node: TSESTree.MemberExpression) => boolean;
Expand Down Expand Up @@ -180,6 +186,13 @@ export function detectTestingLibraryUtils<
return isFindByQuery(node);
};

const isCustomQuery: DetectionHelpers['isCustomQuery'] = (node) => {
return (
(isSyncQuery(node) || isAsyncQuery(node)) &&
!ALL_QUERIES_COMBINATIONS.includes(node.name)
);
};

/**
* Determines whether a given node is async util or not.
*/
Expand Down Expand Up @@ -356,6 +369,7 @@ export function detectTestingLibraryUtils<
isFindByQuery,
isSyncQuery,
isAsyncQuery,
isCustomQuery,
isAsyncUtil,
isFireEventMethod,
isPresenceAssert,
Expand Down
134 changes: 72 additions & 62 deletions lib/rules/prefer-find-by.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import {
ESLintUtils,
TSESTree,
ASTUtils,
} from '@typescript-eslint/experimental-utils';
import { TSESTree, ASTUtils } from '@typescript-eslint/experimental-utils';
import {
ReportFixFunction,
RuleFix,
Expand All @@ -15,7 +11,7 @@ import {
isObjectPattern,
isProperty,
} from '../node-utils';
import { getDocsUrl, SYNC_QUERIES_COMBINATIONS } from '../utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';

export const RULE_NAME = 'prefer-find-by';
export type MessageIds = 'preferFindBy';
Expand Down Expand Up @@ -51,7 +47,7 @@ function findRenderDefinitionDeclaration(
return findRenderDefinitionDeclaration(scope.upper, query);
}

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
export default createTestingLibraryRule<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'suggestion',
Expand All @@ -70,7 +66,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
},
defaultOptions: [],

create(context) {
create(context, _, helpers) {
const sourceCode = context.getSourceCode();

/**
Expand Down Expand Up @@ -126,7 +122,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
isMemberExpression(argument.body.callee) &&
ASTUtils.isIdentifier(argument.body.callee.property) &&
ASTUtils.isIdentifier(argument.body.callee.object) &&
SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)
helpers.isSyncQuery(argument.body.callee.property)
) {
// shape of () => screen.getByText
const fullQueryMethod = argument.body.callee.property.name;
Expand All @@ -139,6 +135,11 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
queryMethod,
queryVariant,
fix(fixer) {
const property = ((argument.body as TSESTree.CallExpression)
.callee as TSESTree.MemberExpression).property;
if (helpers.isCustomQuery(property as TSESTree.Identifier)) {
return;
}
const newCode = `${caller}.${queryVariant}${queryMethod}(${callArguments
.map((node) => sourceCode.getText(node))
.join(', ')})`;
Expand All @@ -148,65 +149,74 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
return;
}
if (
ASTUtils.isIdentifier(argument.body.callee) &&
SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)
!ASTUtils.isIdentifier(argument.body.callee) ||
!helpers.isSyncQuery(argument.body.callee)
) {
// shape of () => getByText
const fullQueryMethod = argument.body.callee.name;
const queryMethod = fullQueryMethod.split('By')[1];
const queryVariant = getFindByQueryVariant(fullQueryMethod);
const callArguments = argument.body.arguments;
return;
}
// shape of () => getByText
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[] = [];
// this updates waitFor with findBy*
const newCode = `${findByMethod}(${callArguments
.map((node) => sourceCode.getText(node))
.join(', ')})`;
allFixes.push(fixer.replaceText(node, newCode));
reportInvalidUsage(node, {
queryMethod,
queryVariant,
fix(fixer) {
// we know from above callee is an Identifier
if (
helpers.isCustomQuery(
(argument.body as TSESTree.CallExpression)
.callee as TSESTree.Identifier
)
) {
return;
}
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) {
// 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) &&
ASTUtils.isIdentifier(p.key) &&
p.key.name === findByMethod
)
) {
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) &&
ASTUtils.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));
}
// 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;
}
return allFixes;
},
});
},
};
},
Expand Down
18 changes: 9 additions & 9 deletions tests/create-testing-library-rule.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,21 +468,21 @@ ruleTester.run(RULE_NAME, rule, {
// case: custom "getBy*" query reported without import (aggressive reporting)
getByIcon('search')
`,
errors: [{ line: 3, column: 7, messageId: 'getByError' }],
errors: [{ line: 3, column: 7, messageId: 'customQueryError' }],
},
{
code: `
// case: custom "queryBy*" query reported without import (aggressive reporting)
queryByIcon('search')
`,
errors: [{ line: 3, column: 7, messageId: 'queryByError' }],
errors: [{ line: 3, column: 7, messageId: 'customQueryError' }],
},
{
code: `
// case: custom "findBy*" query reported without import (aggressive reporting)
findByIcon('search')
`,
errors: [{ line: 3, column: 7, messageId: 'findByError' }],
errors: [{ line: 3, column: 7, messageId: 'customQueryError' }],
},
{
settings: {
Expand Down Expand Up @@ -564,7 +564,7 @@ ruleTester.run(RULE_NAME, rule, {
import { render } from '@testing-library/react'
getByIcon('search')
`,
errors: [{ line: 4, column: 7, messageId: 'getByError' }],
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
},
{
filename: 'MyComponent.spec.js',
Expand All @@ -576,7 +576,7 @@ ruleTester.run(RULE_NAME, rule, {
import { render } from '@testing-library/framework'
queryByIcon('search')
`,
errors: [{ line: 4, column: 7, messageId: 'queryByError' }],
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
},
{
filename: 'MyComponent.spec.js',
Expand All @@ -588,7 +588,7 @@ ruleTester.run(RULE_NAME, rule, {
import { render } from '@testing-library/framework'
findByIcon('search')
`,
errors: [{ line: 4, column: 7, messageId: 'findByError' }],
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
},
{
settings: {
Expand All @@ -599,7 +599,7 @@ ruleTester.run(RULE_NAME, rule, {
import { render } from 'test-utils'
getByIcon('search')
`,
errors: [{ line: 4, column: 7, messageId: 'getByError' }],
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
},
{
filename: 'MyComponent.spec.js',
Expand All @@ -611,7 +611,7 @@ ruleTester.run(RULE_NAME, rule, {
import { render } from 'test-utils'
queryByIcon('search')
`,
errors: [{ line: 4, column: 7, messageId: 'queryByError' }],
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
},
{
filename: 'MyComponent.spec.js',
Expand All @@ -623,7 +623,7 @@ ruleTester.run(RULE_NAME, rule, {
import { render } from 'test-utils'
findByIcon('search')
`,
errors: [{ line: 4, column: 7, messageId: 'findByError' }],
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
},
{
settings: {
Expand Down
6 changes: 6 additions & 0 deletions tests/fake-rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type MessageIds =
| 'getByError'
| 'queryByError'
| 'findByError'
| 'customQueryError'
| 'presenceAssertError'
| 'absenceAssertError';

Expand All @@ -29,6 +30,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
getByError: 'some error related to getBy reported',
queryByError: 'some error related to queryBy reported',
findByError: 'some error related to findBy reported',
customQueryError: 'some error related to a customQuery reported',
presenceAssertError: 'some error related to presence assert reported',
absenceAssertError: 'some error related to absence assert reported',
},
Expand All @@ -43,6 +45,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
return context.report({ node, messageId: 'fakeError' });
}

if (helpers.isCustomQuery(node)) {
return context.report({ node, messageId: 'customQueryError' });
}

// force queries to be reported
if (helpers.isGetByQuery(node)) {
return context.report({ node, messageId: 'getByError' });
Expand Down
Loading