Skip to content

feat(eslint-plugin-jest): add prefer-to-have-been-called rule#1019

Open
eryue0220 wants to merge 3 commits into
web-infra-dev:mainfrom
eryue0220:feat/jest-prefer-to-have-been-called
Open

feat(eslint-plugin-jest): add prefer-to-have-been-called rule#1019
eryue0220 wants to merge 3 commits into
web-infra-dev:mainfrom
eryue0220:feat/jest-prefer-to-have-been-called

Conversation

@eryue0220
Copy link
Copy Markdown
Contributor

Summary

  • Port prefer-to-have-been-called from eslint-plugin-jest to rslint.

Related Links

Tracking issue: #476
eslint-plugin-jest/prefer-to-have-been-called doc code

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the jest/prefer-to-have-been-called rule and refactors existing Jest rules to use new shared utility functions for type assertion unwrapping and member access. Feedback on the new rule suggests consolidating the fix logic into a single range replacement to improve robustness and ensure alignment with ESLint semantics regarding edge cases like comments or trailing commas.

Comment on lines +79 to +86
rule.RuleFixReplaceRange(
core.NewTextRange(matcherCall.Arguments.Pos(), matcherCall.Arguments.End()),
"",
),
rule.RuleFixReplaceRange(
core.NewTextRange(replaceStart, matcherParent.End()),
replacementMatcher,
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation uses two separate fixes to replace the matcher name and clear the arguments. This can be simplified into a single fix that replaces the entire range from the receiver's end to the end of the call expression. This approach is more robust as it correctly handles potential edge cases like trailing commas or comments within the parentheses, ensuring exact alignment with ESLint's semantics for this rule.

Suggested change
rule.RuleFixReplaceRange(
core.NewTextRange(matcherCall.Arguments.Pos(), matcherCall.Arguments.End()),
"",
),
rule.RuleFixReplaceRange(
core.NewTextRange(replaceStart, matcherParent.End()),
replacementMatcher,
),
rule.RuleFixReplaceRange(
core.NewTextRange(replaceStart, node.End()),
replacementMatcher+"()",
),
References
  1. For linter rules ported from or inspired by ESLint, maintain exact alignment with ESLint's semantics, even for edge cases where a simpler implementation might seem practically equivalent.

}

func isZeroLiteral(node *ast.Node) bool {
node = jestUtils.UnwrapTypeAssertions(node)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit worth fixing: this should be UnwrapBasicTypeAssertions. Upstream's getFirstMatcherArgfollowTypeAssertionChain only unwraps as and <T> assertions — not ! or satisfies — so these stay unflagged upstream but get reported (and autofixed) here:

expect(method).toBeCalledTimes(0!);
expect(method).toBeCalledTimes(0 satisfies number);

prefer_to_be uses the same upstream helper and already calls UnwrapBasicTypeAssertions, so this keeps the two rules consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants