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.

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

Move rules settings to ESLint shared config: refactor no-container rule #295

Merged
merged 7 commits into from
Mar 20, 2021
6 changes: 0 additions & 6 deletions docs/rules/no-container.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ render(<Example />);
screen.getByRole('button', { name: /click me/i });
```

If you use [custom render functions](https://testing-library.com/docs/example-react-redux) then you can set a config option in your `.eslintrc` to look for these.

```
"testing-library/no-container": ["error", {"renderFunctions":["renderWithRedux", "renderWithRouter"]}],
```

## Further Reading

- [about the `container` element](https://testing-library.com/docs/react-testing-library/api#container-1)
Expand Down
16 changes: 16 additions & 0 deletions lib/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
} from '@typescript-eslint/experimental-utils';
import {
getAssertNodeInfo,
getDeepestIdentifierNode,
getImportModuleName,
getPropertyIdentifierNode,
getReferenceNode,
Expand Down Expand Up @@ -69,6 +70,9 @@ type IsAsyncUtilFn = (
) => boolean;
type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean;
type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean;
type IsRenderVariableDeclaratorFn = (
node: TSESTree.VariableDeclarator
) => boolean;
type IsDebugUtilFn = (node: TSESTree.Identifier) => boolean;
type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean;
type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean;
Expand Down Expand Up @@ -97,6 +101,7 @@ export interface DetectionHelpers {
isAsyncUtil: IsAsyncUtilFn;
isFireEventMethod: IsFireEventMethodFn;
isRenderUtil: IsRenderUtilFn;
isRenderVariableDeclarator: IsRenderVariableDeclaratorFn;
isDebugUtil: IsDebugUtilFn;
isPresenceAssert: IsPresenceAssertFn;
isAbsenceAssert: IsAbsenceAssertFn;
Expand Down Expand Up @@ -149,6 +154,10 @@ export function detectTestingLibraryUtils<
originalNodeName?: string
) => boolean
): boolean {
if (!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.

Safety check. Apparently there was some place calling this method with null node. I need to investigate this properly, but I did this workaround meanwhile.

return false;
}

const referenceNode = getReferenceNode(node);
const referenceNodeIdentifier = getPropertyIdentifierNode(referenceNode);
const importedUtilSpecifier = getImportedUtilSpecifier(
Expand Down Expand Up @@ -408,6 +417,12 @@ export function detectTestingLibraryUtils<
);
};

const isRenderVariableDeclarator: IsRenderVariableDeclaratorFn = (node) => {
const initIdentifierNode = getDeepestIdentifierNode(node.init);

return isRenderUtil(initIdentifierNode);
};

const isDebugUtil: IsDebugUtilFn = (node) => {
return isTestingLibraryUtil(
node,
Expand Down Expand Up @@ -562,6 +577,7 @@ export function detectTestingLibraryUtils<
isAsyncUtil,
isFireEventMethod,
isRenderUtil,
isRenderVariableDeclarator,
isDebugUtil,
isPresenceAssert,
isAbsenceAssert,
Expand Down
25 changes: 0 additions & 25 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,31 +477,6 @@ export function isRenderFunction(
});
}

// TODO: should be removed after v4 is finished
export function isRenderVariableDeclarator(
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed finally. A better equivalent has been created in detect-testing-library-utils.ts

node: TSESTree.VariableDeclarator,
renderFunctions: string[]
): boolean {
if (node.init) {
if (ASTUtils.isAwaitExpression(node.init)) {
return (
node.init.argument &&
isRenderFunction(
node.init.argument as TSESTree.CallExpression,
renderFunctions
)
);
} else {
return (
isCallExpression(node.init) &&
isRenderFunction(node.init, renderFunctions)
);
}
}

return false;
}

// TODO: extract into types file?
export type ImportModuleNode =
| TSESTree.ImportDeclaration
Expand Down
134 changes: 77 additions & 57 deletions lib/rules/no-container.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import {
ESLintUtils,
TSESTree,
ASTUtils,
} from '@typescript-eslint/experimental-utils';
import { getDocsUrl } from '../utils';
import {
getDeepestIdentifierNode,
getFunctionName,
getInnermostReturningFunction,
isMemberExpression,
isObjectPattern,
isProperty,
isRenderVariableDeclarator,
} from '../node-utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';

export const RULE_NAME = 'no-container';
export type MessageIds = 'noContainer';
type Options = [{ renderFunctions?: string[] }];
type Options = [];

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
export default createTestingLibraryRule<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'problem',
Expand All @@ -29,48 +27,52 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
'Avoid using container methods. Prefer using the methods from Testing Library, such as "getByRole()"',
},
fixable: null,
schema: [
{
type: 'object',
properties: {
renderFunctions: {
type: 'array',
},
},
},
],
schema: [],
},
defaultOptions: [
{
renderFunctions: [],
},
],
defaultOptions: [],

create(context, [options]) {
const { renderFunctions } = options;
create(context, [], helpers) {
const destructuredContainerPropNames: string[] = [];
let renderWrapperName: string = null;
const renderWrapperNames: string[] = [];
let renderResultVarName: string = null;
let containerName: string = null;
let containerCallsMethod = false;

function detectRenderWrapper(node: TSESTree.Identifier): void {
const innerFunction = getInnermostReturningFunction(context, node);

if (innerFunction) {
renderWrapperNames.push(getFunctionName(innerFunction));
}
}

function showErrorIfChainedContainerMethod(
innerNode: TSESTree.MemberExpression
) {
if (isMemberExpression(innerNode)) {
if (ASTUtils.isIdentifier(innerNode.object)) {
const isContainerName = innerNode.object.name === containerName;
const isRenderWrapper = innerNode.object.name === renderWrapperName;

if (isContainerName) {
context.report({
node: innerNode,
messageId: 'noContainer',
});
return;
}

const isRenderWrapper = innerNode.object.name === renderResultVarName;
containerCallsMethod =
ASTUtils.isIdentifier(innerNode.property) &&
innerNode.property.name === 'container' &&
isRenderWrapper;

if (isContainerName || containerCallsMethod) {
if (containerCallsMethod) {
context.report({
node: innerNode,
node: innerNode.property,
Copy link
Member Author

Choose a reason for hiding this comment

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

If container is chained, report container node rather than chained one.

messageId: 'noContainer',
});
return;
}
}
showErrorIfChainedContainerMethod(
Expand All @@ -80,35 +82,12 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
}

return {
VariableDeclarator(node) {
if (isRenderVariableDeclarator(node, ['render', ...renderFunctions])) {
if (isObjectPattern(node.id)) {
const containerIndex = node.id.properties.findIndex(
(property) =>
isProperty(property) &&
ASTUtils.isIdentifier(property.key) &&
property.key.name === 'container'
);
const nodeValue =
containerIndex !== -1 && node.id.properties[containerIndex].value;
if (ASTUtils.isIdentifier(nodeValue)) {
containerName = nodeValue.name;
} else {
isObjectPattern(nodeValue) &&
nodeValue.properties.forEach(
(property) =>
isProperty(property) &&
ASTUtils.isIdentifier(property.key) &&
destructuredContainerPropNames.push(property.key.name)
);
}
} else {
renderWrapperName = ASTUtils.isIdentifier(node.id) && node.id.name;
}
CallExpression(node) {
const callExpressionIdentifier = getDeepestIdentifierNode(node);
if (helpers.isRenderUtil(callExpressionIdentifier)) {
detectRenderWrapper(callExpressionIdentifier);
}
},

CallExpression(node) {
if (isMemberExpression(node.callee)) {
showErrorIfChainedContainerMethod(node.callee);
} else {
Expand All @@ -120,6 +99,47 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
});
}
},

VariableDeclarator(node) {
const initIdentifierNode = getDeepestIdentifierNode(node.init);

const isRenderWrapperVariableDeclarator = initIdentifierNode
? renderWrapperNames.includes(initIdentifierNode.name)
: false;

if (
!helpers.isRenderVariableDeclarator(node) &&
!isRenderWrapperVariableDeclarator
) {
return;
}
Comment on lines +106 to +115
Copy link
Member Author

Choose a reason for hiding this comment

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

The rule implementation is mostly the same, except this bit to detect reportable render or some wrapper around it.


if (isObjectPattern(node.id)) {
const containerIndex = node.id.properties.findIndex(
(property) =>
isProperty(property) &&
ASTUtils.isIdentifier(property.key) &&
property.key.name === 'container'
);

const nodeValue =
containerIndex !== -1 && node.id.properties[containerIndex].value;

if (ASTUtils.isIdentifier(nodeValue)) {
containerName = nodeValue.name;
} else {
isObjectPattern(nodeValue) &&
nodeValue.properties.forEach(
(property) =>
isProperty(property) &&
ASTUtils.isIdentifier(property.key) &&
destructuredContainerPropNames.push(property.key.name)
);
}
} else {
renderResultVarName = ASTUtils.isIdentifier(node.id) && node.id.name;
}
},
};
},
});
24 changes: 23 additions & 1 deletion lib/rules/no-debug.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {
getDeepestIdentifierNode,
getFunctionName,
getInnermostReturningFunction,
getPropertyIdentifierNode,
getReferenceNode,
isObjectPattern,
Expand Down Expand Up @@ -41,12 +43,28 @@ export default createTestingLibraryRule<Options, MessageIds>({
create(context, [], helpers) {
const suspiciousDebugVariableNames: string[] = [];
const suspiciousReferenceNodes: TSESTree.Identifier[] = [];
const renderWrapperNames: string[] = [];

function detectRenderWrapper(node: TSESTree.Identifier): void {
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 forgot to detect this properly in the previous PR, so I added it here.

const innerFunction = getInnermostReturningFunction(context, node);

if (innerFunction) {
renderWrapperNames.push(getFunctionName(innerFunction));
}
}

return {
VariableDeclarator(node) {
const initIdentifierNode = getDeepestIdentifierNode(node.init);

if (!helpers.isRenderUtil(initIdentifierNode)) {
const isRenderWrapperVariableDeclarator = initIdentifierNode
? renderWrapperNames.includes(initIdentifierNode.name)
: false;

if (
!helpers.isRenderVariableDeclarator(node) &&
!isRenderWrapperVariableDeclarator
) {
return;
}

Expand Down Expand Up @@ -74,6 +92,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
},
CallExpression(node) {
const callExpressionIdentifier = getDeepestIdentifierNode(node);
if (helpers.isRenderUtil(callExpressionIdentifier)) {
detectRenderWrapper(callExpressionIdentifier);
}

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

Expand Down
2 changes: 1 addition & 1 deletion lib/rules/render-result-naming-convention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
}

if (
!helpers.isRenderUtil(initIdentifierNode) &&
!helpers.isRenderVariableDeclarator(node) &&
!renderWrapperNames.includes(initIdentifierNode.name)
) {
return;
Expand Down
Loading