From fdf95e02c45e137325c9ddd9d30e7f6b404f4514 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 8 Apr 2022 19:34:20 -0400 Subject: [PATCH] feat(eslint-plugin): [unified-signatures] add `ignoreDifferentlyNamedParameters` option (#4659) --- .../docs/rules/unified-signatures.md | 46 ++++++++- .../src/rules/unified-signatures.ts | 84 +++++++++++----- .../tests/rules/unified-signatures.test.ts | 95 +++++++++++++++++++ 3 files changed, 199 insertions(+), 26 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/unified-signatures.md b/packages/eslint-plugin/docs/rules/unified-signatures.md index 0b5b527568b..4fb7d6a7a80 100644 --- a/packages/eslint-plugin/docs/rules/unified-signatures.md +++ b/packages/eslint-plugin/docs/rules/unified-signatures.md @@ -6,7 +6,23 @@ Warns for any two overloads that could be unified into one by using a union or a This rule aims to keep the source code as maintainable as possible by reducing the amount of overloads. -Examples of code for this rule: +## Options + +```ts +type Options = { + ignoreDifferentlyNamedParameters?: boolean; +}; + +const defaultOptions: Options = { + ignoreDifferentlyNamedParameters: false, +}; +``` + +The rule accepts an options object with the following property: + +- `ignoreDifferentlyNamedParameters`: whether two parameters with different names at the same index should be considered different even if their types are the same. + +Examples of code for this rule with the default options: @@ -32,6 +48,34 @@ function x(x: number | string): void; function y(...x: number[]): void; ``` +Examples of code for this rule with `ignoreDifferentlyNamedParameters`: + + + +### ❌ Incorrect + +```ts +function f(a: number): void; +function f(a: string): void; +``` + +```ts +function f(...a: number[]): void; +function f(...b: string[]): void; +``` + +### ✅ Correct + +```ts +function f(a: number): void; +function f(b: string): void; +``` + +```ts +function f(...a: number[]): void; +function f(...a: string[]): void; +``` + ## Related To - TSLint: [`unified-signatures`](https://palantir.github.io/tslint/rules/unified-signatures/) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index b5a1d8a97ba..8a261b43c2e 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -48,7 +48,18 @@ type MethodDefinition = | TSESTree.MethodDefinition | TSESTree.TSAbstractMethodDefinition; -export default util.createRule({ +type MessageIds = + | 'omittingRestParameter' + | 'omittingSingleParameter' + | 'singleParameterDifference'; + +type Options = [ + { + ignoreDifferentlyNamedParameters?: boolean; + }, +]; + +export default util.createRule({ name: 'unified-signatures', meta: { docs: { @@ -65,10 +76,24 @@ export default util.createRule({ singleParameterDifference: '{{failureStringStart}} taking `{{type1}} | {{type2}}`.', }, - schema: [], + schema: [ + { + additionalProperties: false, + properties: { + ignoreDifferentlyNamedParameters: { + type: 'boolean', + }, + }, + type: 'object', + }, + ], }, - defaultOptions: [], - create(context) { + defaultOptions: [ + { + ignoreDifferentlyNamedParameters: false, + }, + ], + create(context, [{ ignoreDifferentlyNamedParameters }]) { const sourceCode = context.getSourceCode(); //---------------------------------------------------------------------- @@ -140,11 +165,9 @@ export default util.createRule({ const result: Failure[] = []; const isTypeParameter = getIsTypeParameter(typeParameters); for (const overloads of signatures) { - if (overloads.length === 2) { - const signature0 = - (overloads[0] as MethodDefinition).value || overloads[0]; - const signature1 = - (overloads[1] as MethodDefinition).value || overloads[1]; + forEachPair(overloads, (a, b) => { + const signature0 = (a as MethodDefinition).value ?? a; + const signature1 = (b as MethodDefinition).value ?? b; const unify = compareSignatures( signature0, @@ -152,23 +175,9 @@ export default util.createRule({ isTypeParameter, ); if (unify !== undefined) { - result.push({ unify, only2: true }); + result.push({ unify, only2: overloads.length === 2 }); } - } else { - forEachPair(overloads, (a, b) => { - const signature0 = (a as MethodDefinition).value || a; - const signature1 = (b as MethodDefinition).value || b; - - const unify = compareSignatures( - signature0, - signature1, - isTypeParameter, - ); - if (unify !== undefined) { - result.push({ unify, only2: false }); - } - }); - } + }); } return result; } @@ -199,6 +208,21 @@ export default util.createRule({ const bTypeParams = b.typeParameters !== undefined ? b.typeParameters.params : undefined; + if ( + ignoreDifferentlyNamedParameters && + a.params.length === b.params.length + ) { + for (let i = 0; i < a.params.length; i += 1) { + if ( + a.params[i].type === b.params[i].type && + getStaticParameterName(a.params[i]) !== + getStaticParameterName(b.params[i]) + ) { + return false; + } + } + } + return ( typesAreEqual(a.returnType, b.returnType) && // Must take the same type parameters. @@ -591,6 +615,16 @@ function getOverloadInfo(node: OverloadNode): string { } } +function getStaticParameterName(param: TSESTree.Node): string | undefined { + switch (param.type) { + case AST_NODE_TYPES.Identifier: + return param.name; + case AST_NODE_TYPES.RestElement: + return getStaticParameterName(param.argument); + default: + return undefined; + } +} function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { return node.type === AST_NODE_TYPES.Identifier; } diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index ea259aea432..9bd8a07e997 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -159,10 +159,60 @@ async function rest(...args: any[], y?: string): Promise { return y || args; } `, + { + code: ` +function f(a: number): void; +function f(b: string): void; +function f(a: number | string): void {} + `, + options: [{ ignoreDifferentlyNamedParameters: true }], + }, + { + code: ` +function f(a: boolean, ...c: number[]): void; +function f(a: boolean, ...d: string[]): void; +function f(a: boolean, ...c: (number | string)[]): void {} + `, + options: [{ ignoreDifferentlyNamedParameters: true }], + }, + { + code: ` +class C { + constructor(); + constructor(a: number, b: number); + constructor(c?: number, b?: number) {} + + a(): void; + a(a: number, b: number): void; + a(a?: number, d?: number): void {} +} + `, + options: [{ ignoreDifferentlyNamedParameters: true }], + }, ], invalid: [ { code: ` +function f(a: number): void; +function f(b: string): void; +function f(a: number | string): void {} + `, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, + line: 3, + column: 12, + }, + ], + }, + { + code: ` function f(x: number): void; function f(x: string): void; function f(x: any): any { @@ -185,6 +235,29 @@ function f(x: any): any { }, { code: ` +function f(x: number): void; +function f(x: string): void; +function f(x: any): any { + return x; +} + `, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, + line: 3, + column: 12, + }, + ], + options: [{ ignoreDifferentlyNamedParameters: true }], + }, + { + code: ` function opt(xs?: number[]): void; function opt(xs: number[], y: string): void; function opt(...args: any[]) {} @@ -222,6 +295,28 @@ interface I { }, ], }, + { + // For 3 or more overloads, mentions the line. + code: ` +interface I { + a0(): void; + a0(x: string): string; + a0(x: number): void; +} + `, + errors: [ + { + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'This overload and the one on line 3 can be combined into one signature', + }, + line: 5, + column: 6, + }, + ], + options: [{ ignoreDifferentlyNamedParameters: true }], + }, { // Error for extra parameter. code: `