Skip to content
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
51 changes: 0 additions & 51 deletions src/rules/__tests__/no-hooks.test.js

This file was deleted.

60 changes: 60 additions & 0 deletions src/rules/__tests__/no-hooks.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import rule from '../no-hooks';
import { HookName } from '../tsUtils';

const ruleTester = new TSESLint.RuleTester({
parserOptions: {
ecmaVersion: 6,
},
});

ruleTester.run('no-hooks', rule, {
valid: [
'test("foo")',
'describe("foo", () => { it("bar") })',
'test("foo", () => { expect(subject.beforeEach()).toBe(true) })',
{
code: 'afterEach(() => {}); afterAll(() => {});',
options: [{ allow: [HookName.afterEach, HookName.afterAll] }],
},
],
invalid: [
{
code: 'beforeAll(() => {})',
errors: [
{ messageId: 'unexpectedHook', data: { hookName: HookName.beforeAll } },
],
},
{
code: 'beforeEach(() => {})',
errors: [
{
messageId: 'unexpectedHook',
data: { hookName: HookName.beforeEach },
},
],
},
{
code: 'afterAll(() => {})',
errors: [
{ messageId: 'unexpectedHook', data: { hookName: HookName.afterAll } },
],
},
{
code: 'afterEach(() => {})',
errors: [
{ messageId: 'unexpectedHook', data: { hookName: HookName.afterEach } },
],
},
{
code: 'beforeEach(() => {}); afterEach(() => { jest.resetModules() });',
options: [{ allow: [HookName.afterEach] }],
errors: [
{
messageId: 'unexpectedHook',
data: { hookName: HookName.beforeEach },
},
],
},
],
});
46 changes: 0 additions & 46 deletions src/rules/no-hooks.js

This file was deleted.

50 changes: 50 additions & 0 deletions src/rules/no-hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { HookName, createRule, isHook } from './tsUtils';

export default createRule({
name: __filename,
meta: {
docs: {
category: 'Best Practices',
description: 'Disallow setup and teardown hooks',
recommended: false,
},
messages: {
unexpectedHook: "Unexpected '{{ hookName }}' hook",
},
schema: [
{
type: 'object',
properties: {
allow: {
type: 'array',
contains: ['beforeAll', 'beforeEach', 'afterAll', 'afterEach'],
},
},
additionalProperties: false,
},
],
type: 'suggestion',
},
defaultOptions: [{ allow: [] } as { allow: readonly HookName[] }],
create(context, [{ allow }]) {
const whitelistedHookNames = allow.reduce((hashMap, value) => {
hashMap[value] = true;
return hashMap;
}, Object.create(null));

const isWhitelisted = (node: { callee: { name: string } }) =>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const isWhitelisted = (node: { callee: { name: string } }) =>
const isWhitelisted = (node: JestFunctionCallExpressionWithIdentifierCallee<HookName>) =>

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.

Could we possibly do export type JestHookFunction = JestFunctionCallExpressionWithIdentifierCallee<HookName> or something? 🙂 The name is so long... 😛

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could we possibly do...

fo sho

The name is so long... 😛

Yeah, it is - I might do a clean up PR soon, and rename them to something like Fn to make them a touch shorter.

Sadly, that's just pretty much what always happens when working w/ AST nodes - there's so much contextual type information that you end up w/ really very verbose but long type names 😂

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we can go through after everything is migrated. I bet we'll see some patterns or something we can abstract out to make it a bit more readable at that point

whitelistedHookNames[node.callee.name];

return {
CallExpression(node) {
if (isHook(node) && !isWhitelisted(node)) {
context.report({
node,
messageId: 'unexpectedHook',
data: { hookName: node.callee.name },
});
}
},
};
},
});
1 change: 0 additions & 1 deletion src/rules/tsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ export const isFunction = (node: TSESTree.Node): node is FunctionExpression =>
node.type === AST_NODE_TYPES.FunctionExpression ||
node.type === AST_NODE_TYPES.ArrowFunctionExpression;

/* istanbul ignore next */
export const isHook = (
node: TSESTree.CallExpression,
): node is JestFunctionCallExpressionWithIdentifierCallee<HookName> => {
Expand Down
13 changes: 0 additions & 13 deletions src/rules/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,6 @@ const describeAliases = new Set(['describe', 'fdescribe', 'xdescribe']);

const testCaseNames = new Set(['fit', 'it', 'test', 'xit', 'xtest']);

const testHookNames = new Set([
'beforeAll',
'beforeEach',
'afterAll',
'afterEach',
]);

export const getNodeName = node => {
function joinNames(a, b) {
return a && b ? `${a}.${b}` : null;
Expand All @@ -128,12 +121,6 @@ export const getNodeName = node => {
return null;
};

export const isHook = node =>
node &&
node.type === 'CallExpression' &&
node.callee.type === 'Identifier' &&
testHookNames.has(node.callee.name);

export const isTestCase = node =>
node &&
node.type === 'CallExpression' &&
Expand Down