Skip to content

ts-migration/convert-is-utils-fns #310

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 9 commits into from
Jul 19, 2019
Merged

ts-migration/convert-is-utils-fns #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7af4d68
chore: convert `isHook` util fn to typescript
G-Rath Jul 18, 2019
0cd9c35
chore: convert `isTestCase` util fn to typescript
G-Rath Jul 18, 2019
4292e6c
chore: convert util enums to string enums
G-Rath Jul 18, 2019
ddd65ad
chore: export util enums
G-Rath Jul 18, 2019
51a4d62
chore: make `is<x>` util functions type guards
G-Rath Jul 18, 2019
b46d546
chore: refactor `lowercase-name` to use new util guards
G-Rath Jul 18, 2019
f91e05a
chore: remove unneeded `istanbul ignore next`
G-Rath Jul 18, 2019
9a80161
chore: use util enums in `lowercase-name` tests
G-Rath Jul 18, 2019
1e168e0
chore: use range type
SimenB Jul 19, 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
39 changes: 20 additions & 19 deletions src/rules/__tests__/lowercase-name.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import rule from '../lowercase-name';
import { DescribeAlias, TestCaseName } from '../tsUtils';

const ruleTester = new TSESLint.RuleTester({
parserOptions: {
Expand Down Expand Up @@ -54,7 +55,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'it' },
data: { method: TestCaseName.it },
column: 1,
line: 1,
},
Expand All @@ -66,7 +67,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'it' },
data: { method: TestCaseName.it },
column: 1,
line: 1,
},
Expand All @@ -78,7 +79,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'it' },
data: { method: TestCaseName.it },
column: 1,
line: 1,
},
Expand All @@ -90,7 +91,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'test' },
data: { method: TestCaseName.test },
column: 1,
line: 1,
},
Expand All @@ -102,7 +103,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'test' },
data: { method: TestCaseName.test },
column: 1,
line: 1,
},
Expand All @@ -114,7 +115,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'test' },
data: { method: TestCaseName.test },
column: 1,
line: 1,
},
Expand All @@ -126,7 +127,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'describe' },
data: { method: DescribeAlias.describe },
column: 1,
line: 1,
},
Expand All @@ -138,7 +139,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'describe' },
data: { method: DescribeAlias.describe },
column: 1,
line: 1,
},
Expand All @@ -150,7 +151,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'describe' },
data: { method: DescribeAlias.describe },
column: 1,
line: 1,
},
Expand All @@ -162,7 +163,7 @@ ruleTester.run('lowercase-name', rule, {
errors: [
{
messageId: 'unexpectedLowercase',
data: { method: 'describe' },
data: { method: DescribeAlias.describe },
column: 1,
line: 1,
},
Expand All @@ -175,15 +176,15 @@ ruleTester.run('lowercase-name with ignore=describe', rule, {
valid: [
{
code: "describe('Foo', function () {})",
options: [{ ignore: ['describe'] }],
options: [{ ignore: [DescribeAlias.describe] }],
},
{
code: 'describe("Foo", function () {})',
options: [{ ignore: ['describe'] }],
options: [{ ignore: [DescribeAlias.describe] }],
},
{
code: 'describe(`Foo`, function () {})',
options: [{ ignore: ['describe'] }],
options: [{ ignore: [DescribeAlias.describe] }],
},
],
invalid: [],
Expand All @@ -193,15 +194,15 @@ ruleTester.run('lowercase-name with ignore=test', rule, {
valid: [
{
code: "test('Foo', function () {})",
options: [{ ignore: ['test'] }],
options: [{ ignore: [TestCaseName.test] }],
},
{
code: 'test("Foo", function () {})',
options: [{ ignore: ['test'] }],
options: [{ ignore: [TestCaseName.test] }],
},
{
code: 'test(`Foo`, function () {})',
options: [{ ignore: ['test'] }],
options: [{ ignore: [TestCaseName.test] }],
},
],
invalid: [],
Expand All @@ -211,15 +212,15 @@ ruleTester.run('lowercase-name with ignore=it', rule, {
valid: [
{
code: "it('Foo', function () {})",
options: [{ ignore: ['it'] }],
options: [{ ignore: [TestCaseName.it] }],
},
{
code: 'it("Foo", function () {})',
options: [{ ignore: ['it'] }],
options: [{ ignore: [TestCaseName.it] }],
},
{
code: 'it(`Foo`, function () {})',
options: [{ ignore: ['it'] }],
options: [{ ignore: [TestCaseName.it] }],
},
],
invalid: [],
Expand Down
45 changes: 17 additions & 28 deletions src/rules/lowercase-name.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,29 @@
import {
AST_NODE_TYPES,
TSESLint,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import { createRule } from './tsUtils';

type JestFunctionName = 'it' | 'test' | 'describe';

interface JestFunctionIdentifier extends TSESTree.Identifier {
name: JestFunctionName;
}

interface TestFunctionCallExpression extends TSESTree.CallExpression {
callee: JestFunctionIdentifier;
}
import {
DescribeAlias,
JestFunctionCallExpression,
JestFunctionName,
TestCaseName,
createRule,
isDescribe,
isTestCase,
} from './tsUtils';

type ArgumentLiteral = TSESTree.Literal | TSESTree.TemplateLiteral;

interface FirstArgumentStringCallExpression extends TSESTree.CallExpression {
arguments: [ArgumentLiteral];
}

type CallExpressionWithCorrectCalleeAndArguments = TestFunctionCallExpression &
type CallExpressionWithCorrectCalleeAndArguments = JestFunctionCallExpression<
TestCaseName.it | TestCaseName.test | DescribeAlias.describe
> &
FirstArgumentStringCallExpression;

const isItTestOrDescribeFunction = (
node: TSESTree.CallExpression,
): node is TestFunctionCallExpression => {
const { callee } = node;

if (callee.type !== AST_NODE_TYPES.Identifier) {
return false;
}

const { name } = callee;

return name === 'it' || name === 'test' || name === 'describe';
};

const hasStringAsFirstArgument = (
node: TSESTree.CallExpression,
): node is FirstArgumentStringCallExpression =>
Expand All @@ -48,7 +35,9 @@ const hasStringAsFirstArgument = (
const isJestFunctionWithLiteralArg = (
node: TSESTree.CallExpression,
): node is CallExpressionWithCorrectCalleeAndArguments =>
isItTestOrDescribeFunction(node) && hasStringAsFirstArgument(node);
(isTestCase(node) || isDescribe(node)) &&
['it', 'test', 'describe'].includes(node.callee.name) &&
Copy link
Member

@SimenB SimenB Jul 18, 2019

Choose a reason for hiding this comment

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

doesn't (isTestCase(node) || isDescribe(node)) cover this line?

Copy link
Collaborator Author

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

Choose a reason for hiding this comment

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

I was going to ask you about that :)

Previously the check was just for these three functions, and the description & tests for the rule only focus on those three:

Enforce it, test and describe to have descriptions that begin with a lowercase letter. This provides more readable test failures.

I think it should be fine to remove that line, but wanted to implement the original behaviour + ask first, in case that was part of the point of that rule that the community requested (or something)

Copy link
Member

@SimenB SimenB Jul 19, 2019

Choose a reason for hiding this comment

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

ah right, since we'd now get xtest, ftest etc? I think it's fine to detect all - they're just modifiers on the same functions

Copy link
Member

@SimenB SimenB Jul 19, 2019

Choose a reason for hiding this comment

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

We can do that in a follow-up, though

hasStringAsFirstArgument(node);

const testDescription = (argument: ArgumentLiteral): string | null => {
if (argument.type === AST_NODE_TYPES.Literal) {
Expand Down Expand Up @@ -142,7 +131,7 @@ export default createRule({
// guaranteed by jestFunctionName
const description = testDescription(firstArg)!;

const rangeIgnoringQuotes: [number, number] = [
const rangeIgnoringQuotes: TSESLint.AST.Range = [
firstArg.range[0] + 1,
firstArg.range[1] - 1,
];
Expand Down
65 changes: 60 additions & 5 deletions src/rules/tsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,38 @@ export const createRule = ESLintUtils.RuleCreator(name => {
return `${REPO_URL}/blob/v${version}/docs/rules/${ruleName}.md`;
});

enum DescribeAlias {
'describe',
'fdescribe',
'xdescribe',
export enum DescribeAlias {
'describe' = 'describe',
'fdescribe' = 'fdescribe',
'xdescribe' = 'xdescribe',
}

export enum TestCaseName {
'fit' = 'fit',
'it' = 'it',
'test' = 'test',
'xit' = 'xit',
'xtest' = 'xtest',
}

export enum HookName {
'beforeAll' = 'beforeAll',
'beforeEach' = 'beforeEach',
'afterAll' = 'afterAll',
'afterEach' = 'afterEach',
}

export type JestFunctionName = DescribeAlias | TestCaseName | HookName;

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

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

export type FunctionExpression =
Expand All @@ -28,7 +56,34 @@ export const isFunction = (node: TSESTree.Node): node is FunctionExpression =>
node.type === AST_NODE_TYPES.FunctionExpression ||
node.type === AST_NODE_TYPES.ArrowFunctionExpression;

export const isDescribe = (node: TSESTree.CallExpression): boolean => {
/* istanbul ignore next */
export const isHook = (
node: TSESTree.CallExpression,
): node is JestFunctionCallExpression<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)
);
};

export const isTestCase = (
node: TSESTree.CallExpression,
): node is JestFunctionCallExpression<TestCaseName> => {
return (
(node.callee.type === AST_NODE_TYPES.Identifier &&
node.callee.name in TestCaseName) ||
(node.callee.type === AST_NODE_TYPES.MemberExpression &&
node.callee.object.type === AST_NODE_TYPES.Identifier &&
node.callee.object.name in TestCaseName)
);
};

export const isDescribe = (
node: TSESTree.CallExpression,
): node is JestFunctionCallExpression<DescribeAlias> => {
return (
(node.callee.type === AST_NODE_TYPES.Identifier &&
node.callee.name in DescribeAlias) ||
Expand Down