Skip to content

ts-migration/improve-guards #316

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 3 commits into from
Jul 20, 2019
Merged

ts-migration/improve-guards #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbd9bac
chore: improve type guards
G-Rath Jul 20, 2019
65a7014
chore(ts-utils): remove check for nested `MemberExpression` in `isHook`
G-Rath Jul 20, 2019
797fe1f
chore: move function for smaller diff
SimenB Jul 20, 2019
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: 3 additions & 3 deletions src/rules/lowercase-name.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
} from '@typescript-eslint/experimental-utils';
import {
DescribeAlias,
JestFunctionCallExpression,
JestFunctionCallExpressionWithIdentifierCallee,
JestFunctionName,
TestCaseName,
createRule,
Expand All @@ -19,7 +19,7 @@ interface FirstArgumentStringCallExpression extends TSESTree.CallExpression {
arguments: [ArgumentLiteral];
}

type CallExpressionWithCorrectCalleeAndArguments = JestFunctionCallExpression<
type CallExpressionWithCorrectCalleeAndArguments = JestFunctionCallExpressionWithIdentifierCallee<
TestCaseName.it | TestCaseName.test | DescribeAlias.describe
> &
FirstArgumentStringCallExpression;
Expand All @@ -36,7 +36,7 @@ const isJestFunctionWithLiteralArg = (
node: TSESTree.CallExpression,
): node is CallExpressionWithCorrectCalleeAndArguments =>
(isTestCase(node) || isDescribe(node)) &&
['it', 'test', 'describe'].includes(node.callee.name) &&
node.callee.type === AST_NODE_TYPES.Identifier &&
hasStringAsFirstArgument(node);

const testDescription = (argument: ArgumentLiteral): string | null => {
Expand Down
33 changes: 24 additions & 9 deletions src/rules/tsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,35 @@ export enum HookName {

export type JestFunctionName = DescribeAlias | TestCaseName | HookName;

interface JestFunctionIdentifier<FunctionName extends JestFunctionName>
export interface JestFunctionIdentifier<FunctionName extends JestFunctionName>
extends TSESTree.Identifier {
name: FunctionName;
}

export interface JestFunctionCallExpression<
FunctionName extends JestFunctionName = JestFunctionName
export interface JestFunctionMemberExpression<
FunctionName extends JestFunctionName
> extends TSESTree.MemberExpression {
object: JestFunctionIdentifier<FunctionName>;
}

export interface JestFunctionCallExpressionWithMemberExpressionCallee<
FunctionName extends JestFunctionName
> extends TSESTree.CallExpression {
callee: JestFunctionMemberExpression<FunctionName>;
}

export interface JestFunctionCallExpressionWithIdentifierCallee<
FunctionName extends JestFunctionName
> extends TSESTree.CallExpression {
callee: JestFunctionIdentifier<FunctionName>;
}

export type JestFunctionCallExpression<
FunctionName extends JestFunctionName = JestFunctionName
> =
| JestFunctionCallExpressionWithMemberExpressionCallee<FunctionName>
| JestFunctionCallExpressionWithIdentifierCallee<FunctionName>;

export const getNodeName = (node: TSESTree.Node): string | null => {
function joinNames(a?: string | null, b?: string | null): string | null {
return a && b ? `${a}.${b}` : null;
Expand Down Expand Up @@ -80,13 +98,10 @@ export const isFunction = (node: TSESTree.Node): node is FunctionExpression =>
/* istanbul ignore next */
export const isHook = (
node: TSESTree.CallExpression,
): node is JestFunctionCallExpression<HookName> => {
): node is JestFunctionCallExpressionWithIdentifierCallee<HookName> => {
return (
(node.callee.type === AST_NODE_TYPES.Identifier &&
node.callee.name in HookName) ||
(node.callee.type === AST_NODE_TYPES.MemberExpression &&
node.callee.object.type === AST_NODE_TYPES.Identifier &&
node.callee.object.name in HookName)
node.callee.type === AST_NODE_TYPES.Identifier &&
node.callee.name in HookName
Copy link
Collaborator Author

@G-Rath G-Rath Jul 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I've changed this b/c jest doesn't expect hooks to expose a method property, and this way the two rules that use them don't need to be changed :D

isHook is only used by no-duplicate-hooks and no-hooks.

Copy link
Member

@SimenB SimenB Jul 20, 2019

Choose a reason for hiding this comment

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

I don't follow - you can do describe.skip (etc). Or do I misunderstand your comment?

Copy link
Collaborator Author

@G-Rath G-Rath Jul 20, 2019

Choose a reason for hiding this comment

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

But describe isn't a hook - the HookName enum is for beforeAll, beforeEach, afterEach & afterAll.

That's why it's the only one I made this change to - the others I left as it b/c in the JS version they have member calls (i.e describe.skip)

Copy link
Member

@SimenB SimenB Jul 20, 2019

Choose a reason for hiding this comment

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

hah, good point! There are indeed no identifiers on hooks

);
};

Expand Down