eslint-plugin-jest
eslint-plugin-jest copied to clipboard
[prefer-expect-assertions] expect check doesn't account for scope
I realised this while doing #1191 and gosh is it a tough one: prefer-expect-assertions
is currently checking if a function call is expect.hasAssertions
or expect.assertions
using non-scoped means:
const isExpectAssertionsOrHasAssertionsCall = (
expression: TSESTree.Node,
): expression is KnownCallExpression<'assertions' | 'hasAssertions'> =>
expression.type === AST_NODE_TYPES.CallExpression &&
expression.callee.type === AST_NODE_TYPES.MemberExpression &&
isSupportedAccessor(expression.callee.object, 'expect') &&
isSupportedAccessor(expression.callee.property) &&
['assertions', 'hasAssertions'].includes(
getAccessorValue(expression.callee.property),
);
This can be fixed with this:
const isExpectAssertionsOrHasAssertionsCall2 = (
jestFnCall: ParsedJestFnCall,
) => {
return (
jestFnCall.type === 'expect' &&
jestFnCall.head.node.parent?.type ===
AST_NODE_TYPES.MemberExpression &&
jestFnCall.members.length === 1 &&
['assertions', 'hasAssertions'].includes(
getAccessorValue(jestFnCall.members[0]),
)
);
};
However, there's a very subtle problem which is that the rule works by checking the first statement in the body of the function being passed to test
or it
- this is a problem because context.getScope()
is stateful, so calling parseJestFnCall
will have it use the scope of the test
/it
method rather than the scope of the function body the expression is in.
Because we're checking the first expression it's harder to exploit, but e.g. this won't fail:
it("returns numbers that are greater than four", function(expect) {
expect.assertions(2);
for(let thing in things) {
expect(number).toBeGreaterThan(4);
}
});
I think we should be able to solve this by switching to preemptively checking every first statement in a test
/it
function body and then check if is anything that means we should not report (i.e. loops) - it'll add a bit more complexity but I don't think we can resolve this bug otherwise 🤷
When I get a chance I'll probably land the initial fix first since that is still part of this and will resolve the main bug.