From e61d6cdd795cef863939f0d246806c9731af4379 Mon Sep 17 00:00:00 2001 From: Andy Edwards Date: Fri, 28 Mar 2025 12:12:50 -0500 Subject: [PATCH 1/4] fix(unified-signatures): exempt `this` from optional parameter overload check --- .../src/rules/unified-signatures.ts | 10 +++++ .../tests/rules/unified-signatures.test.ts | 38 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index c58e33085721..e53e142adc9c 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -294,6 +294,10 @@ export default createRule({ : undefined; } + function isThisParam(param: TSESTree.Parameter | undefined) { + return param?.type === AST_NODE_TYPES.Identifier && param.name === 'this'; + } + /** * Detect `a(): void` and `a(x: number): void`. * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. @@ -310,6 +314,12 @@ export default createRule({ const shorter = sig1.length < sig2.length ? sig1 : sig2; const shorterSig = sig1.length < sig2.length ? a : b; + // If one signature has explicit this type and another doesn't, they can't + // be unified. + if (isThisParam(sig1[0]) !== isThisParam(sig2[0])) { + return undefined; + } + // If one is has 2+ parameters more than the other, they must all be optional/rest. // Differ by optional parameters: f() and f(x), f() and f(x, ?y, ...z) // Not allowed: f() and f(x, y) diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index dd8412b53ef7..a8f97b00e5da 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -380,6 +380,16 @@ declare function f(x: boolean): unknown; `, options: [{ ignoreOverloadsWithDifferentJSDoc: true }], }, + ` +function f(): void; +function f(this: {}): void; +function f(this: void | {}): void {} + `, + ` +function f(a: boolean): void; +function f(this: {}, a: boolean): void; +function f(this: void | {}, a: boolean): void {} + `, ], invalid: [ { @@ -1136,5 +1146,33 @@ declare function f(x: boolean): unknown; ], options: [{ ignoreOverloadsWithDifferentJSDoc: true }], }, + { + code: ` +function f(this: {}, a: boolean): void; +function f(this: {}, a: string): void; +function f(this: {}, a: boolean | string): void {} + `, + errors: [ + { + column: 22, + line: 3, + messageId: 'singleParameterDifference', + }, + ], + }, + { + code: ` +function f(this: {}): void; +function f(this: {}, a: string): void; +function f(this: {}, a?: string): void {} + `, + errors: [ + { + column: 22, + line: 3, + messageId: 'omittingSingleParameter', + }, + ], + }, ], }); From 58fef3a7cc7b2fe2bfc861e0ff31fe8c3b0b0c67 Mon Sep 17 00:00:00 2001 From: Andy Edwards Date: Tue, 1 Apr 2025 12:28:50 -0500 Subject: [PATCH 2/4] fix: exempt signatures with `this: void` from unified-signatures --- .../src/rules/unified-signatures.ts | 29 +++++++++++- .../tests/rules/unified-signatures.test.ts | 45 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index e53e142adc9c..f5c251feee26 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -264,6 +264,14 @@ export default createRule({ types1: readonly TSESTree.Parameter[], types2: readonly TSESTree.Parameter[], ): Unify | undefined { + const firstParam1 = types1[0]; + const firstParam2 = types2[0]; + + // exempt signatures with `this: void` from the rule + if (isThisVoidParam(firstParam1) || isThisVoidParam(firstParam2)) { + return undefined; + } + const index = getIndexOfFirstDifference( types1, types2, @@ -294,10 +302,20 @@ export default createRule({ : undefined; } - function isThisParam(param: TSESTree.Parameter | undefined) { + function isThisParam( + param: TSESTree.Parameter | undefined, + ): param is TSESTree.Identifier { return param?.type === AST_NODE_TYPES.Identifier && param.name === 'this'; } + function isThisVoidParam(param: TSESTree.Parameter | undefined) { + return ( + isThisParam(param) && + param?.typeAnnotation?.typeAnnotation?.type === + AST_NODE_TYPES.TSVoidKeyword + ); + } + /** * Detect `a(): void` and `a(x: number): void`. * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. @@ -314,9 +332,16 @@ export default createRule({ const shorter = sig1.length < sig2.length ? sig1 : sig2; const shorterSig = sig1.length < sig2.length ? a : b; + const firstParam1 = sig1[0]; + const firstParam2 = sig2[0]; // If one signature has explicit this type and another doesn't, they can't // be unified. - if (isThisParam(sig1[0]) !== isThisParam(sig2[0])) { + if (isThisParam(firstParam1) !== isThisParam(firstParam2)) { + return undefined; + } + + // exempt signatures with `this: void` from the rule + if (isThisVoidParam(firstParam1) || isThisVoidParam(firstParam2)) { return undefined; } diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index a8f97b00e5da..e10c6295f3a6 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -388,6 +388,11 @@ function f(this: void | {}): void {} ` function f(a: boolean): void; function f(this: {}, a: boolean): void; +function f(this: void | {}, a: boolean): void {} + `, + ` +function f(this: void, a: boolean): void; +function f(this: {}, a: boolean): void; function f(this: void | {}, a: boolean): void {} `, ], @@ -1174,5 +1179,45 @@ function f(this: {}, a?: string): void {} }, ], }, + { + code: ` +function f(this: string): void; +function f(this: number): void; +function f(this: string | number): void {} + `, + errors: [ + { + column: 12, + line: 3, + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'string', + type2: 'number', + }, + }, + ], + }, + { + code: ` +function f(this: string, a: boolean): void; +function f(this: number, a: boolean): void; +function f(this: string | number, a: boolean): void {} + `, + errors: [ + { + column: 12, + line: 3, + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'string', + type2: 'number', + }, + }, + ], + }, ], }); From 99e300f7b1613c3ade982d56c62c3ef1d21819ef Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Sun, 27 Apr 2025 22:49:26 -0600 Subject: [PATCH 3/4] lint fix --- packages/eslint-plugin/src/rules/unified-signatures.ts | 2 +- .../eslint-plugin/tests/rules/unified-signatures.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index f5c251feee26..ce87b88328df 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -311,7 +311,7 @@ export default createRule({ function isThisVoidParam(param: TSESTree.Parameter | undefined) { return ( isThisParam(param) && - param?.typeAnnotation?.typeAnnotation?.type === + param.typeAnnotation?.typeAnnotation.type === AST_NODE_TYPES.TSVoidKeyword ); } diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index e10c6295f3a6..30952ddebb0f 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -1188,14 +1188,14 @@ function f(this: string | number): void {} errors: [ { column: 12, - line: 3, - messageId: 'singleParameterDifference', data: { failureStringStart: 'These overloads can be combined into one signature', type1: 'string', type2: 'number', }, + line: 3, + messageId: 'singleParameterDifference', }, ], }, @@ -1208,14 +1208,14 @@ function f(this: string | number, a: boolean): void {} errors: [ { column: 12, - line: 3, - messageId: 'singleParameterDifference', data: { failureStringStart: 'These overloads can be combined into one signature', type1: 'string', type2: 'number', }, + line: 3, + messageId: 'singleParameterDifference', }, ], }, From f6301ab74d2bd8ac40ed80e3f853cf3051d77f79 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Sun, 27 Apr 2025 23:02:38 -0600 Subject: [PATCH 4/4] remove unsafe type predicate --- .../src/rules/unified-signatures.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index ce87b88328df..809b19dd1aa6 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -302,16 +302,18 @@ export default createRule({ : undefined; } - function isThisParam( - param: TSESTree.Parameter | undefined, - ): param is TSESTree.Identifier { - return param?.type === AST_NODE_TYPES.Identifier && param.name === 'this'; + function isThisParam(param: TSESTree.Parameter | undefined): boolean { + return ( + param != null && + param.type === AST_NODE_TYPES.Identifier && + param.name === 'this' + ); } function isThisVoidParam(param: TSESTree.Parameter | undefined) { return ( isThisParam(param) && - param.typeAnnotation?.typeAnnotation.type === + (param as TSESTree.Identifier).typeAnnotation?.typeAnnotation.type === AST_NODE_TYPES.TSVoidKeyword ); } @@ -332,8 +334,8 @@ export default createRule({ const shorter = sig1.length < sig2.length ? sig1 : sig2; const shorterSig = sig1.length < sig2.length ? a : b; - const firstParam1 = sig1[0]; - const firstParam2 = sig2[0]; + const firstParam1 = sig1.at(0); + const firstParam2 = sig2.at(0); // If one signature has explicit this type and another doesn't, they can't // be unified. if (isThisParam(firstParam1) !== isThisParam(firstParam2)) {