Skip to content

fix: guard against null identifier nodes #309

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 8 commits into from
Apr 3, 2021
Merged

fix: guard against null identifier nodes #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c176c2a
fix: guard against null deepest identifier node
Belco90 Apr 2, 2021
83f27d8
fix: guard against null property identifier node
Belco90 Apr 2, 2021
91abe97
ci: decrease coverage threshold
Belco90 Apr 2, 2021
abfe5e8
fix: enable TS strict mode
Belco90 Apr 3, 2021
751889b
fix: remove closing comment leftover
Belco90 Apr 3, 2021
8ceb80f
refactor: declare test cases typings as expected
Belco90 Apr 3, 2021
c20dbb9
fix: remove wrong boolean check for detection
Belco90 Apr 3, 2021
cc040a5
Merge "main" branch into "306_null-identifiers"
Belco90 Apr 3, 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
6 changes: 0 additions & 6 deletions jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ module.exports = {
},
coverageThreshold: {
global: {
branches: 100,
functions: 100,
lines: 100,
statements: 100,
},
'./lib/node-utils.ts': {
branches: 90,
functions: 90,
lines: 90,
Comment on lines 9 to 11
Copy link
Member Author

Choose a reason for hiding this comment

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

I've dropped the coverage threshold a bit since I prefer to guard against null nodes everywhere even if there is no possible code for it. We'll see what I do with this after releasing v4.

Expand Down
52 changes: 39 additions & 13 deletions lib/detect-testing-library-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ export function detectTestingLibraryUtils<

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

/**
* Small method to extract common checks to determine whether a node is
Expand All @@ -160,6 +161,11 @@ export function detectTestingLibraryUtils<

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

if (!referenceNodeIdentifier) {
return false;
}

const importedUtilSpecifier = getImportedUtilSpecifier(
referenceNodeIdentifier
);
Expand Down Expand Up @@ -309,7 +315,8 @@ export function detectTestingLibraryUtils<
(identifierNodeName, originalNodeName) => {
return (
(validNames as string[]).includes(identifierNodeName) ||
(validNames as string[]).includes(originalNodeName)
(!!originalNodeName &&
(validNames as string[]).includes(originalNodeName))
);
}
);
Expand Down Expand Up @@ -370,9 +377,10 @@ export function detectTestingLibraryUtils<
return false;
}

const parentMemberExpression:
| TSESTree.MemberExpression
| undefined = isMemberExpression(node.parent) ? node.parent : undefined;
const parentMemberExpression: TSESTree.MemberExpression | undefined =
node.parent && isMemberExpression(node.parent)
? node.parent
: undefined;

if (!parentMemberExpression) {
return false;
Expand Down Expand Up @@ -417,9 +425,10 @@ export function detectTestingLibraryUtils<
return false;
}

const parentMemberExpression:
| TSESTree.MemberExpression
| undefined = isMemberExpression(node.parent) ? node.parent : undefined;
const parentMemberExpression: TSESTree.MemberExpression | undefined =
node.parent && isMemberExpression(node.parent)
? node.parent
: undefined;

if (!parentMemberExpression) {
return false;
Expand Down Expand Up @@ -483,8 +492,15 @@ export function detectTestingLibraryUtils<
};

const isRenderVariableDeclarator: IsRenderVariableDeclaratorFn = (node) => {
if (!node.init) {
return false;
}
const initIdentifierNode = getDeepestIdentifierNode(node.init);

if (!initIdentifierNode) {
return false;
}

return isRenderUtil(initIdentifierNode);
};

Expand Down Expand Up @@ -547,7 +563,7 @@ export function detectTestingLibraryUtils<
const node = getCustomModuleImportNode() ?? getTestingLibraryImportNode();

if (!node) {
return null;
return undefined;
}

if (isImportDeclaration(node)) {
Expand Down Expand Up @@ -613,7 +629,11 @@ export function detectTestingLibraryUtils<
node: TSESTree.MemberExpression | TSESTree.Identifier
): TSESTree.ImportClause | TSESTree.Identifier | undefined => {
const identifierName: string | undefined = getPropertyIdentifierNode(node)
.name;
?.name;

if (!identifierName) {
return undefined;
}

return findImportedUtilSpecifier(identifierName);
};
Expand Down Expand Up @@ -641,7 +661,11 @@ export function detectTestingLibraryUtils<
}

const identifierName: string | undefined = getPropertyIdentifierNode(node)
.name;
?.name;

if (!identifierName) {
return false;
}

return hasImportMatch(importNode, identifierName);
};
Expand Down Expand Up @@ -696,6 +720,7 @@ export function detectTestingLibraryUtils<
// check only if custom module import not found yet so we avoid
// to override importedCustomModuleNode after it's found
if (
customModule &&
!importedCustomModuleNode &&
String(node.source.value).endsWith(customModule)
) {
Expand Down Expand Up @@ -735,6 +760,7 @@ export function detectTestingLibraryUtils<
!importedCustomModuleNode &&
args.some(
(arg) =>
customModule &&
isLiteral(arg) &&
typeof arg.value === 'string' &&
arg.value.endsWith(customModule)
Expand Down Expand Up @@ -770,11 +796,11 @@ export function detectTestingLibraryUtils<
allKeys.forEach((instruction) => {
enhancedRuleInstructions[instruction] = (node) => {
if (instruction in detectionInstructions) {
detectionInstructions[instruction](node);
detectionInstructions[instruction]?.(node);
}

if (canReportErrors() && ruleInstructions[instruction]) {
return ruleInstructions[instruction](node);
return ruleInstructions[instruction]?.(node);
}
};
});
Expand Down
41 changes: 25 additions & 16 deletions lib/node-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ export function isCallExpression(
}

export function isNewExpression(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.NewExpression {
return node?.type === 'NewExpression';
}

export function isMemberExpression(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.MemberExpression {
return node?.type === AST_NODE_TYPES.MemberExpression;
}
Expand All @@ -60,31 +60,31 @@ export function isLiteral(
}

export function isImportSpecifier(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.ImportSpecifier {
return node?.type === AST_NODE_TYPES.ImportSpecifier;
}

export function isImportNamespaceSpecifier(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.ImportNamespaceSpecifier {
return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier;
}

export function isImportDefaultSpecifier(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.ImportDefaultSpecifier {
return node?.type === AST_NODE_TYPES.ImportDefaultSpecifier;
}

export function isBlockStatement(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.BlockStatement {
return node?.type === AST_NODE_TYPES.BlockStatement;
}

export function isObjectPattern(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.ObjectPattern {
return node?.type === AST_NODE_TYPES.ObjectPattern;
}
Expand All @@ -96,13 +96,13 @@ export function isProperty(
}

export function isJSXAttribute(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.JSXAttribute {
return node?.type === AST_NODE_TYPES.JSXAttribute;
}

export function isExpressionStatement(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.ExpressionStatement {
return node?.type === AST_NODE_TYPES.ExpressionStatement;
}
Expand Down Expand Up @@ -137,7 +137,7 @@ export function findClosestCallExpressionNode(
export function findClosestCallNode(
node: TSESTree.Node,
name: string
): TSESTree.CallExpression {
): TSESTree.CallExpression | null {
if (!node.parent) {
return null;
}
Expand Down Expand Up @@ -195,12 +195,12 @@ export function hasChainedThen(node: TSESTree.Node): boolean {
const parent = node.parent;

// wait(...).then(...)
if (isCallExpression(parent)) {
if (isCallExpression(parent) && parent.parent) {
return hasThenProperty(parent.parent);
}

// promise.then(...)
return hasThenProperty(parent);
return !!parent && hasThenProperty(parent);
}

export function isPromiseIdentifier(
Expand Down Expand Up @@ -239,6 +239,7 @@ export function isPromisesArrayResolved(node: TSESTree.Node): boolean {
}

return (
!!closestCallExpression.parent &&
isArrayExpression(closestCallExpression.parent) &&
isCallExpression(closestCallExpression.parent.parent) &&
(isPromiseAll(closestCallExpression.parent.parent) ||
Expand Down Expand Up @@ -268,6 +269,9 @@ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean {
);

for (const node of suspiciousNodes) {
if (!node || !node.parent) {
continue;
}
if (ASTUtils.isAwaitExpression(node.parent)) {
return true;
}
Expand Down Expand Up @@ -436,7 +440,10 @@ export function getReferenceNode(
| TSESTree.MemberExpression
| TSESTree.Identifier
): TSESTree.CallExpression | TSESTree.MemberExpression | TSESTree.Identifier {
if (isMemberExpression(node.parent) || isCallExpression(node.parent)) {
if (
node.parent &&
(isMemberExpression(node.parent) || isCallExpression(node.parent))
) {
return getReferenceNode(node.parent);
}

Expand Down Expand Up @@ -505,9 +512,10 @@ export function getAssertNodeInfo(
let matcher = ASTUtils.getPropertyName(node);
const isNegated = matcher === 'not';
if (isNegated) {
matcher = isMemberExpression(node.parent)
? ASTUtils.getPropertyName(node.parent)
: null;
matcher =
node.parent && isMemberExpression(node.parent)
? ASTUtils.getPropertyName(node.parent)
: null;
}

if (!matcher) {
Expand All @@ -526,6 +534,7 @@ export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean {
if (
isCallExpression(node) &&
ASTUtils.isIdentifier(node.callee) &&
node.parent &&
isMemberExpression(node.parent) &&
node.callee.name === 'expect'
) {
Expand Down
3 changes: 1 addition & 2 deletions lib/rules/await-async-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export default createTestingLibraryRule<Options, MessageIds>({
asyncQueryWrapper:
'promise returned from {{ name }} wrapper over async query must be handled',
},
fixable: null,
schema: [],
},
defaultOptions: [],
Expand All @@ -52,7 +51,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
true
);

if (!closestCallExpressionNode) {
if (!closestCallExpressionNode || !closestCallExpressionNode.parent) {
return;
}

Expand Down
3 changes: 1 addition & 2 deletions lib/rules/await-async-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export default createTestingLibraryRule<Options, MessageIds>({
asyncUtilWrapper:
'Promise returned from {{ name }} wrapper over async util must be handled',
},
fixable: null,
schema: [],
},
defaultOptions: [],
Expand All @@ -53,7 +52,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
true
);

if (!closestCallExpression) {
if (!closestCallExpression || !closestCallExpression.parent) {
return;
}

Expand Down
3 changes: 1 addition & 2 deletions lib/rules/await-fire-event.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export default createTestingLibraryRule<Options, MessageIds>({
fireEventWrapper:
'Promise returned from `fireEvent.{{ wrapperName }}` wrapper over fire event method must be handled',
},
fixable: null,
schema: [],
},
defaultOptions: [],
Expand Down Expand Up @@ -67,7 +66,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
true
);

if (!closestCallExpression) {
if (!closestCallExpression || !closestCallExpression.parent) {
return;
}

Expand Down
10 changes: 5 additions & 5 deletions lib/rules/consistent-data-testid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
messages: {
consistentDataTestId: '`{{attr}}` "{{value}}" should match `{{regex}}`',
},
fixable: null,
schema: [
{
type: 'object',
Expand Down Expand Up @@ -72,7 +71,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({

function getFileNameData() {
const splitPath = getFilename().split('/');
const fileNameWithExtension = splitPath.pop();
const fileNameWithExtension = splitPath.pop() ?? '';
const parent = splitPath.pop();
const fileName = fileNameWithExtension.split('.').shift();

Expand All @@ -85,17 +84,18 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
return new RegExp(testIdPattern.replace(FILENAME_PLACEHOLDER, fileName));
}

function isTestIdAttribute(name: string) {
function isTestIdAttribute(name: string): boolean {
if (typeof attr === 'string') {
return attr === name;
} else {
return attr.includes(name);
return attr?.includes(name) ?? false;
}
}

return {
JSXIdentifier: (node) => {
if (
!node.parent ||
!isJSXAttribute(node.parent) ||
!isLiteral(node.parent.value) ||
!isTestIdAttribute(node.name)
Expand All @@ -105,7 +105,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({

const value = node.parent.value.value;
const { fileName } = getFileNameData();
const regex = getTestIdValidator(fileName);
const regex = getTestIdValidator(fileName ?? '');

if (value && typeof value === 'string' && !regex.test(value)) {
context.report({
Expand Down
Loading