Skip to content

Commit

Permalink
fix(valid-expect-in-promise): handle multi-chained promises properly
Browse files Browse the repository at this point in the history
Method call chains are represented in AST in a nested fashion, meaning
it's not enough to just separately track if we're both in a promise
chain call and then if we've found an `expect` call, because when we
exit another CallExpression in the same chain it'll look like that has
an `expect` call.

Instead, we need to track our depth as we enter CallExpression nodes so
that when we exit those nodes we can check if we encountered an `expect`
 call at that same depth.
  • Loading branch information
G-Rath committed Oct 2, 2021
1 parent 10abcdb commit 3be31bf
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 8 deletions.
122 changes: 122 additions & 0 deletions src/rules/__tests__/valid-expect-in-promise.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,26 @@ ruleTester.run('valid-expect-in-promise', rule, {
})
});
`,
dedent`
it('it1', () => {
return somePromise.then(() => {
return value;
})
.then(value => {
expect(someThing).toEqual(value);
})
});
`,
dedent`
it('it1', () => {
return somePromise.then(() => {
expect(someThing).toEqual(true);
})
.then(() => {
console.log('this is silly');
})
});
`,
dedent`
it('it1', () => {
return somePromise.then(() => {
Expand Down Expand Up @@ -481,6 +501,108 @@ ruleTester.run('valid-expect-in-promise', rule, {
`,
errors: [{ column: 3, endColumn: 5, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise
.then(() => {})
.then(() => expect(someThing).toEqual(value))
});
`,
errors: [{ column: 3, endColumn: 50, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise
.then(() => expect(someThing).toEqual(value))
.then(() => {})
});
`,
errors: [{ column: 3, endColumn: 20, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise.then(() => {
return value;
})
.then(value => {
expect(someThing).toEqual(value);
})
});
`,
errors: [{ column: 3, endColumn: 5, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise.then(() => {
expect(someThing).toEqual(true);
})
.then(() => {
console.log('this is silly');
})
});
`,
errors: [{ column: 3, endColumn: 5, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise.then(() => {
// return value;
})
.then(value => {
expect(someThing).toEqual(value);
})
});
`,
errors: [{ column: 3, endColumn: 5, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise.then(() => {
return value;
})
.then(value => {
expect(someThing).toEqual(value);
})
return anotherPromise.then(() => expect(x).toBe(y));
});
`,
errors: [{ column: 3, endColumn: 5, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise
.then(() => 1)
.then(x => x + 1)
.catch(() => -1)
.then(v => expect(v).toBe(2));
return anotherPromise.then(() => expect(x).toBe(y));
});
`,
errors: [{ column: 3, endColumn: 35, messageId: 'returnPromise' }],
},
{
code: dedent`
it('is a test', () => {
somePromise
.then(() => 1)
.then(v => expect(v).toBe(2))
.then(x => x + 1)
.catch(() => -1);
return anotherPromise.then(() => expect(x).toBe(y));
});
`,
errors: [{ column: 3, endColumn: 22, messageId: 'returnPromise' }],
},
{
code: dedent`
it('it1', () => {
Expand Down
13 changes: 5 additions & 8 deletions src/rules/valid-expect-in-promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ export default createRule<unknown[], MessageIds>({
defaultOptions: [],
create(context) {
let inTestCaseWithDoneCallback = false;
let inPromiseChain = false;
let hasExpectCall = false;
const chains: boolean[] = [];

return {
CallExpression(node) {
Expand All @@ -146,17 +145,17 @@ export default createRule<unknown[], MessageIds>({
}

if (isPromiseChainCall(node)) {
inPromiseChain = true;
chains.unshift(false);

return;
}

if (!inPromiseChain) {
if (chains.length === 0) {
return;
}

if (isExpectCall(node)) {
hasExpectCall = true;
chains[0] = true;

return;
}
Expand All @@ -171,9 +170,7 @@ export default createRule<unknown[], MessageIds>({
}

if (isPromiseChainCall(node)) {
inPromiseChain = false;

if (hasExpectCall) {
if (chains.shift()) {
const topNode = findTopOfBodyNode(node);

if (
Expand Down

0 comments on commit 3be31bf

Please sign in to comment.