feat(eslint-plugin): [unified-signatures] add `ignoreDifferentlyNamed…

…Parameters` option (#4659)
typescript-eslint · Apr 8, 2022 · fdf95e0 · fdf95e0
1 parent 19c600a
commit fdf95e0
Show file tree

Hide file tree

Showing 3 changed files with 199 additions and 26 deletions.
diff --git 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:
 
 <!--tabs-->
 
@@ -32,6 +48,34 @@ function x(x: number | string): void;
 function y(...x: number[]): void;
 ```
 
+Examples of code for this rule with `ignoreDifferentlyNamedParameters`:
+
+<!--tabs-->
+
+### ❌ 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
@@ -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<Options, MessageIds>({
   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,35 +165,19 @@ 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,
             signature1,
             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
@@ -159,10 +159,60 @@ async function rest(...args: any[], y?: string): Promise<number[] | string> {
   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: `