Skip to content

refactor: detection helpers tweaks #254

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 4 commits into from
Nov 10, 2020
Merged

refactor: detection helpers tweaks #254

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a9973f
refactor(extract helpers for detecting presence/absence assets): add …
Belco90 Nov 5, 2020
691dc28
refactor(prefer-presence-queries): use presence/absence helpers
Belco90 Nov 5, 2020
86e827c
refactor: rename boolean detection helpers
Belco90 Nov 9, 2020
b428cc1
refactor: create helpers as separated functions
Belco90 Nov 9, 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
286 changes: 151 additions & 135 deletions lib/detect-testing-library-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ export type DetectionHelpers = {
getCustomModuleImportNode: () => ImportModuleNode | null;
getTestingLibraryImportName: () => string | undefined;
getCustomModuleImportName: () => string | undefined;
getIsTestingLibraryImported: () => boolean;
getIsValidFilename: () => boolean;
isTestingLibraryImported: () => boolean;
isValidFilename: () => boolean;
isGetByQuery: (node: TSESTree.Identifier) => boolean;
isQueryByQuery: (node: TSESTree.Identifier) => boolean;
isSyncQuery: (node: TSESTree.Identifier) => boolean;
Expand Down Expand Up @@ -81,153 +81,169 @@ export function detectTestingLibraryUtils<
DEFAULT_FILENAME_PATTERN;

// Helpers for Testing Library detection.
const helpers: DetectionHelpers = {
getTestingLibraryImportNode() {
return importedTestingLibraryNode;
},
getCustomModuleImportNode() {
return importedCustomModuleNode;
},
getTestingLibraryImportName() {
return getImportModuleName(importedTestingLibraryNode);
},
getCustomModuleImportName() {
return getImportModuleName(importedCustomModuleNode);
},
/**
* Determines whether Testing Library utils are imported or not for
* current file being analyzed.
*
* By default, it is ALWAYS considered as imported. This is what we call
* "aggressive reporting" so we don't miss TL utils reexported from
* 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,
* then this method will return `true` ONLY IF a testing-library package
* or custom module are imported.
*/
getIsTestingLibraryImported() {
if (!customModule) {
return true;
}
const getTestingLibraryImportNode: DetectionHelpers['getTestingLibraryImportNode'] = () => {
Copy link
Member Author

@Belco90 Belco90 Nov 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'm defining DetectionHelpers outside so then I can both pass it to EnhancedRuleCreate and reuse it for individual function definitions like this one. Ideally I'd like to do the opposite: type each function definition, then extract the type of final helper object and pass that to EnhancedRuleCreate but I don't know if there is a way of doing that since the type should be extracted to an outer scope somehow. This works tho.

return importedTestingLibraryNode;
};

return !!importedTestingLibraryNode || !!importedCustomModuleNode;
},
const getCustomModuleImportNode: DetectionHelpers['getCustomModuleImportNode'] = () => {
return importedCustomModuleNode;
};

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

/**
* Determines whether a given node is `getBy*` or `getAllBy*` query variant or not.
*/
isGetByQuery(node) {
return !!node.name.match(/^get(All)?By.+$/);
},
const getCustomModuleImportName: DetectionHelpers['getCustomModuleImportName'] = () => {
return getImportModuleName(importedCustomModuleNode);
};
/**
* Determines whether Testing Library utils are imported or not for
* current file being analyzed.
*
* By default, it is ALWAYS considered as imported. This is what we call
* "aggressive reporting" so we don't miss TL utils reexported from
* 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,
* then this method will return `true` ONLY IF a testing-library package
* or custom module are imported.
*/
const isTestingLibraryImported: DetectionHelpers['isTestingLibraryImported'] = () => {
if (!customModule) {
return true;
}

/**
* Determines whether a given node is `queryBy*` or `queryAllBy*` query variant or not.
*/
isQueryByQuery(node) {
return !!node.name.match(/^query(All)?By.+$/);
},
return !!importedTestingLibraryNode || !!importedCustomModuleNode;
};

/**
* Determines whether a given node is sync query or not.
*/
isSyncQuery(node) {
return this.isGetByQuery(node) || this.isQueryByQuery(node);
},
/**
* Determines whether filename is valid or not for current file
* being analyzed based on "testing-library/filename-pattern" setting.
*/
const isValidFilename: DetectionHelpers['isValidFilename'] = () => {
const fileName = context.getFilename();
return !!fileName.match(filenamePattern);
};

/**
* Determines whether a given MemberExpression node is a presence assert
*
* Presence asserts could have shape of:
* - expect(element).toBeInTheDocument()
* - expect(element).not.toBeNull()
*/
isPresenceAssert(node) {
const { matcher, isNegated } = getAssertNodeInfo(node);
/**
* Determines whether a given node is `getBy*` or `getAllBy*` query variant or not.
*/
const isGetByQuery: DetectionHelpers['isGetByQuery'] = (node) => {
return !!node.name.match(/^get(All)?By.+$/);
};

if (!matcher) {
return false;
}
/**
* Determines whether a given node is `queryBy*` or `queryAllBy*` query variant or not.
*/
const isQueryByQuery: DetectionHelpers['isQueryByQuery'] = (node) => {
return !!node.name.match(/^query(All)?By.+$/);
};

return isNegated
? ABSENCE_MATCHERS.includes(matcher)
: PRESENCE_MATCHERS.includes(matcher);
},
/**
* Determines whether a given node is sync query or not.
*/
const isSyncQuery: DetectionHelpers['isSyncQuery'] = (node) => {
return isGetByQuery(node) || isQueryByQuery(node);
};

/**
* Determines whether a given MemberExpression node is an absence assert
*
* Absence asserts could have shape of:
* - expect(element).toBeNull()
* - expect(element).not.toBeInTheDocument()
*/
isAbsenceAssert(node) {
const { matcher, isNegated } = getAssertNodeInfo(node);
/**
* Determines whether a given MemberExpression node is a presence assert
*
* Presence asserts could have shape of:
* - expect(element).toBeInTheDocument()
* - expect(element).not.toBeNull()
*/
const isPresenceAssert: DetectionHelpers['isPresenceAssert'] = (node) => {
const { matcher, isNegated } = getAssertNodeInfo(node);

if (!matcher) {
return false;
}
if (!matcher) {
return false;
}

return isNegated
? PRESENCE_MATCHERS.includes(matcher)
: ABSENCE_MATCHERS.includes(matcher);
},
return isNegated
? ABSENCE_MATCHERS.includes(matcher)
: PRESENCE_MATCHERS.includes(matcher);
};

/**
* Determines if file inspected meets all conditions to be reported by rules or not.
*/
canReportErrors() {
return (
helpers.getIsTestingLibraryImported() && helpers.getIsValidFilename()
/**
* Determines whether a given MemberExpression node is an absence assert
*
* Absence asserts could have shape of:
* - expect(element).toBeNull()
* - expect(element).not.toBeInTheDocument()
*/
const isAbsenceAssert: DetectionHelpers['isAbsenceAssert'] = (node) => {
const { matcher, isNegated } = getAssertNodeInfo(node);

if (!matcher) {
return false;
}

return isNegated
? PRESENCE_MATCHERS.includes(matcher)
: ABSENCE_MATCHERS.includes(matcher);
};

/**
* Gets a string and verifies if it was imported/required by our custom module node
*/
const findImportedUtilSpecifier: DetectionHelpers['findImportedUtilSpecifier'] = (
specifierName
) => {
const node = getCustomModuleImportNode() ?? getTestingLibraryImportNode();
if (!node) {
return null;
}
if (isImportDeclaration(node)) {
const namedExport = node.specifiers.find(
(n) => isImportSpecifier(n) && n.imported.name === specifierName
);
},
/**
* Gets a string and verifies if it was imported/required by our custom module node
*/
findImportedUtilSpecifier(specifierName: string) {
const node =
helpers.getCustomModuleImportNode() ??
helpers.getTestingLibraryImportNode();
if (!node) {
return null;
// it is "import { foo [as alias] } from 'baz'""
if (namedExport) {
return namedExport;
}
if (isImportDeclaration(node)) {
const namedExport = node.specifiers.find(
(n) => isImportSpecifier(n) && n.imported.name === specifierName
);
// it is "import { foo [as alias] } from 'baz'""
if (namedExport) {
return namedExport;
}
// it could be "import * as rtl from 'baz'"
return node.specifiers.find((n) => isImportNamespaceSpecifier(n));
} else {
const requireNode = node.parent as TSESTree.VariableDeclarator;
if (ASTUtils.isIdentifier(requireNode.id)) {
// this is const rtl = require('foo')
return requireNode.id;
}
// this should be const { something } = require('foo')
const destructuring = requireNode.id as TSESTree.ObjectPattern;
const property = destructuring.properties.find(
(n) =>
isProperty(n) &&
ASTUtils.isIdentifier(n.key) &&
n.key.name === specifierName
);
return (property as TSESTree.Property).key as TSESTree.Identifier;
// it could be "import * as rtl from 'baz'"
return node.specifiers.find((n) => isImportNamespaceSpecifier(n));
} else {
const requireNode = node.parent as TSESTree.VariableDeclarator;
if (ASTUtils.isIdentifier(requireNode.id)) {
// this is const rtl = require('foo')
return requireNode.id;
}
},
// this should be const { something } = require('foo')
const destructuring = requireNode.id as TSESTree.ObjectPattern;
const property = destructuring.properties.find(
(n) =>
isProperty(n) &&
ASTUtils.isIdentifier(n.key) &&
n.key.name === specifierName
);
return (property as TSESTree.Property).key as TSESTree.Identifier;
}
};

/**
* Determines if file inspected meets all conditions to be reported by rules or not.
*/
const canReportErrors: DetectionHelpers['canReportErrors'] = () => {
return isTestingLibraryImported() && isValidFilename();
};

const helpers = {
getTestingLibraryImportNode,
getCustomModuleImportNode,
getTestingLibraryImportName,
getCustomModuleImportName,
isTestingLibraryImported,
isValidFilename,
isGetByQuery,
isQueryByQuery,
isSyncQuery,
isPresenceAssert,
isAbsenceAssert,
canReportErrors,
findImportedUtilSpecifier,
};

// Instructions for Testing Library detection.
Expand Down Expand Up @@ -308,7 +324,7 @@ export function detectTestingLibraryUtils<
detectionInstructions[instruction](node);
}

if (helpers.canReportErrors() && ruleInstructions[instruction]) {
if (canReportErrors() && ruleInstructions[instruction]) {
return ruleInstructions[instruction](node);
}
};
Expand Down