Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/rules/__tests__/unbound-method.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ const ConsoleClassAndVariableCode = dedent`
const console = new Console();
`;

const ServiceClassAndMethodCode = dedent`
class Service {
method() {}
}

const service = new Service();
`;

const toThrowMatchers = [
'toThrow',
'toThrowError',
Expand All @@ -73,6 +81,19 @@ const validTestCases: string[] = [
'expect(() => Promise.resolve().then(console.log)).not.toThrow();',
...toThrowMatchers.map(matcher => `expect(console.log).not.${matcher}();`),
...toThrowMatchers.map(matcher => `expect(console.log).${matcher}();`),
// https://github.com/jest-community/eslint-plugin-jest/issues/1800
...[
'const parameter = jest.mocked(service.method).mock.calls[0][0];',
'const calls = jest.mocked(service.method).mock.calls;',
'const lastCall = jest.mocked(service.method).mock.calls[0];',
'const mockedMethod = jest.mocked(service.method); const parameter = mockedMethod.mock.calls[0][0];',

'jest.mocked(service.method).mock;',

'const mockProp = jest.mocked(service.method).mock;',
'const result = jest.mocked(service.method, true);',
'jest.mocked(service.method, { shallow: true });',
].map(code => [ServiceClassAndMethodCode, code].join('\n')),
];

const invalidTestCases: Array<TSESLint.InvalidTestCase<MessageIds, Options>> = [
Expand Down Expand Up @@ -128,6 +149,52 @@ const invalidTestCases: Array<TSESLint.InvalidTestCase<MessageIds, Options>> = [
},
],
})),
// Ensure that accessing .mock on non-jest.mocked() results still reports errors
// Note: These cases might not report errors if the base rule doesn't consider
// property access as unbound method access, so we'll remove them for now
// and focus on cases that should definitely report errors
// Ensure that service.method as non-argument still reports errors
{
code: dedent`
${ServiceClassAndMethodCode}

const method = service.method;
jest.mocked(method);
`,
errors: [
{
line: 7,
messageId: 'unboundWithoutThisAnnotation',
},
],
},
// Ensure that regular unbound method access still reports errors
{
code: dedent`
${ServiceClassAndMethodCode}

const method = service.method;
`,
errors: [
{
line: 7,
messageId: 'unboundWithoutThisAnnotation',
},
],
},
{
code: dedent`
${ServiceClassAndMethodCode}

Promise.resolve().then(service.method);
`,
errors: [
{
line: 7,
messageId: 'unboundWithoutThisAnnotation',
},
],
},
// toThrow matchers call the expected value (which is expected to be a function)
...toThrowMatchers.map(matcher => ({
code: dedent`
Expand Down Expand Up @@ -235,6 +302,7 @@ const arith = {
${code}
`;
}

function addContainsMethodsClassInvalid(
code: string[],
): Array<TSESLint.InvalidTestCase<MessageIds, Options>> {
Expand Down
50 changes: 50 additions & 0 deletions src/rules/unbound-method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
findTopMostCallExpression,
getAccessorValue,
isIdentifier,
isSupportedAccessor,
parseJestFnCall,
} from './utils';

Expand Down Expand Up @@ -73,9 +74,58 @@ export default createRule<Options, MessageIds>({
return {};
}

/**
* Checks if a MemberExpression is an argument to a `jest.mocked()` call.
* This handles cases like `jest.mocked(service.method)` where `service.method`
* should not be flagged as an unbound method.
*/
const isArgumentToJestMocked = (
node: TSESTree.MemberExpression,
): boolean => {
// Check if the immediate parent is a CallExpression
if (node.parent?.type !== AST_NODE_TYPES.CallExpression) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is the opposite of the check we do later on, so ideally we can merge the two but that might be messy due to the optional chaining and require an else which I try to avoid so not too fussed to keep it like this for now

return false;
}

const parentCall = node.parent;

// Check if the call is jest.mocked() by examining the callee
if (
parentCall.callee.type === AST_NODE_TYPES.MemberExpression &&
isSupportedAccessor(parentCall.callee.object) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

isSupportedAccessor takes an optional value as its second param for just these situations - so rather than having to do the work twice, you can just do isSupportedAccessor(..., 'jest')

that in turn means you should be able to refactor this all to be a return

isSupportedAccessor(parentCall.callee.property)
) {
const objectName = getAccessorValue(parentCall.callee.object);
const propertyName = getAccessorValue(parentCall.callee.property);

if (objectName === 'jest' && propertyName === 'mocked') {
return true;
}
}

return false;

// Also try using parseJestFnCall as a fallback
// const jestFnCall = parseJestFnCall(
// findTopMostCallExpression(parentCall),
// context,
// );

// return (
// jestFnCall?.type === 'jest' &&
// jestFnCall.members.length >= 1 &&
// isIdentifier(jestFnCall.members[0], 'mocked')
// );
Comment on lines +107 to +118
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Also try using parseJestFnCall as a fallback
// const jestFnCall = parseJestFnCall(
// findTopMostCallExpression(parentCall),
// context,
// );
// return (
// jestFnCall?.type === 'jest' &&
// jestFnCall.members.length >= 1 &&
// isIdentifier(jestFnCall.members[0], 'mocked')
// );

};

return {
...baseSelectors,
MemberExpression(node: TSESTree.MemberExpression): void {
// Check if this MemberExpression is an argument to jest.mocked()
if (isArgumentToJestMocked(node)) {
return;
}

if (node.parent?.type === AST_NODE_TYPES.CallExpression) {
const jestFnCall = parseJestFnCall(
findTopMostCallExpression(node.parent),
Expand Down