[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
vitest-dev · Dec 2, 2024 · a1ef516 · a1ef516
1 parent 6ad414f
commit a1ef516
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 22 deletions.
diff --git 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
@@ -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<RuleContext<MESSAGE_IDS, []>>) => {
+  blockStatement.body.forEach((node) => {
+    if (node.type !== AST_NODE_TYPES.ReturnStatement) return
+
+    context.report({
+      messageId: 'unexpectedReturnInDescribe',
+      node
+    })
+  })
+}
+
 export default createEslintRule<Options, MESSAGE_IDS>({
   name: RULE_NAME,
   meta: {
@@ -55,48 +71,62 @@ export default createEslintRule<Options, MESSAGE_IDS>({
           })
         }
 
-        const [, callback] = node.arguments
+        const [, arg2, arg3] = node.arguments
 
-        if (!callback) {
+        if (!arg2) {
           context.report({
             messageId: 'nameAndCallback',
             loc: paramsLocation(node.arguments)
           })
           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)
           })
           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
@@ -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 }
+      ],
+    },
   ]
 })