feat(prefer-jest-mocked): add new rule #1599

MillerSvt · 2024-05-30T14:24:33Z

Resolves #1470

MillerSvt · 2024-05-30T16:05:38Z

error - 2024-05-30 16:03:56,619 -- Commit creating failed: {"detail":"Tokenless has reached GitHub rate limit. Please upload using a token: https://docs.codecov.com/docs/adding-the-codecov-token. Expected available in 2006 seconds."}
Error: Codecov: Failed to properly create commit: The process '/home/runner/work/_actions/codecov/codecov-action/v4/dist/codecov' failed with exit code 1

It looks like CI is broken.

G-Rath

thanks, looks like a good start

in addition to my specific comments:

some of your tests have whitespace on their "empty" lines; would be great if you could remove that
we should have a couple of cases that cover chained assertions and with jest in the middle (e.g. as unknown as string as unknown as jest.MockedFunction and as unknown as jest.MockedFunction as string as jest.MockedFunction - it's silly but legal)
we should support angle bracket assertions too

G-Rath · 2024-05-30T19:14:10Z

src/rules/__tests__/prefer-mocked.test.ts

+function getFixturesRootDir(): string {
+  return path.join(__dirname, 'fixtures');
+}
+
+const rootPath = getFixturesRootDir();
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+  parserOptions: {
+    sourceType: 'module',
+    tsconfigRootDir: rootPath,
+    project: './tsconfig.json',
+  },
+});


you shouldn't need project and co since we're not using type information

Suggested change

function getFixturesRootDir(): string {

return path.join(__dirname, 'fixtures');

}

const rootPath = getFixturesRootDir();

const ruleTester = new RuleTester({

parser: require.resolve('@typescript-eslint/parser'),

parserOptions: {

sourceType: 'module',

tsconfigRootDir: rootPath,

project: './tsconfig.json',

},

});

const ruleTester = new RuleTester({

parser: require.resolve('@typescript-eslint/parser'),

});

G-Rath · 2024-05-30T19:21:07Z

src/rules/prefer-mocked.ts

+  defaultOptions: [],
+  create(context) {
+    return {
+      'TSAsExpression:has(TSTypeReference > TSQualifiedName:has(Identifier.left[name="jest"]):has(Identifier.right[name="Mock"],Identifier.right[name="MockedFunction"]))'(


Please don't use complex expressions like this - they're brittle and harder to maintain (in part because TypeScript has no idea what they're doing)

G-Rath · 2024-05-30T19:22:16Z

src/rules/prefer-mocked.ts

+      'TSAsExpression:has(TSTypeReference > TSQualifiedName:has(Identifier.left[name="jest"]):has(Identifier.right[name="Mock"],Identifier.right[name="MockedFunction"]))'(
+        node: ValidatedTsAsExpression,
+      ) {
+        const fnName = getFnName(node.expression, context.getSourceCode().text);


Can you see if we can reuse our util here? - I expect you probably can by checking if the parent is an assertion node

G-Rath

Looks good, just got a few last tweaks - could you also add a test case or two covering multiline shenanigans?

i.e.

(
  new Array(100)
    .fill(undefined)
    .map(x => x.value)
    .filter(v => !!v).myProperty as jest.MockedFunction<{
    method: () => void;
  }>
).mockReturnValue(1);

Also could you check in a test that uses square brackets, just for coverage? i.e. (Obj['foo'] as jest.MockedFunction).mockReturnValue(1);

G-Rath · 2024-06-02T19:07:29Z

src/rules/__tests__/prefer-mocked.test.ts

+  parser: require.resolve('@typescript-eslint/parser'),
+});
+
+ruleTester.run('prefer-mocked', rule, {


nit: generally I'd prefer to not use dedent for single line calls, and not have a newline between cases

G-Rath · 2024-06-02T19:08:11Z

src/rules/prefer-mocked.ts

+  name: __filename,
+  meta: {
+    docs: {
+      description: 'Prefer jest.mocked() over (fn as jest.Mock)',


Suggested change

description: 'Prefer jest.mocked() over (fn as jest.Mock)',

description: 'Prefer `jest.mocked()` over `fn as jest.Mock`',

G-Rath · 2024-06-02T19:08:22Z

src/rules/prefer-mocked.ts

+      description: 'Prefer jest.mocked() over (fn as jest.Mock)',
+    },
+    messages: {
+      useJestMocked: 'Prefer jest.mocked({{ replacement }})',


Suggested change

useJestMocked: 'Prefer jest.mocked({{ replacement }})',

useJestMocked: 'Prefer `jest.mocked({{ replacement }})`',

G-Rath · 2024-06-02T19:09:00Z

src/rules/__tests__/prefer-mocked.test.ts

+      (foo as Mock.jest).mockReturnValue(1);
+    `,
+  ],
+  invalid: [


can you add the expected data values for the message in each error?

G-Rath · 2024-06-02T19:15:41Z

src/rules/prefer-mocked.ts

+        .text.slice(...followTypeAssertionChain(node.expression).range);
+
+      context.report({
+        node: node,


Suggested change

node: node,

node,

G-Rath · 2024-06-02T19:17:56Z

src/rules/prefer-mocked.ts

+        node: node,
+        messageId: 'useJestMocked',
+        data: {
+          replacement: '',


either we shouldn't have this or it should have a proper value (this'll get picked up by adding it to the tests)

I think it will be better to remove this parameter, because multi-line code will affect the output of errors

G-Rath · 2024-06-02T19:18:28Z

src/rules/prefer-mocked.ts

+      }
+
+      const fnName = context
+        .getSourceCode()


please use our getSourceCode helper here, as getSourceCode is removed in ESLint v9

G-Rath · 2024-06-02T19:23:01Z

src/rules/prefer-mocked.ts

nit: I wonder if it might be slightly better to call this prefer-jest-mocked? 🤔

G-Rath

thanks!

# [28.6.0](v28.5.0...v28.6.0) (2024-06-06) ### Features * **prefer-jest-mocked:** add new rule ([#1599](#1599)) ([4b6a4f2](4b6a4f2)) * **valid-expect:** supporting automatically fixing adding async in some cases ([#1579](#1579)) ([5b9b47e](5b9b47e))

github-actions · 2024-06-06T19:40:30Z

🎉 This PR is included in version 28.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

MillerSvt force-pushed the main branch from fc3801d to 74cd4ae Compare May 30, 2024 14:37

MillerSvt changed the title ~~feat(prefer-mocked): add new rule (#1470)~~ feat(prefer-mocked): add new rule May 30, 2024

MillerSvt force-pushed the main branch from 74cd4ae to f132161 Compare May 30, 2024 15:27

SimenB requested a review from G-Rath May 30, 2024 15:30

MillerSvt force-pushed the main branch from f132161 to 9195234 Compare May 30, 2024 16:02

G-Rath requested changes May 30, 2024

View reviewed changes

MillerSvt force-pushed the main branch from 9195234 to 81f455f Compare May 31, 2024 04:26

MillerSvt requested a review from G-Rath May 31, 2024 04:43

MillerSvt force-pushed the main branch 2 times, most recently from a4c08ed to f381f58 Compare May 31, 2024 05:17

G-Rath requested changes Jun 2, 2024

View reviewed changes

feat(prefer-mocked): add new rule (jest-community#1470)

ffe57e0

MillerSvt force-pushed the main branch from f381f58 to ffe57e0 Compare June 3, 2024 05:14

MillerSvt requested a review from G-Rath June 3, 2024 05:14

Merge branch 'main' into main

fc383e8

G-Rath added the new rule label Jun 6, 2024

G-Rath changed the title ~~feat(prefer-mocked): add new rule~~ feat(prefer-jest-mocked): add new rule Jun 6, 2024

G-Rath approved these changes Jun 6, 2024

View reviewed changes

G-Rath merged commit 4b6a4f2 into jest-community:main Jun 6, 2024
38 checks passed

github-actions bot added the released label Jun 6, 2024

qdraw mentioned this pull request Aug 25, 2024

[Snyk] Upgrade eslint-plugin-jest from 28.5.0 to 28.7.0 qdraw/starsky#1672

Merged

MelvinBot mentioned this pull request Aug 25, 2024

[Snyk] Upgrade eslint-plugin-jest from 28.6.0 to 28.7.0 Expensify/expensify-common#792

Closed

This was referenced Sep 2, 2024

[Snyk] Upgrade eslint-plugin-jest from 27.9.0 to 28.8.0 Sec32fun32/setup-dotnet#11

Open

[Snyk] Upgrade eslint-plugin-jest from 28.5.0 to 28.8.0 Sec32fun32/checkout#8

Open

MelvinBot mentioned this pull request Sep 21, 2024

[Snyk] Upgrade eslint-plugin-jest from 28.6.0 to 28.8.1 Expensify/expensify-common#802

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(prefer-jest-mocked): add new rule #1599

feat(prefer-jest-mocked): add new rule #1599

MillerSvt commented May 30, 2024 •

edited by G-Rath

Loading

MillerSvt commented May 30, 2024

G-Rath left a comment

G-Rath May 30, 2024

G-Rath May 30, 2024

G-Rath May 30, 2024

G-Rath left a comment

G-Rath Jun 2, 2024

G-Rath Jun 2, 2024

G-Rath Jun 2, 2024

G-Rath Jun 2, 2024

G-Rath Jun 2, 2024

G-Rath Jun 2, 2024

MillerSvt Jun 3, 2024

G-Rath Jun 2, 2024

G-Rath Jun 2, 2024

G-Rath left a comment

github-actions bot commented Jun 6, 2024

	description: 'Prefer jest.mocked() over (fn as jest.Mock)',
	description: 'Prefer `jest.mocked()` over `fn as jest.Mock`',

	useJestMocked: 'Prefer jest.mocked({{ replacement }})',
	useJestMocked: 'Prefer `jest.mocked({{ replacement }})`',

feat(prefer-jest-mocked): add new rule #1599

feat(prefer-jest-mocked): add new rule #1599

Conversation

MillerSvt commented May 30, 2024 • edited by G-Rath Loading

MillerSvt commented May 30, 2024

G-Rath left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

G-Rath left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

G-Rath left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 6, 2024

MillerSvt commented May 30, 2024 •

edited by G-Rath

Loading