From a1ef5162f50c934e28c9136a817ec2ce578677c7 Mon Sep 17 00:00:00 2001 From: Yukihiro Hasegawa <49516827+y-hsgw@users.noreply.github.com> Date: Mon, 2 Dec 2024 23:23:44 +0900 Subject: [PATCH] [valid-describe-callback] Fix to support options as second parameter for valid-describe-callback (#582) * fix: valid-describe-callback * test: add test cases for valid-describe-callback * docs: fix sample --- docs/rules/valid-describe-callback.md | 4 +- src/rules/valid-describe-callback.ts | 68 +++++++++++++++++++-------- tests/valid-describe-callback.test.ts | 49 ++++++++++++++++++- 3 files changed, 99 insertions(+), 22 deletions(-) diff --git a/docs/rules/valid-describe-callback.md b/docs/rules/valid-describe-callback.md index 43ffadb4..eb28f33e 100644 --- a/docs/rules/valid-describe-callback.md +++ b/docs/rules/valid-describe-callback.md @@ -29,11 +29,11 @@ describe("myfunc", () => { }) // returning a value from a describe block is not allowed -describe("myfunc", () => { +describe("myfunc", () => it("should do something", () => { // }) -}) +) ``` The following are not considered warnings: diff --git a/src/rules/valid-describe-callback.ts b/src/rules/valid-describe-callback.ts index ca86ee2d..5aea4e0f 100644 --- a/src/rules/valid-describe-callback.ts +++ b/src/rules/valid-describe-callback.ts @@ -1,6 +1,7 @@ import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils' -import { createEslintRule, getAccessorValue, isFunction } from '../utils' -import { parseVitestFnCall } from '../utils/parse-vitest-fn-call' +import { createEslintRule, FunctionExpression, getAccessorValue, isFunction } from '../utils' +import { ParsedVitestFnCall, parseVitestFnCall } from '../utils/parse-vitest-fn-call' +import { RuleContext } from '@typescript-eslint/utils/ts-eslint' export const RULE_NAME = 'valid-describe-callback' type MESSAGE_IDS = @@ -21,6 +22,21 @@ const paramsLocation = (params: TSESTree.CallExpressionArgument[] | TSESTree.Par } } +const hasNonEachMembersAndParams = (vitestFnCall: ParsedVitestFnCall, functionExpression: FunctionExpression) => { + return vitestFnCall.members.every(s => getAccessorValue(s) !== 'each') && functionExpression.params.length +} + +const reportUnexpectedReturnInDescribe = (blockStatement: TSESTree.BlockStatement, context: Readonly>) => { + blockStatement.body.forEach((node) => { + if (node.type !== AST_NODE_TYPES.ReturnStatement) return + + context.report({ + messageId: 'unexpectedReturnInDescribe', + node + }) + }) +} + export default createEslintRule({ name: RULE_NAME, meta: { @@ -55,9 +71,9 @@ export default createEslintRule({ }) } - const [, callback] = node.arguments + const [, arg2, arg3] = node.arguments - if (!callback) { + if (!arg2) { context.report({ messageId: 'nameAndCallback', loc: paramsLocation(node.arguments) @@ -65,7 +81,29 @@ export default createEslintRule({ return } - if (!isFunction(callback)) { + if (!isFunction(arg2)) { + if(arg3 && isFunction(arg3)) { + if (hasNonEachMembersAndParams(vitestFnCall, arg3)) { + context.report({ + messageId: 'unexpectedDescribeArgument', + node: arg3 + }) + } + + if (arg3.body.type === AST_NODE_TYPES.CallExpression) { + context.report({ + messageId: 'unexpectedReturnInDescribe', + node: arg3 + }) + } + + if (arg3.body.type === AST_NODE_TYPES.BlockStatement) { + reportUnexpectedReturnInDescribe(arg3.body, context) + } + + return + } + context.report({ messageId: 'secondArgumentMustBeFunction', loc: paramsLocation(node.arguments) @@ -73,30 +111,22 @@ export default createEslintRule({ return } - if (vitestFnCall.members.every(s => getAccessorValue(s) !== 'each') - && callback.params.length) { + if (hasNonEachMembersAndParams(vitestFnCall, arg2)) { context.report({ messageId: 'unexpectedDescribeArgument', - node: callback + node: arg2 }) } - if (callback.body.type === AST_NODE_TYPES.CallExpression) { + if (arg2.body.type === AST_NODE_TYPES.CallExpression) { context.report({ messageId: 'unexpectedReturnInDescribe', - node: callback + node: arg2 }) } - if (callback.body.type === AST_NODE_TYPES.BlockStatement) { - callback.body.body.forEach((node) => { - if (node.type === AST_NODE_TYPES.ReturnStatement) { - context.report({ - messageId: 'unexpectedReturnInDescribe', - node - }) - } - }) + if (arg2.body.type === AST_NODE_TYPES.BlockStatement) { + reportUnexpectedReturnInDescribe(arg2.body, context) } } } diff --git a/tests/valid-describe-callback.test.ts b/tests/valid-describe-callback.test.ts index ce0857f1..bc967b59 100644 --- a/tests/valid-describe-callback.test.ts +++ b/tests/valid-describe-callback.test.ts @@ -56,7 +56,16 @@ ruleTester.run(RULE_NAME, rule, { }); }); }); + `, ` + describe('foo', { only: true }, () => { + it('bar', () => { + return Promise.resolve(42).then(value => { + expect(value).toBe(42) + }) + }) + }) + `, ], invalid: [ { @@ -171,6 +180,38 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'unexpectedReturnInDescribe', line: 2, column: 24 } ] }, + { + code: ` + describe('foo', { only: true }, () => + test('bar', () => {}) + ) + `, + errors: [ + { messageId: 'unexpectedReturnInDescribe', line: 2, column: 40 } + ], + }, + { + code: ` + describe('foo', { only: true }, () => { + return Promise.resolve().then(() => { + it('breaks', () => { + throw new Error('Fail') + }) + }) + describe('nested', () => { + return Promise.resolve().then(() => { + it('breaks', () => { + throw new Error('Fail') + }) + }) + }) + }) + `, + errors: [ + { messageId: 'unexpectedReturnInDescribe', line: 3, column: 7 }, + { messageId: 'unexpectedReturnInDescribe', line: 9, column: 9 } + ], + }, { code: 'describe("foo", done => {})', errors: [ @@ -194,6 +235,12 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'unexpectedDescribeArgument', line: 1, column: 17 } ] - } + }, + { + code: 'describe("foo", { only: true }, done => {})', + errors: [ + { messageId: 'unexpectedDescribeArgument', line: 1, column: 33 } + ], + }, ] })