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

Jump to bottom

feat(require-typed-module-mocks): add rule to use strict module mock factory types #1314

Merged
merged 1 commit into from Dec 31, 2022
Merged

feat(require-typed-module-mocks): add rule to use strict module mock factory types #1314

merged 1 commit into from Dec 31, 2022

Conversation

NotWoods
Copy link
Contributor

Closes #1313

While the rule itself doesn't need type-checking, the auto-fix does. It it possible to disable the auto-fix based on the parser & source file? If not, the rule could be switched to use editor suggestions.

G-Rath
G-Rath reviewed Dec 24, 2022
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

Thanks, I'll do a proper review when I'm back at my laptop, but giving it a quick look over I've already got two main comments:

  1. You don't need type checking becuase you're not using the TypeScript service
  2. I feel like ideally we should have "factory" in the rule name, and maybe "generic" too, since this rule is pretty specific to those things - what do you think about no-untyped-mock-factory or require-generic-type-with-mock-factory?

@G-Rath
Copy link
Collaborator

G-Rath commented Dec 24, 2022

Also you need to use parseJestFn to check if a function is a call to the jest global/import - have a look at how some of the other rules are doing this, or otherwise I can give you some snippets when I'm back at my laptop :)

G-Rath
G-Rath requested changes Dec 24, 2022
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment •
edited

Choose a reason for hiding this comment

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

Looks like a good start, but need to address my comments so far + ensure CI is passing; let me know if you need any help :)

G-Rath
G-Rath requested changes Dec 27, 2022
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

Looks good - have a few tweaks I'd like to see, but they should be pretty straightforward.

src/rules/__tests__/no-untyped-mock-factory.test.ts Outdated Show resolved Hide resolved
src/rules/__tests__/no-untyped-mock-factory.test.ts Outdated Show resolved Hide resolved
src/rules/__tests__/no-untyped-mock-factory.test.ts Show resolved Hide resolved
src/rules/no-untyped-mock-factory.ts Outdated Show resolved Hide resolved
src/rules/no-untyped-mock-factory.ts Outdated Show resolved Hide resolved
src/rules/no-untyped-mock-factory.ts Outdated Show resolved Hide resolved
src/rules/no-untyped-mock-factory.ts Outdated Show resolved Hide resolved
src/rules/no-untyped-mock-factory.ts Outdated Show resolved Hide resolved
docs/rules/no-untyped-mock-factory.md Outdated Show resolved Hide resolved
docs/rules/no-untyped-mock-factory.md Show resolved Hide resolved
@NotWoods
Copy link
Contributor Author

Thank you for pointing me at some useful utilities! Updated the PR to address comments.

@NotWoods
Copy link
Contributor Author

Added an extra test to check for spots where the rule shouldn't match, in order to reach full code coverage.

G-Rath
G-Rath reviewed Dec 30, 2022
View reviewed changes
src/rules/no-untyped-mock-factory.ts
} from './utils';

const findModuleName = (
node: TSESTree.Literal | TSESTree.Node,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: iirc TSESTree.Literal is a subset of TSESTree.Node, so you shouldn't need it here

G-Rath
G-Rath approved these changes Dec 30, 2022
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

Looks good, just need to re-generate the docs :)

@G-Rath G-Rath merged commit ee43c3f into jest-community:main Dec 31, 2022
github-actions bot pushed a commit that referenced this pull request Dec 31, 2022
@semantic-release-bot
chore(release): 27.2.0 [skip ci]
b00b9b6 
# [27.2.0](v27.1.7...v27.2.0) (2022-12-31)

### Features

* create `require-typed-module-mocks` rule ([#1314](#1314)) ([ee43c3f](ee43c3f))
@github-actions
Copy link

🎉 This PR is included in version 27.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@NotWoods NotWoods deleted the required-typed-module-mocks branch January 1, 2023 04:29
@Blfrg
Copy link

Blfrg commented Jan 1, 2023

Thank you, @NotWoods, for this PR.

To clarify for anybody looking for this rule, it was ultimately named no-untyped-mock-factory.
The release notes and changelog picked up the original working name.

@G-Rath G-Rath mentioned this pull request Jan 1, 2023
Merged
@G-Rath
Copy link
Collaborator

G-Rath commented Jan 1, 2023

@Blfrg thanks for flagging - I've updated the release notes and have opened #1317 updating the changelog

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

@G-Rath G-Rath G-Rath approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[new rule] Require that TypeScript tests use a typed jest.mock factory
3 participants
@NotWoods @G-Rath @Blfrg