Skip to content
New issue

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

#

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

Already on GitHub? # 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
Loading

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

# for free to join this conversation on GitHub. Already have an account? # 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