From cc96b7fa09eb0ae5ea1169e150927c81b0c963c4 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 1 Dec 2025 10:57:17 -0500 Subject: [PATCH 1/5] fix(eslint-plugin): [method-signature-style] ignore methods that return `this` --- .../src/rules/method-signature-style.ts | 18 +++++- packages/eslint-plugin/src/util/astUtils.ts | 56 +++++++++++++++++++ .../rules/method-signature-style.test.ts | 20 +++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/method-signature-style.ts b/packages/eslint-plugin/src/rules/method-signature-style.ts index c4e76b0593a4..cfea8e594502 100644 --- a/packages/eslint-plugin/src/rules/method-signature-style.ts +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -4,6 +4,7 @@ import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { createRule, + forEachChildESTree, isClosingParenToken, isCommaToken, isOpeningParenToken, @@ -127,7 +128,10 @@ export default createRule({ return { ...(mode === 'property' && { TSMethodSignature(methodNode): void { - if (methodNode.kind !== 'method') { + if ( + methodNode.kind !== 'method' || + returnTypeReferencesThisType(methodNode.returnType) + ) { return; } @@ -243,3 +247,15 @@ export default createRule({ }; }, }); + +function returnTypeReferencesThisType( + node: TSESTree.TSTypeAnnotation | undefined, +) { + return ( + node && + forEachChildESTree( + node.typeAnnotation, + child => child.type === AST_NODE_TYPES.TSThisType, + ) + ); +} diff --git a/packages/eslint-plugin/src/util/astUtils.ts b/packages/eslint-plugin/src/util/astUtils.ts index 1d8500052634..10c9b75e9947 100644 --- a/packages/eslint-plugin/src/util/astUtils.ts +++ b/packages/eslint-plugin/src/util/astUtils.ts @@ -1,5 +1,6 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import { visitorKeys } from '@typescript-eslint/visitor-keys'; import * as ts from 'typescript'; import { escapeRegExp } from './escapeRegExp'; @@ -79,3 +80,58 @@ export function forEachReturnStatement( return undefined; } } + +function isESTreeNodeLike(node: unknown): node is TSESTree.Node { + return ( + typeof node === 'object' && + node != null && + 'type' in node && + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + typeof (node as any).type === 'string' + ); +} + +/** + * Rough equivalent to ts.forEachChild for ESTree nodes. + * It returns the first truthy value returned by the callback, if any. + */ +export function forEachChildESTree( + node: TSESTree.Node, + callback: (child: TSESTree.Node) => false | Result | null | undefined, +): Result | undefined { + function visit(currentNode: TSESTree.Node): Result | undefined { + const result = callback(currentNode); + if (result) { + return result; + } + + const currentKeys = visitorKeys[currentNode.type]; + if (!currentKeys) { + return undefined; + } + + for (const key of currentKeys) { + const currentProperty = currentNode[key as keyof typeof currentNode]; + + if (Array.isArray(currentProperty)) { + for (const child of currentProperty) { + if (isESTreeNodeLike(child)) { + const result = visit(child); + if (result) { + return result; + } + } + } + } else if (isESTreeNodeLike(currentProperty)) { + const result = visit(currentProperty); + if (result) { + return result; + } + } + } + + return undefined; + } + + return visit(node); +} diff --git a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts index 9a7ab2b345c2..0e7a85421b72 100644 --- a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -39,6 +39,26 @@ interface Test { ` interface Test { set f(value: number): void; +} + `, + ` +interface Test { + f(value: number): this; +} + `, + ` +interface Test { + f(value: number): this | undefined; +} + `, + ` +interface Test { + f(value: number): Promise; +} + `, + ` +interface Test { + f(value: number): Promise; } `, 'type Test = { readonly f: (a: string) => number };', From 48fc9a99a4a90bf4883dc6a41c42294c3f2e5a1d Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 1 Dec 2025 11:18:18 -0500 Subject: [PATCH 2/5] Switch to just disabling the fixer --- .../src/rules/method-signature-style.ts | 92 ++++++++++--------- .../rules/method-signature-style.test.ts | 72 +++++++++++---- 2 files changed, 99 insertions(+), 65 deletions(-) diff --git a/packages/eslint-plugin/src/rules/method-signature-style.ts b/packages/eslint-plugin/src/rules/method-signature-style.ts index cfea8e594502..d0533da10da6 100644 --- a/packages/eslint-plugin/src/rules/method-signature-style.ts +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -128,13 +128,11 @@ export default createRule({ return { ...(mode === 'property' && { TSMethodSignature(methodNode): void { - if ( - methodNode.kind !== 'method' || - returnTypeReferencesThisType(methodNode.returnType) - ) { + if (methodNode.kind !== 'method') { return; } + const canFix = !returnTypeReferencesThisType(methodNode.returnType); const parent = methodNode.parent; const members = parent.type === AST_NODE_TYPES.TSInterfaceBody @@ -160,39 +158,41 @@ export default createRule({ context.report({ node: methodNode, messageId: 'errorMethod', - *fix(fixer) { - const methodNodes = [ - methodNode, - ...duplicatedKeyMethodNodes, - ].sort((a, b) => (a.range[0] < b.range[0] ? -1 : 1)); - const typeString = methodNodes - .map(node => { - const params = getMethodParams(node); - const returnType = getMethodReturnType(node); - return `(${params} => ${returnType})`; - }) - .join(' & '); - const key = getMethodKey(methodNode); - const delimiter = getDelimiter(methodNode); - yield fixer.replaceText( - methodNode, - `${key}: ${typeString}${delimiter}`, - ); - for (const node of duplicatedKeyMethodNodes) { - const lastToken = context.sourceCode.getLastToken(node); - if (lastToken) { - const nextToken = - context.sourceCode.getTokenAfter(lastToken); - if (nextToken) { - yield fixer.remove(node); - yield fixer.replaceTextRange( - [lastToken.range[1], nextToken.range[0]], - '', - ); + fix: + canFix && + function* fix(fixer) { + const methodNodes = [ + methodNode, + ...duplicatedKeyMethodNodes, + ].sort((a, b) => (a.range[0] < b.range[0] ? -1 : 1)); + const typeString = methodNodes + .map(node => { + const params = getMethodParams(node); + const returnType = getMethodReturnType(node); + return `(${params} => ${returnType})`; + }) + .join(' & '); + const key = getMethodKey(methodNode); + const delimiter = getDelimiter(methodNode); + yield fixer.replaceText( + methodNode, + `${key}: ${typeString}${delimiter}`, + ); + for (const node of duplicatedKeyMethodNodes) { + const lastToken = context.sourceCode.getLastToken(node); + if (lastToken) { + const nextToken = + context.sourceCode.getTokenAfter(lastToken); + if (nextToken) { + yield fixer.remove(node); + yield fixer.replaceTextRange( + [lastToken.range[1], nextToken.range[0]], + '', + ); + } } } - } - }, + }, }); } return; @@ -207,16 +207,18 @@ export default createRule({ context.report({ node: methodNode, messageId: 'errorMethod', - fix: fixer => { - const key = getMethodKey(methodNode); - const params = getMethodParams(methodNode); - const returnType = getMethodReturnType(methodNode); - const delimiter = getDelimiter(methodNode); - return fixer.replaceText( - methodNode, - `${key}: ${params} => ${returnType}${delimiter}`, - ); - }, + fix: + canFix && + (fixer => { + const key = getMethodKey(methodNode); + const params = getMethodParams(methodNode); + const returnType = getMethodReturnType(methodNode); + const delimiter = getDelimiter(methodNode); + return fixer.replaceText( + methodNode, + `${key}: ${params} => ${returnType}${delimiter}`, + ); + }), }); } }, diff --git a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts index 0e7a85421b72..e0044adf2dbb 100644 --- a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -39,26 +39,6 @@ interface Test { ` interface Test { set f(value: number): void; -} - `, - ` -interface Test { - f(value: number): this; -} - `, - ` -interface Test { - f(value: number): this | undefined; -} - `, - ` -interface Test { - f(value: number): Promise; -} - `, - ` -interface Test { - f(value: number): Promise; } `, 'type Test = { readonly f: (a: string) => number };', @@ -690,5 +670,57 @@ interface MyInterface { } `, }, + { + code: ` +interface Test { + f(value: number): this; +} + `, + errors: [ + { + line: 3, + messageId: 'errorMethod', + }, + ], + }, + { + code: ` +interface Test { + f(value: number): this | undefined; +} + `, + errors: [ + { + line: 3, + messageId: 'errorMethod', + }, + ], + }, + { + code: ` +interface Test { + f(value: number): Promise; +} + `, + errors: [ + { + line: 3, + messageId: 'errorMethod', + }, + ], + }, + { + code: ` +interface Test { + f(value: number): Promise; +} + `, + errors: [ + { + line: 3, + messageId: 'errorMethod', + }, + ], + }, ], }); From 21f4aa6f2b68782421dc7bc0ee46acd7fa453185 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 1 Dec 2025 11:19:50 -0500 Subject: [PATCH 3/5] Type touchups --- .../src/rules/method-signature-style.ts | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/packages/eslint-plugin/src/rules/method-signature-style.ts b/packages/eslint-plugin/src/rules/method-signature-style.ts index d0533da10da6..15b3a864abf9 100644 --- a/packages/eslint-plugin/src/rules/method-signature-style.ts +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -158,41 +158,41 @@ export default createRule({ context.report({ node: methodNode, messageId: 'errorMethod', - fix: - canFix && - function* fix(fixer) { - const methodNodes = [ - methodNode, - ...duplicatedKeyMethodNodes, - ].sort((a, b) => (a.range[0] < b.range[0] ? -1 : 1)); - const typeString = methodNodes - .map(node => { - const params = getMethodParams(node); - const returnType = getMethodReturnType(node); - return `(${params} => ${returnType})`; - }) - .join(' & '); - const key = getMethodKey(methodNode); - const delimiter = getDelimiter(methodNode); - yield fixer.replaceText( - methodNode, - `${key}: ${typeString}${delimiter}`, - ); - for (const node of duplicatedKeyMethodNodes) { - const lastToken = context.sourceCode.getLastToken(node); - if (lastToken) { - const nextToken = - context.sourceCode.getTokenAfter(lastToken); - if (nextToken) { - yield fixer.remove(node); - yield fixer.replaceTextRange( - [lastToken.range[1], nextToken.range[0]], - '', - ); + fix: canFix + ? undefined + : function* fix(fixer) { + const methodNodes = [ + methodNode, + ...duplicatedKeyMethodNodes, + ].sort((a, b) => (a.range[0] < b.range[0] ? -1 : 1)); + const typeString = methodNodes + .map(node => { + const params = getMethodParams(node); + const returnType = getMethodReturnType(node); + return `(${params} => ${returnType})`; + }) + .join(' & '); + const key = getMethodKey(methodNode); + const delimiter = getDelimiter(methodNode); + yield fixer.replaceText( + methodNode, + `${key}: ${typeString}${delimiter}`, + ); + for (const node of duplicatedKeyMethodNodes) { + const lastToken = context.sourceCode.getLastToken(node); + if (lastToken) { + const nextToken = + context.sourceCode.getTokenAfter(lastToken); + if (nextToken) { + yield fixer.remove(node); + yield fixer.replaceTextRange( + [lastToken.range[1], nextToken.range[0]], + '', + ); + } } } - } - }, + }, }); } return; @@ -207,18 +207,18 @@ export default createRule({ context.report({ node: methodNode, messageId: 'errorMethod', - fix: - canFix && - (fixer => { - const key = getMethodKey(methodNode); - const params = getMethodParams(methodNode); - const returnType = getMethodReturnType(methodNode); - const delimiter = getDelimiter(methodNode); - return fixer.replaceText( - methodNode, - `${key}: ${params} => ${returnType}${delimiter}`, - ); - }), + fix: canFix + ? undefined + : fixer => { + const key = getMethodKey(methodNode); + const params = getMethodParams(methodNode); + const returnType = getMethodReturnType(methodNode); + const delimiter = getDelimiter(methodNode); + return fixer.replaceText( + methodNode, + `${key}: ${params} => ${returnType}${delimiter}`, + ); + }, }); } }, From 2d80906b2b42a7ad491e59c1acda126fe933da37 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 1 Dec 2025 12:19:56 -0500 Subject: [PATCH 4/5] invert logic --- packages/eslint-plugin/src/rules/method-signature-style.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/method-signature-style.ts b/packages/eslint-plugin/src/rules/method-signature-style.ts index 15b3a864abf9..8480ecb98eba 100644 --- a/packages/eslint-plugin/src/rules/method-signature-style.ts +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -132,7 +132,7 @@ export default createRule({ return; } - const canFix = !returnTypeReferencesThisType(methodNode.returnType); + const skipFix = returnTypeReferencesThisType(methodNode.returnType); const parent = methodNode.parent; const members = parent.type === AST_NODE_TYPES.TSInterfaceBody @@ -158,7 +158,7 @@ export default createRule({ context.report({ node: methodNode, messageId: 'errorMethod', - fix: canFix + fix: skipFix ? undefined : function* fix(fixer) { const methodNodes = [ @@ -207,7 +207,7 @@ export default createRule({ context.report({ node: methodNode, messageId: 'errorMethod', - fix: canFix + fix: skipFix ? undefined : fixer => { const key = getMethodKey(methodNode); From 99cb780ae0cead270c0c156a7c2c2ff1b489e3fb Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 16 Dec 2025 11:03:18 +0200 Subject: [PATCH 5/5] Fill in coverage --- .../rules/method-signature-style.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts index e0044adf2dbb..a747c6f8aa99 100644 --- a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -682,6 +682,26 @@ interface Test { messageId: 'errorMethod', }, ], + output: null, + }, + { + code: ` +interface Test { + foo(): this; + foo(): Promise; +} + `, + errors: [ + { + line: 3, + messageId: 'errorMethod', + }, + { + line: 4, + messageId: 'errorMethod', + }, + ], + output: null, }, { code: ` @@ -695,6 +715,7 @@ interface Test { messageId: 'errorMethod', }, ], + output: null, }, { code: ` @@ -708,6 +729,7 @@ interface Test { messageId: 'errorMethod', }, ], + output: null, }, { code: ` @@ -721,6 +743,7 @@ interface Test { messageId: 'errorMethod', }, ], + output: null, }, ], });