feat: add prefer-query-matchers rule

[CodingItWrong]

Checks

• I have read the contributing guidelines.

Changes

• Adds prefer-query-matchers rule, with tests and docs

Context

Fixes #728

[CodingItWrong]

HT @obsoke who paired with me on all this work.

@Belco90 if you get a moment, could you review what we've written so far to see if it seems to be on the right track? We appreciate it!

[CodingItWrong]

@Belco90 Ready for review!

As we worked on this, we thought of another options schema idea. With the current API, there is some repetition if you have a lot of configured entries:

{
  "validEntries": [
    {"matcher": "toBeA", "query": "get"},
    {"matcher": "toBeB", "query": "query"},
    {"matcher": "toBeC", "query": "get"},
    {"matcher": "toBeD", "query": "query"},
    {"matcher": "toBeE", "query": "get"},
    {"matcher": "toBeF", "query": "query"},
  ]
}

It might reduce repetition in the options if we group the get and query matchers:

{
  "getMatchers": ["toBeA", "toBeC", "toBeE"],
  "queryMatchers": ["toBeB", "toBeD", "toBeF"]
}

Please let us know if you'd like us to make that change; it would be quick.

[Belco90]

@CodingItWrong makes sense! I'll try to review it by the end of the month.

[CodingItWrong]

Thanks @Belco90!

@MichaelDeBoey was changing this PR to draft intentional? If so, could you provide some rationale? In my understanding it's complete and ready to review and merge.

EDIT: I just noticed that the PR description was out of date, and still referred to a draft and work remaining to be done. I've now edited the description to correctly indicate that that work is done.

[obsoke]

Hello! I worked on this rule with @CodingItWrong.

We were wondering if it made sense for this rule to come with a default configuration. This default would ensure that the matcher .toBeVisible was attached to a get query. After all, this was the motivation (see #728) for creating this rule in the first place. I know I've personally made the mistake of pairing a query* query with toBeVisible before. An ESLint rule letting me know what I did wrong would have been helpful.

[CodingItWrong]

@MichaelDeBoey I've moved this PR back out of draft as my intention is that it's ready for review, and you haven't given a reason it should be in draft instead. If you do think it needs to be a draft, please provide a reason—thank you.

[Belco90]

@CodingItWrong thanks for your patience! I'm slowly catching up, so I hope I can review it soon.

[CodingItWrong]

@Belco90 No worries! I just fixed the style issue that CI was reporting. I'll keep an eye on any other failures to address them.

[CodingItWrong]

@Belco90 BTW we see that the branch is out-of-date with main. We are thinking we will wait until review to update it, but if you need us to do that in advance please let us know

Is there an easy way to pre-approve CI to rerun after any fix I push up? If so that would allow us to resolve issues in advance

[Belco90]

@Belco90 BTW we see that the branch is out-of-date with main. We are thinking we will wait until review to update it, but if you need us to do that in advance please let us know

Is there an easy way to pre-approve CI to rerun after any fix I push up? If so that would allow us to resolve issues in advance

I'm afraid there isn't! Any way of automating it would lead to the same issue that restriction prevents.

[Belco90]

It was really easy to follow the implementation of the rule, great job! I've seen you used a couple of existing helpers, I hope that made working with the codebase easier.

I just asked for a change to improve the code scenarios of the tests, everything else LGTM. It would be nice to also catch asserts with elements queried before the assertion, but it's fine for this first implementation.

[Belco90]

LGTM! Thank you both for your contribution!

[Belco90]

It might reduce repetition in the options if we group the get and query matchers:

{
  "getMatchers": ["toBeA", "toBeC", "toBeE"],
  "queryMatchers": ["toBeB", "toBeD", "toBeF"]
}

Please let us know if you'd like us to make that change; it would be quick.

I think I forgot to answer to this, I just realized now… I think for the first implementation of the rule is fine like the current one (although it can cause tons of entries in the options). Still, we can implement this alternative way of configuring it living together with the current implementation.

[github-actions]

🎉 This PR is included in version 5.11.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀

[Belco90]

@all-contributors
please add @CodingItWrong for code, test, docs, and ideas

[CodingItWrong]

@Belco90 would you mind adding @obsoke for test and docs as well? i may not have reflected in the commit attribution correctly, but we wrote all three of those while actively pairing

[Belco90]

@all-contributors please add @obsoke for code, test, and docs

[allcontributors]

@Belco90

I've put up a pull request to add @CodingItWrong! 🎉

I've put up a pull request to add @obsoke! 🎉

[allcontributors]

@Belco90

I've updated the pull request to add @obsoke! 🎉

[Belco90]

@all-contributors please add @obsoke for code, test, and docs

[allcontributors]

@Belco90

I've put up a pull request to add @obsoke! 🎉

[github-actions]

🎉 This PR is included in version 6.0.0-alpha.15 🎉

The release is available on:

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

Your semantic-release bot 📦🚀