Skip to content

refactor: second round of tweaks #281

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

Merged
merged 12 commits into from
Mar 8, 2021
Merged

refactor: second round of tweaks #281

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ace7c3f
refactor(shared-settings): rename utils-module
Belco90 Mar 3, 2021
51b5d0b
refactor(detection-helpers): improve fn type definitions
Belco90 Mar 3, 2021
46016d5
test(filename-pattern): simplify settings patterns
Belco90 Mar 4, 2021
330c0a0
fix: check member expression properly within isRenderUtil helper
Belco90 Mar 4, 2021
c3859ed
test: improve create-testing-library-rule test cases
Belco90 Mar 7, 2021
1a7db9d
refactor: check if coming from Testing Library within isAsyncUtil
Belco90 Mar 7, 2021
33267dc
refactor: extract common method for determining if node is TL util
Belco90 Mar 7, 2021
e98cfa0
refactor: improve TL util node detection from identifier
Belco90 Mar 7, 2021
4c5f358
refactor: rename getIdentifierNode to clarify its behavior
Belco90 Mar 7, 2021
1ed9912
fix: improve check for determining if node coming from TL
Belco90 Mar 7, 2021
a378605
test: add async util test cases to fake rule
Belco90 Mar 7, 2021
acaf3df
docs: format jsdoc
Belco90 Mar 8, 2021
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
213 changes: 121 additions & 92 deletions lib/detect-testing-library-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import {
} from '@typescript-eslint/experimental-utils';
import {
getAssertNodeInfo,
getIdentifierNode,
getImportModuleName,
getPropertyIdentifierNode,
getReferenceNode,
ImportModuleNode,
isImportDeclaration,
isImportNamespaceSpecifier,
Expand All @@ -23,7 +24,7 @@ import {
} from './utils';

export type TestingLibrarySettings = {
'testing-library/module'?: string;
'testing-library/utils-module'?: string;
'testing-library/filename-pattern'?: string;
'testing-library/custom-renders'?: string[];
};
Expand All @@ -47,32 +48,54 @@ export type EnhancedRuleCreate<
detectionHelpers: Readonly<DetectionHelpers>
) => TRuleListener;

export type DetectionHelpers = {
getTestingLibraryImportNode: () => ImportModuleNode | null;
getCustomModuleImportNode: () => ImportModuleNode | null;
getTestingLibraryImportName: () => string | undefined;
getCustomModuleImportName: () => string | undefined;
isTestingLibraryImported: () => boolean;
isValidFilename: () => boolean;
isGetQueryVariant: (node: TSESTree.Identifier) => boolean;
isQueryQueryVariant: (node: TSESTree.Identifier) => boolean;
isFindQueryVariant: (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;
isRenderUtil: (node: TSESTree.Node) => boolean;
isPresenceAssert: (node: TSESTree.MemberExpression) => boolean;
isAbsenceAssert: (node: TSESTree.MemberExpression) => boolean;
canReportErrors: () => boolean;
findImportedUtilSpecifier: (
specifierName: string
) => TSESTree.ImportClause | TSESTree.Identifier | undefined;
isNodeComingFromTestingLibrary: (
node: TSESTree.MemberExpression | TSESTree.Identifier
) => boolean;
};
// Helpers methods
type GetTestingLibraryImportNodeFn = () => ImportModuleNode | null;
type GetCustomModuleImportNodeFn = () => ImportModuleNode | null;
type GetTestingLibraryImportNameFn = () => string | undefined;
type GetCustomModuleImportNameFn = () => string | undefined;
type IsTestingLibraryImportedFn = () => boolean;
type IsValidFilenameFn = () => boolean;
type IsGetQueryVariantFn = (node: TSESTree.Identifier) => boolean;
type IsQueryQueryVariantFn = (node: TSESTree.Identifier) => boolean;
type IsFindQueryVariantFn = (node: TSESTree.Identifier) => boolean;
type IsSyncQueryFn = (node: TSESTree.Identifier) => boolean;
type IsAsyncQueryFn = (node: TSESTree.Identifier) => boolean;
type IsCustomQueryFn = (node: TSESTree.Identifier) => boolean;
type IsAsyncUtilFn = (node: TSESTree.Identifier) => boolean;
type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean;
type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean;
type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean;
type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean;
type CanReportErrorsFn = () => boolean;
type FindImportedUtilSpecifierFn = (
specifierName: string
) => TSESTree.ImportClause | TSESTree.Identifier | undefined;
type IsNodeComingFromTestingLibraryFn = (
node: TSESTree.MemberExpression | TSESTree.Identifier
) => boolean;

export interface DetectionHelpers {
getTestingLibraryImportNode: GetTestingLibraryImportNodeFn;
getCustomModuleImportNode: GetCustomModuleImportNodeFn;
getTestingLibraryImportName: GetTestingLibraryImportNameFn;
getCustomModuleImportName: GetCustomModuleImportNameFn;
isTestingLibraryImported: IsTestingLibraryImportedFn;
isValidFilename: IsValidFilenameFn;
isGetQueryVariant: IsGetQueryVariantFn;
isQueryQueryVariant: IsQueryQueryVariantFn;
isFindQueryVariant: IsFindQueryVariantFn;
isSyncQuery: IsSyncQueryFn;
isAsyncQuery: IsAsyncQueryFn;
isCustomQuery: IsCustomQueryFn;
isAsyncUtil: IsAsyncUtilFn;
isFireEventMethod: IsFireEventMethodFn;
isRenderUtil: IsRenderUtilFn;
isPresenceAssert: IsPresenceAssertFn;
isAbsenceAssert: IsAbsenceAssertFn;
canReportErrors: CanReportErrorsFn;
findImportedUtilSpecifier: FindImportedUtilSpecifierFn;
isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn;
}

const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$';

Expand All @@ -95,12 +118,33 @@ export function detectTestingLibraryUtils<
let importedCustomModuleNode: ImportModuleNode | null = null;

// Init options based on shared ESLint settings
const customModule = context.settings['testing-library/module'];
const customModule = context.settings['testing-library/utils-module'];
const filenamePattern =
context.settings['testing-library/filename-pattern'] ??
DEFAULT_FILENAME_PATTERN;
const customRenders = context.settings['testing-library/custom-renders'];

/**
* Small method to extract common checks to determine whether a node is
* related to Testing Library or not.
*/
function isTestingLibraryUtil(
node: TSESTree.Identifier,
isUtilCallback: (identifierNode: TSESTree.Identifier) => boolean
): boolean {
if (!isUtilCallback(node)) {
return false;
}

const referenceNode = getReferenceNode(node);
const referenceNodeIdentifier = getPropertyIdentifierNode(referenceNode);

return (
isAggressiveModuleReportingEnabled() ||
isNodeComingFromTestingLibrary(referenceNodeIdentifier)
);
}

/**
* Determines whether aggressive module reporting is enabled or not.
*
Expand All @@ -126,21 +170,22 @@ export function detectTestingLibraryUtils<
!Array.isArray(customRenders) || customRenders.length === 0;

// Helpers for Testing Library detection.
const getTestingLibraryImportNode: DetectionHelpers['getTestingLibraryImportNode'] = () => {
const getTestingLibraryImportNode: GetTestingLibraryImportNodeFn = () => {
return importedTestingLibraryNode;
};

const getCustomModuleImportNode: DetectionHelpers['getCustomModuleImportNode'] = () => {
const getCustomModuleImportNode: GetCustomModuleImportNodeFn = () => {
return importedCustomModuleNode;
};

const getTestingLibraryImportName: DetectionHelpers['getTestingLibraryImportName'] = () => {
const getTestingLibraryImportName: GetTestingLibraryImportNameFn = () => {
return getImportModuleName(importedTestingLibraryNode);
};

const getCustomModuleImportName: DetectionHelpers['getCustomModuleImportName'] = () => {
const getCustomModuleImportName: GetCustomModuleImportNameFn = () => {
return getImportModuleName(importedCustomModuleNode);
};

/**
* Determines whether Testing Library utils are imported or not for
* current file being analyzed.
Expand All @@ -150,84 +195,92 @@ export function detectTestingLibraryUtils<
* custom modules.
*
* However, there is a setting to customize the module where TL utils can
* be imported from: "testing-library/module". If this setting is enabled,
* be imported from: "testing-library/utils-module". If this setting is enabled,
* then this method will return `true` ONLY IF a testing-library package
* or custom module are imported.
*/
const isTestingLibraryImported: DetectionHelpers['isTestingLibraryImported'] = () => {
if (isAggressiveModuleReportingEnabled()) {
return true;
}

return !!importedTestingLibraryNode || !!importedCustomModuleNode;
const isTestingLibraryImported: IsTestingLibraryImportedFn = () => {
return (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally find the old syntax more readable but it's nit-picking.

isAggressiveModuleReportingEnabled() ||
!!importedTestingLibraryNode ||
!!importedCustomModuleNode
);
};

/**
* Determines whether filename is valid or not for current file
* being analyzed based on "testing-library/filename-pattern" setting.
*/
const isValidFilename: DetectionHelpers['isValidFilename'] = () => {
const isValidFilename: IsValidFilenameFn = () => {
const fileName = context.getFilename();
return !!fileName.match(filenamePattern);
};

/**
* Determines whether a given node is `get*` query variant or not.
*/
const isGetQueryVariant: DetectionHelpers['isGetQueryVariant'] = (node) => {
const isGetQueryVariant: IsGetQueryVariantFn = (node) => {
return /^get(All)?By.+$/.test(node.name);
};

/**
* Determines whether a given node is `query*` query variant or not.
*/
const isQueryQueryVariant: DetectionHelpers['isQueryQueryVariant'] = (
node
) => {
const isQueryQueryVariant: IsQueryQueryVariantFn = (node) => {
return /^query(All)?By.+$/.test(node.name);
};

/**
* Determines whether a given node is `find*` query variant or not.
*/
const isFindQueryVariant: DetectionHelpers['isFindQueryVariant'] = (
node
) => {
const isFindQueryVariant: IsFindQueryVariantFn = (node) => {
return /^find(All)?By.+$/.test(node.name);
};

/**
* Determines whether a given node is sync query or not.
*/
const isSyncQuery: DetectionHelpers['isSyncQuery'] = (node) => {
const isSyncQuery: IsSyncQueryFn = (node) => {
return isGetQueryVariant(node) || isQueryQueryVariant(node);
};

/**
* Determines whether a given node is async query or not.
*/
const isAsyncQuery: DetectionHelpers['isAsyncQuery'] = (node) => {
const isAsyncQuery: IsAsyncQueryFn = (node) => {
return isFindQueryVariant(node);
};

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

/**
* Determines whether a given node is async util or not.
* Determines whether a given node is a valid async util or not.
*
* A node will be interpreted as a valid async util based on two conditions:
* the name matches with some Testing Library async util, and the node is
* coming from Testing Library module.
*
* The latter depends on Aggressive
* module reporting: if enabled, then it doesn't matter from
* where the given node was imported from as it will be considered part of
* Testing Library. Otherwise, it means `custom-module` has been set up, so
* only those nodes coming from Testing Library will be considered as valid.
*/
const isAsyncUtil: DetectionHelpers['isAsyncUtil'] = (node) => {
return ASYNC_UTILS.includes(node.name);
const isAsyncUtil: IsAsyncUtilFn = (node) => {
return isTestingLibraryUtil(node, (identifierNode) =>
ASYNC_UTILS.includes(identifierNode.name)
);
};

/**
* Determines whether a given node is fireEvent method or not
*/
const isFireEventMethod: DetectionHelpers['isFireEventMethod'] = (node) => {
const isFireEventMethod: IsFireEventMethodFn = (node) => {
const fireEventUtil = findImportedUtilSpecifier(FIRE_EVENT_NAME);
let fireEventUtilName: string | undefined;

Expand Down Expand Up @@ -293,29 +346,14 @@ export function detectTestingLibraryUtils<
* Testing Library. Otherwise, it means `custom-module` has been set up, so
* only those nodes coming from Testing Library will be considered as valid.
*/
const isRenderUtil: DetectionHelpers['isRenderUtil'] = (node) => {
const identifier = getIdentifierNode(node);

if (!identifier) {
return false;
}

const isNameMatching = (function () {
const isRenderUtil: IsRenderUtilFn = (node) => {
return isTestingLibraryUtil(node, (identifierNode) => {
if (isAggressiveRenderReportingEnabled()) {
return identifier.name.toLowerCase().includes(RENDER_NAME);
return identifierNode.name.toLowerCase().includes(RENDER_NAME);
}

return [RENDER_NAME, ...customRenders].includes(identifier.name);
})();

if (!isNameMatching) {
return false;
}

return (
isAggressiveModuleReportingEnabled() ||
isNodeComingFromTestingLibrary(identifier)
);
return [RENDER_NAME, ...customRenders].includes(identifierNode.name);
});
};

/**
Expand All @@ -325,7 +363,7 @@ export function detectTestingLibraryUtils<
* - expect(element).toBeInTheDocument()
* - expect(element).not.toBeNull()
*/
const isPresenceAssert: DetectionHelpers['isPresenceAssert'] = (node) => {
const isPresenceAssert: IsPresenceAssertFn = (node) => {
const { matcher, isNegated } = getAssertNodeInfo(node);

if (!matcher) {
Expand All @@ -344,7 +382,7 @@ export function detectTestingLibraryUtils<
* - expect(element).toBeNull()
* - expect(element).not.toBeInTheDocument()
*/
const isAbsenceAssert: DetectionHelpers['isAbsenceAssert'] = (node) => {
const isAbsenceAssert: IsAbsenceAssertFn = (node) => {
const { matcher, isNegated } = getAssertNodeInfo(node);

if (!matcher) {
Expand All @@ -360,7 +398,7 @@ export function detectTestingLibraryUtils<
* Gets a string and verifies if it was imported/required by Testing Library
* related module.
*/
const findImportedUtilSpecifier: DetectionHelpers['findImportedUtilSpecifier'] = (
const findImportedUtilSpecifier: FindImportedUtilSpecifierFn = (
specifierName
) => {
const node = getCustomModuleImportNode() ?? getTestingLibraryImportNode();
Expand Down Expand Up @@ -398,32 +436,23 @@ export function detectTestingLibraryUtils<
/**
* Determines if file inspected meets all conditions to be reported by rules or not.
*/
const canReportErrors: DetectionHelpers['canReportErrors'] = () => {
const canReportErrors: CanReportErrorsFn = () => {
return isTestingLibraryImported() && isValidFilename();
};
/**
* Takes a MemberExpression or an Identifier and verifies if its name comes from the import in TL
* @param node a MemberExpression (in "foo.property" it would be property) or an Identifier
*/
const isNodeComingFromTestingLibrary: DetectionHelpers['isNodeComingFromTestingLibrary'] = (
const isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn = (
node
) => {
let identifierName: string | undefined;

if (ASTUtils.isIdentifier(node)) {
identifierName = node.name;
} else if (ASTUtils.isIdentifier(node.object)) {
identifierName = node.object.name;
}

if (!identifierName) {
return;
}
const identifierName: string | undefined = getPropertyIdentifierNode(node)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change it reports properly those nodes chained more than once.

.name;

return !!findImportedUtilSpecifier(identifierName);
};

const helpers = {
const helpers: DetectionHelpers = {
getTestingLibraryImportNode,
getCustomModuleImportNode,
getTestingLibraryImportName,
Expand Down
Loading