diff --git a/src/rules/__tests__/valid-expect-in-promise.test.ts b/src/rules/__tests__/valid-expect-in-promise.test.ts index 5dbb0c7fd..7d4c636b7 100644 --- a/src/rules/__tests__/valid-expect-in-promise.test.ts +++ b/src/rules/__tests__/valid-expect-in-promise.test.ts @@ -219,26 +219,42 @@ ruleTester.run('valid-expect-in-promise', rule, { }) }); `, - // todo: as variable - // dedent` - // test('later return', () => { - // const promise = something().then(value => { - // expect(value).toBe('red'); - // }); - // - // return promise; - // }); - // `, - // todo: as variable - // dedent` - // test.only('later return', () => { - // const promise = something().then(value => { - // expect(value).toBe('red'); - // }); - // - // return promise; - // }); - // `, + dedent` + test('later return', () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return promise; + }); + `, + dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + await promise; + }); + `, + dedent` + test.only('later return', () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return promise; + }); + `, + dedent` + test('that we bailout if destructuring is used', () => { + const [promise] = [ + something().then(value => { + expect(value).toBe('red'); + }) + ]; + }); + `, dedent` it('shorthand arrow', () => something().then(value => { @@ -248,37 +264,14 @@ ruleTester.run('valid-expect-in-promise', rule, { }) ); `, - // todo: as variable - // dedent` - // it('promise test', () => { - // const somePromise = getThatPromise(); - // somePromise.then((data) => { - // expect(data).toEqual('foo'); - // }); - // expect(somePromise).toBeDefined(); - // return somePromise; - // }); - // `, - // todo: as variable - // dedent` - // test('promise test', function () { - // let somePromise = getThatPromise(); - // somePromise.then((data) => { - // expect(data).toEqual('foo'); - // }); - // expect(somePromise).toBeDefined(); - // return somePromise; - // }); - // `, - // todo: as variable - // dedent` - // it('crawls for files based on patterns', () => { - // const promise = nodeCrawl({}).then(data => { - // expect(childProcess.spawn).lastCalledWith('find'); - // }); - // return promise; - // }); - // `, + dedent` + it('crawls for files based on patterns', () => { + const promise = nodeCrawl({}).then(data => { + expect(childProcess.spawn).lastCalledWith('find'); + }); + return promise; + }); + `, dedent` it( 'test function', @@ -647,5 +640,43 @@ ruleTester.run('valid-expect-in-promise', rule, { `, errors: [{ column: 3, endColumn: 5, messageId: 'returnPromise' }], }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + promise; + }); + `, + errors: [{ column: 3, endColumn: 6, messageId: 'returnPromise' }], + }, + { + code: dedent` + it('promise test', () => { + const somePromise = getThatPromise(); + somePromise.then((data) => { + expect(data).toEqual('foo'); + }); + expect(somePromise).toBeDefined(); + return somePromise; + }); + `, + errors: [{ column: 3, endColumn: 6, messageId: 'returnPromise' }], + }, + { + code: dedent` + test('promise test', function () { + let somePromise = getThatPromise(); + somePromise.then((data) => { + expect(data).toEqual('foo'); + }); + expect(somePromise).toBeDefined(); + return somePromise; + }); + `, + errors: [{ column: 3, endColumn: 6, messageId: 'returnPromise' }], + }, ], }); diff --git a/src/rules/valid-expect-in-promise.ts b/src/rules/valid-expect-in-promise.ts index 7d5affe9c..5d96618c1 100644 --- a/src/rules/valid-expect-in-promise.ts +++ b/src/rules/valid-expect-in-promise.ts @@ -10,6 +10,7 @@ import { getNodeName, isExpectCall, isFunction, + isIdentifier, isSupportedAccessor, isTestCaseCall, } from './utils'; @@ -60,9 +61,13 @@ const reportReturnRequired = (context: RuleContext, node: TSESTree.Node) => { }); }; -const findTopOfBodyNode = ( +type StartingStatementNode = TSESTree.Statement & { + parent: TSESTree.BlockStatement; +}; + +const findStartingStatementInTestBody = ( node: PromiseChainCallExpression, -): TSESTree.Statement | null => { +): StartingStatementNode | null => { let { parent } = node; while (parent) { @@ -71,7 +76,7 @@ const findTopOfBodyNode = ( parent.parent.parent?.parent?.type === AST_NODE_TYPES.CallExpression && isTestCaseCall(parent.parent.parent?.parent) ) { - return parent as TSESTree.Statement; + return parent as StartingStatementNode; } } @@ -116,6 +121,49 @@ const isTestCaseCallWithCallbackArg = ( return false; }; +const getVariableName = ( + variable: TSESTree.VariableDeclarator, +): string | null => { + if (isIdentifier(variable.id)) { + return variable.id.name; + } + + return null; +}; + +const isVariableAwaitedOrReturned = ( + variables: TSESTree.VariableDeclaration & { parent: TSESTree.BlockStatement }, +): boolean => { + const { body } = variables.parent; + const [variable] = variables.declarations; + const name = getVariableName(variable); + + // null means that the variable is destructured, which is pretty much impossible + // for us to track, so we return true to bailout gracefully + if (name === null) { + return true; + } + + for (const node of body) { + if ( + node.type === AST_NODE_TYPES.ReturnStatement && + node.argument?.type === AST_NODE_TYPES.Identifier + ) { + return isIdentifier(node.argument, name); + } + + if ( + node.type === AST_NODE_TYPES.ExpressionStatement && + node.expression.type === AST_NODE_TYPES.AwaitExpression && + node.expression.argument?.type === AST_NODE_TYPES.Identifier + ) { + return isIdentifier(node.expression.argument, name); + } + } + + return false; +}; + export default createRule({ name: __filename, meta: { @@ -137,7 +185,7 @@ export default createRule({ const chains: boolean[] = []; return { - CallExpression(node) { + CallExpression(node: TSESTree.CallExpression) { if (isTestCaseCallWithCallbackArg(node)) { inTestCaseWithDoneCallback = true; @@ -171,10 +219,20 @@ export default createRule({ if (isPromiseChainCall(node)) { if (chains.shift()) { - const topNode = findTopOfBodyNode(node); + const topNode = findStartingStatementInTestBody(node); + + if (!topNode) { + return; + } + + if ( + topNode.type === AST_NODE_TYPES.VariableDeclaration && + isVariableAwaitedOrReturned(topNode) + ) { + return; + } if ( - !topNode || topNode.type === AST_NODE_TYPES.ReturnStatement || (topNode.type === AST_NODE_TYPES.ExpressionStatement && topNode.expression.type === AST_NODE_TYPES.AwaitExpression)