From d0a9ed6bb419507d36bdc6fda4c79a687dcb57c9 Mon Sep 17 00:00:00 2001 From: M4rk9696 Date: Thu, 18 Jul 2019 19:25:45 +0530 Subject: [PATCH 1/4] chore: migrate valid-describe to ts --- ...escribe.test.js => valid-describe.test.ts} | 5 +- src/rules/tsUtils.ts | 24 ++++++++++ .../{valid-describe.js => valid-describe.ts} | 47 +++++++++++++------ 3 files changed, 59 insertions(+), 17 deletions(-) rename src/rules/__tests__/{valid-describe.test.js => valid-describe.test.ts} (96%) rename src/rules/{valid-describe.js => valid-describe.ts} (66%) diff --git a/src/rules/__tests__/valid-describe.test.js b/src/rules/__tests__/valid-describe.test.ts similarity index 96% rename from src/rules/__tests__/valid-describe.test.js rename to src/rules/__tests__/valid-describe.test.ts index 211d1537b..713b87564 100644 --- a/src/rules/__tests__/valid-describe.test.js +++ b/src/rules/__tests__/valid-describe.test.ts @@ -1,7 +1,8 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../valid-describe'; -const ruleTester = new RuleTester({ +const ruleTester = new TSESLint.RuleTester({ + parser: '@typescript-eslint/parser', parserOptions: { ecmaVersion: 8, }, diff --git a/src/rules/tsUtils.ts b/src/rules/tsUtils.ts index ca3891053..53dcf3718 100644 --- a/src/rules/tsUtils.ts +++ b/src/rules/tsUtils.ts @@ -2,6 +2,10 @@ import { basename } from 'path'; import { ESLintUtils } from '@typescript-eslint/experimental-utils'; import { version } from '../../package.json'; +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; const REPO_URL = 'https://github.com/jest-community/eslint-plugin-jest'; @@ -9,3 +13,23 @@ export const createRule = ESLintUtils.RuleCreator(name => { const ruleName = basename(name, '.ts'); return `${REPO_URL}/blob/v${version}/docs/rules/${ruleName}.md`; }); + +enum DescribeAlias { + 'describe', + 'fdescribe', + 'xdescribe', +} + +export const isFunction = (node: TSESTree.Node): boolean => + node.type === AST_NODE_TYPES.FunctionExpression || + node.type === AST_NODE_TYPES.ArrowFunctionExpression; + +export const isDescribe = (node: TSESTree.CallExpression): boolean => { + return ( + (node.callee.type === AST_NODE_TYPES.Identifier && + node.callee.name in DescribeAlias) || + (node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.Identifier && + node.callee.object.name in DescribeAlias) + ); +}; diff --git a/src/rules/valid-describe.js b/src/rules/valid-describe.ts similarity index 66% rename from src/rules/valid-describe.js rename to src/rules/valid-describe.ts index 0fa9c7cdd..3116c9fc9 100644 --- a/src/rules/valid-describe.js +++ b/src/rules/valid-describe.ts @@ -1,14 +1,21 @@ -import { getDocsUrl, isDescribe, isFunction } from './util'; +import { createRule, isDescribe, isFunction } from './tsUtils'; +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree'; -const isAsync = node => node.async; +type FunctionExpression = + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionExpression; -const isString = node => - (node.type === 'Literal' && typeof node.value === 'string') || - node.type === 'TemplateLiteral'; +const isAsync = (node: FunctionExpression): boolean => node.async; -const hasParams = node => node.params.length > 0; +const isString = (node: TSESTree.Node): boolean => + (node.type === AST_NODE_TYPES.Literal && typeof node.value === 'string') || + node.type === AST_NODE_TYPES.TemplateLiteral; -const paramsLocation = params => { +const hasParams = (node: FunctionExpression): boolean => node.params.length > 0; + +const paramsLocation = ( + params: TSESTree.Expression[] | TSESTree.Parameter[], +) => { const [first] = params; const last = params[params.length - 1]; return { @@ -23,10 +30,15 @@ const paramsLocation = params => { }; }; -export default { +export default createRule({ + name: __filename, meta: { + type: 'problem', docs: { - url: getDocsUrl(__filename), + description: + 'Using an improper `describe()` callback function can lead to unexpected test errors.', + category: 'Possible Errors', + recommended: 'warn', }, messages: { nameAndCallback: 'Describe requires name and callback arguments', @@ -38,7 +50,8 @@ export default { 'Unexpected return statement in describe callback', }, schema: [], - }, + } as const, + defaultOptions: [], create(context) { return { CallExpression(node) { @@ -51,14 +64,14 @@ export default { } const [name] = node.arguments; - const [, callbackFunction] = node.arguments; + let [, callbackFunction] = node.arguments; if (!isString(name)) { context.report({ messageId: 'firstArgumentMustBeName', loc: paramsLocation(node.arguments), }); } - if (callbackFunction === undefined) { + if (!callbackFunction) { context.report({ messageId: 'nameAndCallback', loc: paramsLocation(node.arguments), @@ -73,7 +86,8 @@ export default { }); return; - } + } else callbackFunction = callbackFunction as FunctionExpression; + if (isAsync(callbackFunction)) { context.report({ messageId: 'noAsyncDescribeCallback', @@ -86,7 +100,10 @@ export default { loc: paramsLocation(callbackFunction.params), }); } - if (callbackFunction.body.type === 'BlockStatement') { + if ( + callbackFunction.body && + callbackFunction.body.type === AST_NODE_TYPES.BlockStatement + ) { callbackFunction.body.body.forEach(node => { if (node.type === 'ReturnStatement') { context.report({ @@ -100,4 +117,4 @@ export default { }, }; }, -}; +}); From 3d02e8073f404a22bdca0756344065470ce6281e Mon Sep 17 00:00:00 2001 From: M4rk9696 Date: Thu, 18 Jul 2019 22:42:25 +0530 Subject: [PATCH 2/4] refactor: review changes - Merge import statements - Extract type FunctionExpression - Remove usage of parser - Change the order in which cases are handled - Add type guard --- src/rules/__tests__/valid-describe.test.ts | 1 - src/rules/tsUtils.ts | 10 +++- src/rules/valid-describe.ts | 68 +++++++++++----------- 3 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/rules/__tests__/valid-describe.test.ts b/src/rules/__tests__/valid-describe.test.ts index 713b87564..adfbad592 100644 --- a/src/rules/__tests__/valid-describe.test.ts +++ b/src/rules/__tests__/valid-describe.test.ts @@ -2,7 +2,6 @@ import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../valid-describe'; const ruleTester = new TSESLint.RuleTester({ - parser: '@typescript-eslint/parser', parserOptions: { ecmaVersion: 8, }, diff --git a/src/rules/tsUtils.ts b/src/rules/tsUtils.ts index 53dcf3718..11eb3ecb8 100644 --- a/src/rules/tsUtils.ts +++ b/src/rules/tsUtils.ts @@ -1,11 +1,11 @@ // TODO: rename to utils.ts when TS migration is complete import { basename } from 'path'; -import { ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { version } from '../../package.json'; import { AST_NODE_TYPES, + ESLintUtils, TSESTree, } from '@typescript-eslint/experimental-utils'; +import { version } from '../../package.json'; const REPO_URL = 'https://github.com/jest-community/eslint-plugin-jest'; @@ -20,7 +20,11 @@ enum DescribeAlias { 'xdescribe', } -export const isFunction = (node: TSESTree.Node): boolean => +export type FunctionExpression = + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionExpression; + +export const isFunction = (node: TSESTree.Node): node is FunctionExpression => node.type === AST_NODE_TYPES.FunctionExpression || node.type === AST_NODE_TYPES.ArrowFunctionExpression; diff --git a/src/rules/valid-describe.ts b/src/rules/valid-describe.ts index 3116c9fc9..82dbec4e9 100644 --- a/src/rules/valid-describe.ts +++ b/src/rules/valid-describe.ts @@ -1,10 +1,11 @@ -import { createRule, isDescribe, isFunction } from './tsUtils'; +import { + FunctionExpression, + createRule, + isDescribe, + isFunction, +} from './tsUtils'; import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree'; -type FunctionExpression = - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionExpression; - const isAsync = (node: FunctionExpression): boolean => node.async; const isString = (node: TSESTree.Node): boolean => @@ -64,7 +65,7 @@ export default createRule({ } const [name] = node.arguments; - let [, callbackFunction] = node.arguments; + const [, callbackFunction] = node.arguments; if (!isString(name)) { context.report({ messageId: 'firstArgumentMustBeName', @@ -79,39 +80,38 @@ export default createRule({ return; } - if (!isFunction(callbackFunction)) { + if (isFunction(callbackFunction)) { + if (isAsync(callbackFunction)) { + context.report({ + messageId: 'noAsyncDescribeCallback', + node: callbackFunction, + }); + } + if (hasParams(callbackFunction)) { + context.report({ + messageId: 'unexpectedDescribeArgument', + loc: paramsLocation(callbackFunction.params), + }); + } + if ( + callbackFunction.body && + callbackFunction.body.type === AST_NODE_TYPES.BlockStatement + ) { + callbackFunction.body.body.forEach(node => { + if (node.type === 'ReturnStatement') { + context.report({ + messageId: 'unexpectedReturnInDescribe', + node, + }); + } + }); + } + } else { context.report({ messageId: 'secondArgumentMustBeFunction', loc: paramsLocation(node.arguments), }); - return; - } else callbackFunction = callbackFunction as FunctionExpression; - - if (isAsync(callbackFunction)) { - context.report({ - messageId: 'noAsyncDescribeCallback', - node: callbackFunction, - }); - } - if (hasParams(callbackFunction)) { - context.report({ - messageId: 'unexpectedDescribeArgument', - loc: paramsLocation(callbackFunction.params), - }); - } - if ( - callbackFunction.body && - callbackFunction.body.type === AST_NODE_TYPES.BlockStatement - ) { - callbackFunction.body.body.forEach(node => { - if (node.type === 'ReturnStatement') { - context.report({ - messageId: 'unexpectedReturnInDescribe', - node, - }); - } - }); } } }, From a399661daa63d1ce705a91d3a47f8b1fdabf756f Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 18 Jul 2019 20:00:33 +0200 Subject: [PATCH 3/4] chore: move import --- src/rules/valid-describe.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/valid-describe.ts b/src/rules/valid-describe.ts index 82dbec4e9..ec6772918 100644 --- a/src/rules/valid-describe.ts +++ b/src/rules/valid-describe.ts @@ -1,10 +1,10 @@ +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree'; import { FunctionExpression, createRule, isDescribe, isFunction, } from './tsUtils'; -import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree'; const isAsync = (node: FunctionExpression): boolean => node.async; From a9324ad3d631af63b491db883ab54126ace4ef9e Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 18 Jul 2019 22:39:59 +0200 Subject: [PATCH 4/4] chore: simplify loc --- src/rules/valid-describe.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/rules/valid-describe.ts b/src/rules/valid-describe.ts index ec6772918..f4eafa3a5 100644 --- a/src/rules/valid-describe.ts +++ b/src/rules/valid-describe.ts @@ -20,15 +20,9 @@ const paramsLocation = ( const [first] = params; const last = params[params.length - 1]; return { - start: { - line: first.loc.start.line, - column: first.loc.start.column, - }, - end: { - line: last.loc.end.line, - column: last.loc.end.column, - }, - }; + start: first.loc.start, + end: last.loc.end, + } }; export default createRule({