Skip to content

refactor node-utils type guards to use the AST_NODE_TYPES enum #205

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

Merged
merged 3 commits into from
Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
[![Tweet][tweet-badge]][tweet-url]

<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->

[![All Contributors](https://img.shields.io/badge/all_contributors-28-orange.svg?style=flat-square)](#contributors-)

<!-- ALL-CONTRIBUTORS-BADGE:END -->

## Installation
Expand Down Expand Up @@ -216,6 +218,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d

<!-- markdownlint-enable -->
<!-- prettier-ignore-end -->

<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
2 changes: 1 addition & 1 deletion docs/rules/prefer-wait-for.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const foo = async () => {
await waitForElement(() => {});
await waitForDomChange();
await waitForDomChange(mutationObserverOptions);
await waitForDomChange({ timeout: 100});
await waitForDomChange({ timeout: 100 });
};
```

Expand Down
28 changes: 14 additions & 14 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
@@ -1,69 +1,69 @@
import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/experimental-utils';
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/experimental-utils';

export function isCallExpression(
node: TSESTree.Node
): node is TSESTree.CallExpression {
return node && node.type === 'CallExpression';
return node && node.type === AST_NODE_TYPES.CallExpression;
}

export function isAwaitExpression(
node: TSESTree.Node
): node is TSESTree.AwaitExpression {
return node && node.type === 'AwaitExpression';
return node && node.type === AST_NODE_TYPES.AwaitExpression;
}

export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier {
return node && node.type === 'Identifier';
return node && node.type === AST_NODE_TYPES.Identifier;
}

export function isMemberExpression(
node: TSESTree.Node
): node is TSESTree.MemberExpression {
return node && node.type === 'MemberExpression';
return node && node.type === AST_NODE_TYPES.MemberExpression;
}

export function isLiteral(node: TSESTree.Node): node is TSESTree.Literal {
return node && node.type === 'Literal';
return node && node.type === AST_NODE_TYPES.Literal;
}

export function isImportSpecifier(
node: TSESTree.Node
): node is TSESTree.ImportSpecifier {
return node && node.type === 'ImportSpecifier';
return node && node.type === AST_NODE_TYPES.ImportSpecifier;
}

export function isImportDefaultSpecifier(
node: TSESTree.Node
): node is TSESTree.ImportDefaultSpecifier {
return node && node.type === 'ImportDefaultSpecifier';
return node && node.type === AST_NODE_TYPES.ImportDefaultSpecifier;
}

export function isBlockStatement(
node: TSESTree.Node
): node is TSESTree.BlockStatement {
return node && node.type === 'BlockStatement';
return node && node.type === AST_NODE_TYPES.BlockStatement;
}

export function isVariableDeclarator(
node: TSESTree.Node
): node is TSESTree.VariableDeclarator {
return node && node.type === 'VariableDeclarator';
return node && node.type === AST_NODE_TYPES.VariableDeclarator;
}

export function isObjectPattern(
node: TSESTree.Node
): node is TSESTree.ObjectPattern {
return node && node.type === 'ObjectPattern';
return node && node.type === AST_NODE_TYPES.ObjectPattern;
}

export function isProperty(node: TSESTree.Node): node is TSESTree.Property {
return node && node.type === 'Property';
return node && node.type === AST_NODE_TYPES.Property;
}

export function isJSXAttribute(
node: TSESTree.Node
): node is TSESTree.JSXAttribute {
return node && node.type === 'JSXAttribute';
return node && node.type === AST_NODE_TYPES.JSXAttribute;
}

export function findClosestCallExpressionNode(
Expand Down Expand Up @@ -104,7 +104,7 @@ export function hasThenProperty(node: TSESTree.Node) {
}

export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression {
return node && node.type === 'ArrowFunctionExpression'
return node && node.type === AST_NODE_TYPES.ArrowFunctionExpression
}

export function isObjectExpression(node: TSESTree.Expression): node is TSESTree.ObjectExpression {
Expand Down
195 changes: 125 additions & 70 deletions lib/rules/prefer-find-by.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { ReportFixFunction, Scope, RuleFix } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
import {
isIdentifier,
ReportFixFunction,
RuleFix,
Scope,
} from '@typescript-eslint/experimental-utils/dist/ts-eslint';
import {
isArrowFunctionExpression,
isCallExpression,
isIdentifier,
isMemberExpression,
isArrowFunctionExpression,
isObjectPattern,
isProperty,
} from '../node-utils';
Expand All @@ -15,138 +19,189 @@ export const RULE_NAME = 'prefer-find-by';
type Options = [];
export type MessageIds = 'preferFindBy';

export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait']
export const WAIT_METHODS = ['waitFor', 'waitForElement', 'wait'];

export function getFindByQueryVariant(queryMethod: string) {
return queryMethod.includes('All') ? 'findAllBy' : 'findBy';
}

function findRenderDefinitionDeclaration(
scope: Scope.Scope | null,
query: string
): TSESTree.Identifier | null {
if (!scope) {
return null;
}

const variable = scope.variables.find(
(v: Scope.Variable) => v.name === query
);

if (variable) {
const def = variable.defs.find(({ name }) => name.name === query);
return def.name;
}

return findRenderDefinitionDeclaration(scope.upper, query);
}

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'suggestion',
docs: {
description: 'Suggest using find* instead of waitFor to wait for elements',
description:
'Suggest using find* instead of waitFor to wait for elements',
category: 'Best Practices',
recommended: 'warn',
},
messages: {
preferFindBy: 'Prefer {{queryVariant}}{{queryMethod}} method over using await {{fullQuery}}'
preferFindBy:
'Prefer {{queryVariant}}{{queryMethod}} method over using await {{fullQuery}}',
},
fixable: 'code',
schema: []
schema: [],
},
defaultOptions: [],

create(context) {
const sourceCode = context.getSourceCode();



/**
* Reports the invalid usage of wait* plus getBy/QueryBy methods and automatically fixes the scenario
* @param {TSESTree.CallExpression} node - The CallExpresion node that contains the wait* method
* @param {'findBy' | 'findAllBy'} replacementParams.queryVariant - The variant method used to query: findBy/findByAll.
* @param {string} replacementParams.queryMethod - Suffix string to build the query method (the query-part that comes after the "By"): LabelText, Placeholder, Text, Role, Title, etc.
* @param {ReportFixFunction} replacementParams.fix - Function that applies the fix to correct the code
*/
function reportInvalidUsage(node: TSESTree.CallExpression, { queryVariant, queryMethod, fix }: { queryVariant: 'findBy' | 'findAllBy', queryMethod: string, fix: ReportFixFunction }) {

function reportInvalidUsage(
node: TSESTree.CallExpression,
{
queryVariant,
queryMethod,
fix,
}: {
queryVariant: 'findBy' | 'findAllBy';
queryMethod: string;
fix: ReportFixFunction;
}
) {
context.report({
node,
messageId: "preferFindBy",
data: { queryVariant, queryMethod, fullQuery: sourceCode.getText(node) },
messageId: 'preferFindBy',
data: {
queryVariant,
queryMethod,
fullQuery: sourceCode.getText(node),
},
fix,
});
}

return {
'AwaitExpression > CallExpression'(node: TSESTree.CallExpression) {
if (!isIdentifier(node.callee) || !WAIT_METHODS.includes(node.callee.name)) {
return
if (
!isIdentifier(node.callee) ||
!WAIT_METHODS.includes(node.callee.name)
) {
return;
}
// ensure the only argument is an arrow function expression - if the arrow function is a block
// we skip it
const argument = node.arguments[0]
const argument = node.arguments[0];
if (!isArrowFunctionExpression(argument)) {
return
return;
}
if (!isCallExpression(argument.body)) {
return
return;
}
// ensure here it's one of the sync methods that we are calling
if (isMemberExpression(argument.body.callee) && isIdentifier(argument.body.callee.property) && isIdentifier(argument.body.callee.object) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)) {
if (
isMemberExpression(argument.body.callee) &&
isIdentifier(argument.body.callee.property) &&
isIdentifier(argument.body.callee.object) &&
SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)
) {
// shape of () => screen.getByText
const fullQueryMethod = argument.body.callee.property.name
const caller = argument.body.callee.object.name
const queryVariant = getFindByQueryVariant(fullQueryMethod)
const callArguments = argument.body.arguments
const queryMethod = fullQueryMethod.split('By')[1]
const fullQueryMethod = argument.body.callee.property.name;
const caller = argument.body.callee.object.name;
const queryVariant = getFindByQueryVariant(fullQueryMethod);
const callArguments = argument.body.arguments;
const queryMethod = fullQueryMethod.split('By')[1];

reportInvalidUsage(node, {
queryMethod,
queryVariant,
fix(fixer) {
const newCode = `${caller}.${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})`
return fixer.replaceText(node, newCode)
}
})
return
const newCode = `${caller}.${queryVariant}${queryMethod}(${callArguments
.map(node => sourceCode.getText(node))
.join(', ')})`;
return fixer.replaceText(node, newCode);
},
});
return;
}
if (isIdentifier(argument.body.callee) && SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)) {
if (
isIdentifier(argument.body.callee) &&
SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)
) {
// shape of () => getByText
const fullQueryMethod = argument.body.callee.name
const queryMethod = fullQueryMethod.split('By')[1]
const queryVariant = getFindByQueryVariant(fullQueryMethod)
const callArguments = argument.body.arguments
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[] = []
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))
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)
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
return allFixes;
}
// check the declaration is part of a destructuring
if (isObjectPattern(definition.parent.parent)) {
const allVariableDeclarations = definition.parent.parent
const allVariableDeclarations = definition.parent.parent;
// verify if the findBy* method was already declared
if (allVariableDeclarations.properties.some((p) => isProperty(p) && isIdentifier(p.key) && p.key.name === findByMethod)) {
return allFixes
if (
allVariableDeclarations.properties.some(
p =>
isProperty(p) &&
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))
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;
},
});
return;
}
}
}
}
})

export function getFindByQueryVariant(queryMethod: string) {
return queryMethod.includes('All') ? 'findAllBy' : 'findBy'
}

function findRenderDefinitionDeclaration(scope: Scope.Scope | null, query: string): TSESTree.Identifier | null {
if (!scope) {
return null
}
let variable = scope.variables.find((v: Scope.Variable) => v.name === query)
if (variable) {
const def = variable.defs.find(({ name }) => name.name === query)
return def.name
}
return findRenderDefinitionDeclaration(scope.upper, query)
}
},
};
},
});