[prefer-expect-assertions]: ensure expect.assertions is used when an expect is found to be within a loop

[benmonro]

It would be nice to have a lint rule that enforced a length check or use of expect.assertions when an expect call is found inside a loop.

for example:

for(let thing in things) {
  expect(thing).toBe("2");
}

would report an error

but this would not:

expect.assertions(4);

for(let thing in things) {
  expect(thing).toBe("2");
}

nor would this:

expect(things).toHaveLength(4);

for(let thing in things) {
  expect(thing).toBe("2");
}

Main goal here is to prevent false positive tests if things is an empty list.

[G-Rath]

@SimenB mentioned this case in #482 with a comment about them probably being too hard to detect properly, and I'm inclined to agree: I feel that we'd have either far too many edge-cases, or end up only covering half the ways you can loop.

We already have prefer-expect-assertions which encourages having either expect.assertions() or expect.hasAssertions(), which covers half of the rule.

[benmonro]

@G-Rath perfect is the enemy of good. It doesn't have to cover every possible type of loop. Even if it only captures some or most of them.

[G-Rath]

True, but I'm thinking about if we can hit "good" let alone "perfect": there are not a lot of ways to loop in JS, so it's reasonable to assume that such a rule would get at least the common ones (forEach & for).

I'm not against having said rule only work on say for loops, but that then calls into question how useful that rule actually will be vs the issues that people open about how the rule is "wrong" because it doesn't support forEach.

We already have this with rules like expect-expect: every few months we get an issue opened stating that the plugin incorrectly thinks a file is a test because someone used a function with the same name as one from the jest globals, which is because of the understand but still incorrect assumption that our plugin will "just know" what a test file is (#589 is the latest in this).

This is vs the base complexity that would be require - in particular, detecting a valid expect.assertions: there's a number of different ways you can call that function that is valid, and they're not easy to detect accurately every time.

This is why prefer-expect-assertions works by checking that the first statement in a function is a call to expect.assertions or expect.hasAssertions.

In theory that rule could find a call to either of those functions, and if they're not first report on those calls saying they should be at the top.

In practice, this requires iterating over every statement, going through every form of branch node (i.e every case in every switch, every if/else, try/catch - because those are all "equal" in value of checking, so it's not really easy to justify checking one but not the other), for every test.

You'd then have to do it all over again to try and figure out if something is "a loop", which is easier to to for some loops than others.

While generally that shouldn't grind everything to a halt performance wise, it's still very tied to the size of your test functions, and it's entirely possible for people to have mega sized tests, or even just a lot of them (looking at you Facebook!); this is relevant because IDEs re-run linting rules generally everytime you type a character, so that's a lot of work when you add it all up!

So overall it's a question of burden vs usefulness - I do think this rule could be really useful, and that's again why I'm not straight closing this issue out, but I'm wanting to make sure the rule will catch enough cases to be useful.

---

Overall the main problem I see here is not the loop detection, but detecting other expects, because afaik there's not a nice way to structure a rule to say to eslint "run me over everything that matches this selector, and then let me do some processing and decide what to do"; thus this rule will likely always require you to check the entire body of the function.

I am going to have a play around with a possible implementation for this rule, but I can't make promises.

[G-Rath]

Have implemented an option for native loops in #1013

[github-actions]

🎉 This issue has been resolved in version 25.4.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀