refactor(expect-expect): report test callee instead of call expression

[remcohaszing]

When one starts writing a test, they often start with something like the following:

This triggers the jest/expect-expect rule for a good reason, but it’s annoying, confusing even, when the entire test block has a red underline until an expect statement is added.

This changes the rule to report the callee, only. This limits the scope of the reported code, so the red lines that do appear while typing are more meaningful, i.e. result is unused:

Refs #539
Refs #641

[G-Rath]

Please see #641 & related issues - the node we're reporting on is the right one for this rule, as the problem is with the callback argument, not the function body.

It's up to the editor on how it renders eslint results.

[remcohaszing]

I agree technically the function callback, not the identifier. However, I disagree it’s up to the editor how the ESLint results are highlighted.

ESLint rules report a text range, or a node which has a text range. I think it’s fair to say the editor should underline the text range with a red squirly line.

I believe it’s a bad UX if such a large piece of the code is underlined. Reporting identifier may not be the best solution. What about reporting the last character of the callback.

   it('should do something', () => {
     const result = something();
   });
// ^-- Report this character

   it('should do something', () =>
     something()
     //        ^-- Report this character, because the test has no closing body bracket
   );

This could be interpreted as: I got to the end of your test case, but still didn’t find an expectation.

Alternatively all code between the last statement and the closing bracket of the test could be reported

   it('should do something', () => {
     const result = something();
   });
// ^-- Report this character and all whitespace before it.

   it('should do something', () =>
     something()
   );
// ^-- Report all whitespace before this character.

[G-Rath]

You've just said you disagree that it's up to the editor, and then proceeded to provide possible solutions that are entirely based around assuming how editors render the results :)

Our UX scope does not extend to how editors render ranges from ESLint, since we can't control that at all.

While we can technically report "all whitespace before this character", we have no assurance on how that'll be rendered, and in some cases ESLint itself will overrule us in favor of more accurate ranges (i.e it'll latch on to the closest node).

IMO the UX you're describing of highlighting the last set of characters in a callback is a worse one, because both of the above and because it makes it harder to notice. This is in addition to the fact that our problem is not with the last character in the callback...

Also remember that ESLint output is not just for IDEs - take for example annotations in GIthub Actions:

it(
  'should do something',
  () => {
    //
  }
);

The above code is perfectly valid, and I've seen it in the wild - right now by reporting on the correct node, GHA will show its annotation on the callback which is where the error is.

I don't have a major problem with the change that is suggested in this case - my problem is that this is a UX problem with VSCode, not us. We already have a few other rules that are technically reporting in the wrong node that I've held off correcting because of their UX problem.

The role of an eslint-plugin is to report where errors are as accurately as possible, both in presence (i.e little to no false positives or negatives) and location. The role of an IDE integration is to present what we report in a manner that ideally has a good user experience.

[github-actions]

🎉 This issue has been resolved in version 27.4.3 🎉

The release is available on:

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

Your semantic-release bot 📦🚀