feat: create no-restricted-matchers rule

[G-Rath]

This rule lets us deprecate & drop at least the following:

• no-truthy-falsy
• no-expect-resolves
• prefer-inline-snapshots

In theory we could drop a few more, but I think they have their own context that makes them worth keeping:

• prefer-called-with: I've not come up with a nice syntax that lets you ban a matcher unless it has a modifier, so you could ban toHaveBeenCalled but that would also ban not.toHaveBeenCalled.
• prefer-strict-equals: while this can be handled by ban-matchers, I feel like this is one we want to recommend (so having its own rule gives it a name + specific documentation), and provide an autofix/suggestion for (more on this below)
• no-alias-methods: again as with the case of prefer-strict-equals, this has a fixer, and a history behind it.

The case for syntax

This rule lets you ban *.modifier.matcher & *.matcher but not expect.matcher. The ability to ban just modifier.matcher comes naturally, but I couldn't come up with a nice way of letting you ban only a matcher without any modifiers; in saying that I don't think there's a lot of gain in being able to do that.

The case for naming

While this rule is called ban-matchers, it does let you ban modifiers as well, and in the code I've referred to the actual things that are banned as "chains".

I've named the rule ban-matchers as "matchers" I feel is a commonly understood and used name, and so unlikely to cause confusion - it also has the desired effect of gelling with its sister @typescript-eslint rule ban-types

(In saying that I realise the Expect page does refer to them all under the "methods" headers, and we do have no-alias-methods, so maybe "methods" is the better one to use - the more I type the more I think maybe thats the better name, except its a bit generic for what it's implying. "ban-expect-methods" maybe? 🤔)

At the very least, I struggled to come up with a more catch-all term for not, resolves, & rejects than "modifier", which I don't think it used a lot, and so felt it would just be overly confusing to try and pick an exact name.

The case for no-alias-methods

Having looked over this rule, I think @SimenB a push should be made to deprecate & later remove the alias matchers from jest core.

Here's my thinking:

• we include the rule in the style ruleset, which while not the same as being in the recommended rule set, still counts for something in my eyes i.e we don't have no-truthy-falsy or prefer-called-with - both rules that I personally think are Good, and help strengthen your tests, but didn't make the cut that no-alias-methods did (in saying that, maybe they should be in the style ruleset?)
• the rule itself uses "the canonical name" in both it's description & documentation, indicating the prefered names.
• jest itself is looking to slim down; while there aliases will be a small drop in the ocean, they're a drop nonetheless, and one that I believe can actually still be offered by use of expect.extend in a start script to restore the aliased matchers - taking that one further, you could publish it as a package on npm; jest could even do this as @jest/expect-aliases (or something better named - actually better yet they can be brought into jest-extended).
• finally, imo the canonical name is more grammatically correct (in the situations I've used & can think of - there might be edge cases I've not considered?); while it can be argued they're shorted, I don't think sacrificing readability (especially when you consider non-native speakers) for the sake of saving a few characters in your test files to be all that worth it.

I don't know the history behind the aliases, and happy to be schooled on them, but I know that jest has rejected a fair few matchers in the past to keep things lean and only include the batteries it needs.

This is conversation that definitely happen over at jest, and happy to port it over, but wanted to raise it here as I felt this rule + the addition of no-deprecated-functions provided a good launching point for kicking off this discussion.

Going down this path would mean folding no-alias-methods into no-deprecated-functions.

The case for options

Currently we use the second options parameter as a map for the banned matchers, which takes either null or a string to be used as a custom message.

This means if we ever want to add options, we'd have to do a deprecation & migration.
This also means that if we wanted to recommend ban-matchers, it would be slightly more annoying for people to extend off.

As such, we could go the same vein as @typescript-eslint/ban-types:

We have a matchers property that takes the map, and then can add additional properties i.e extendDefaults (with the "defaults" being any bans we recommend).

While I don't have any bans to recommend off the top of my head, this would also provide a way that we could collapse no-alias-methods into this rule, by having a banAliasMethods option.

The case for fixing

@typescript-eslint/ban-types supports a fixWith option which it uses for providing its fixer.

I think having this is less important for us as jest has a far smaller number of matchers, and an even smaller number of matchers that can be swapped out without being a little dangerous.

In saying that, that's a good reason to look to use the suggestion api to provide a quick-fix.

We could take an object instead of a string for the value of the key-value pair:

interface BannedMatcher {
  message?: string | null;
  suggestedReplacement?: string;
}

This would let us fold in prefer-strict-equal into `ban-matchers (tho we'd still weaken the documentation).

This would increase the rules complexity, but still be relatively easy to do.

closes #330
closes #545
closes #312
closes #234
closes #230

[SimenB]

no-export-resolves

🧐

[SimenB]

Should we go for no-banned-matchers instead of ban-matchers? I feel like that's closer to most eslint rule name

[SimenB]

This looks greta! Only nit I have is the name of the rule.

I think we should deprecated the other rules you mention, but can do that in a follow-up for a cleaner changelog 🙂

[G-Rath]

Should we go for no-banned-matchers instead of ban-matchers? I feel like that's closer to most eslint rule name

Yeah that's fair - for some reason I thought that eslint had some ban-* rules.

[SimenB]

Remember to verify name of rule in commit when squashing

[G-Rath]

Before I merge, do you have any thoughts about the using options for a banAliasMethods options, or suggesting replacements?

I'm just conscious about changing the syntax quickly after merging and requiring a migration until the next (next) major.

[SimenB]

Hmm, rename to no-restricted-matchers? Matches https://eslint.org/docs/rules/no-restricted-globals

And I think the syntax in that rule is a good starting point. For replacements, maybe something like https://github.com/sindresorhus/eslint-plugin-unicorn/blob/master/docs/rules/prevent-abbreviations.md?

[G-Rath]

Ohh I like that implementation; but do you think it's worth implementing that now, or waiting to see if it's asked for?

[SimenB]

I'm fine with waiting

[github-actions]

🎉 This PR is included in version 23.11.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀