Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix prefer-to-have-count rule for expect arguments that need dereferencing #292

Merged
merged 5 commits into from
May 17, 2024

Conversation

An631
Copy link
Contributor

@An631 An631 commented May 16, 2024

ISSUE #293:
The current prefer-to-have-count rule has a problem where any expect checks that take a variable argument will not check to make sure the value is not originating from a .count call.

For example the following code would not be flagged as an error:

   const filesCount = await files.count();
   expect(filesCount).toBe(2);

The current code misses this case because we are not dereferencing the last assigned value of filesCount at the moment of verifying the expect argument. The rule should flag the error and return the following as a fix:

   const filesCount = files;
   await expect(filesCount).toHaveCount(2);

SOLUTION:

This PR updates the prefer-to-have-count code to re-use the dereference method previously used by the prefer-web-first-assertion rule. To make this method available to other files I moved it, along with all its related methods, to the ast.ts file which looked to me like the best place to centralize it (curious to know if there is a better place for these methods though).

I have added multiple new tests to check for other edge cases like variable re-assignment.

@mskelton mskelton merged commit d894de6 into playwright-community:main May 17, 2024
3 checks passed
@An631 An631 deleted the FixPreferToHaveCountRule branch May 17, 2024 17:21
Copy link

🎉 This PR is included in version 1.6.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants