Skip to content

Commit

Permalink
fix(valid-expect-in-promise): handle promises assigned to variables
Browse files Browse the repository at this point in the history
  • Loading branch information
@G-Rath
G-Rath committed Oct 2, 2021
1 parent 3be31bf commit bc86bab
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 57 deletions.
133 changes: 82 additions & 51 deletions src/rules/__tests__/valid-expect-in-promise.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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',
Expand Down Expand Up @@ -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' }],
},
],
});
70 changes: 64 additions & 6 deletions src/rules/valid-expect-in-promise.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getNodeName,
isExpectCall,
isFunction,
isIdentifier,
isSupportedAccessor,
isTestCaseCall,
} from './utils';
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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<unknown[], MessageIds>({
name: __filename,
meta: {
Expand All @@ -137,7 +185,7 @@ export default createRule<unknown[], MessageIds>({
const chains: boolean[] = [];

return {
CallExpression(node) {
CallExpression(node: TSESTree.CallExpression) {
if (isTestCaseCallWithCallbackArg(node)) {
inTestCaseWithDoneCallback = true;

Expand Down Expand Up @@ -171,10 +219,20 @@ export default createRule<unknown[], MessageIds>({

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)
Expand Down

0 comments on commit bc86bab

Please sign in to comment.