Skip to content

Commit 02d9d73

Browse files
ronamironami
authoredMar 3, 2025
feat(eslint-plugin): [unified-signatures] support ignoring overload signatures with different JSDoc comments (#10781)
* initial implementation * add a test for interfaces * remove some redundant 'export function ...' tests * check for block comments instead of a heuristic to check jsdoc comments
·
v8.32.1v8.26.0
1 parent 84af50e commit 02d9d73

File tree

5 files changed

+439
-3
lines changed

5 files changed

+439
-3
lines changed
 

‎packages/eslint-plugin/docs/rules/unified-signatures.mdx

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,50 @@ function f(b: string): void;
7878
</TabItem>
7979
</Tabs>
8080

81+
### `ignoreOverloadsWithDifferentJSDoc`
82+
83+
{/* insert option description */}
84+
85+
Examples of code for this rule with `ignoreOverloadsWithDifferentJSDoc`:
86+
87+
<Tabs>
88+
<TabItem value="❌ Incorrect">
89+
90+
```ts option='{ "ignoreOverloadsWithDifferentJSDoc": true }'
91+
declare function f(x: string): void;
92+
declare function f(x: boolean): void;
93+
/**
94+
* @deprecate
95+
*/
96+
declare function f(x: number): void;
97+
/**
98+
* @deprecate
99+
*/
100+
declare function f(x: null): void;
101+
```
102+
103+
</TabItem>
104+
<TabItem value="✅ Correct">
105+
106+
```ts option='{ "ignoreOverloadsWithDifferentJSDoc": true }'
107+
declare function f(x: string): void;
108+
/**
109+
* This signature does something else.
110+
*/
111+
declare function f(x: boolean): void;
112+
/**
113+
* @async
114+
*/
115+
declare function f(x: number): void;
116+
/**
117+
* @deprecate
118+
*/
119+
declare function f(x: null): void;
120+
```
121+
122+
</TabItem>
123+
</Tabs>
124+
81125
## When Not To Use It
82126

83127
This is purely a stylistic rule to help with readability of function signature overloads.

‎packages/eslint-plugin/src/rules/unified-signatures.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { TSESTree } from '@typescript-eslint/utils';
22

3-
import { AST_NODE_TYPES } from '@typescript-eslint/utils';
3+
import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils';
44

55
import type { Equal } from '../util';
66

@@ -61,6 +61,7 @@ export type MessageIds =
6161
export type Options = [
6262
{
6363
ignoreDifferentlyNamedParameters?: boolean;
64+
ignoreOverloadsWithDifferentJSDoc?: boolean;
6465
},
6566
];
6667

@@ -91,16 +92,25 @@ export default createRule<Options, MessageIds>({
9192
description:
9293
'Whether two parameters with different names at the same index should be considered different even if their types are the same.',
9394
},
95+
ignoreOverloadsWithDifferentJSDoc: {
96+
type: 'boolean',
97+
description:
98+
'Whether two overloads with different JSDoc comments should be considered different even if their parameter and return types are the same.',
99+
},
94100
},
95101
},
96102
],
97103
},
98104
defaultOptions: [
99105
{
100106
ignoreDifferentlyNamedParameters: false,
107+
ignoreOverloadsWithDifferentJSDoc: false,
101108
},
102109
],
103-
create(context, [{ ignoreDifferentlyNamedParameters }]) {
110+
create(
111+
context,
112+
[{ ignoreDifferentlyNamedParameters, ignoreOverloadsWithDifferentJSDoc }],
113+
) {
104114
//----------------------------------------------------------------------
105115
// Helpers
106116
//----------------------------------------------------------------------
@@ -230,6 +240,15 @@ export default createRule<Options, MessageIds>({
230240
}
231241
}
232242

243+
if (ignoreOverloadsWithDifferentJSDoc) {
244+
const aComment = getBlockCommentForNode(getExportingNode(a) ?? a);
245+
const bComment = getBlockCommentForNode(getExportingNode(b) ?? b);
246+
247+
if (aComment?.value !== bComment?.value) {
248+
return false;
249+
}
250+
}
251+
233252
return (
234253
typesAreEqual(a.returnType, b.returnType) &&
235254
// Must take the same type parameters.
@@ -522,6 +541,18 @@ export default createRule<Options, MessageIds>({
522541
currentScope = scopes.pop();
523542
}
524543

544+
/**
545+
* @returns the first valid JSDoc comment annotating `node`
546+
*/
547+
function getBlockCommentForNode(
548+
node: TSESTree.Node,
549+
): TSESTree.Comment | undefined {
550+
return context.sourceCode
551+
.getCommentsBefore(node)
552+
.reverse()
553+
.find(comment => comment.type === AST_TOKEN_TYPES.Block);
554+
}
555+
525556
function addOverload(
526557
signature: OverloadNode,
527558
key?: string,
@@ -590,7 +621,7 @@ export default createRule<Options, MessageIds>({
590621
});
591622

592623
function getExportingNode(
593-
node: TSESTree.TSDeclareFunction,
624+
node: SignatureDefinition,
594625
):
595626
| TSESTree.ExportDefaultDeclaration
596627
| TSESTree.ExportNamedDeclaration

‎packages/eslint-plugin/tests/docs-eslint-output-snapshots/unified-signatures.shot

Lines changed: 39 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/eslint-plugin/tests/rules/unified-signatures.test.ts

Lines changed: 316 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,136 @@ class C {
250250
`,
251251
options: [{ ignoreDifferentlyNamedParameters: true }],
252252
},
253+
{
254+
code: `
255+
/** @deprecated */
256+
declare function f(x: number): unknown;
257+
declare function f(x: boolean): unknown;
258+
`,
259+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
260+
},
261+
{
262+
code: `
263+
declare function f(x: number): unknown;
264+
/** @deprecated */
265+
declare function f(x: boolean): unknown;
266+
`,
267+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
268+
},
269+
{
270+
code: `
271+
declare function f(x: number): unknown;
272+
/** @deprecated */ declare function f(x: boolean): unknown;
273+
`,
274+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
275+
},
276+
{
277+
code: `
278+
declare function f(x: string): void;
279+
/**
280+
* @async
281+
*/
282+
declare function f(x: boolean): void;
283+
/**
284+
* @deprecate
285+
*/
286+
declare function f(x: number): void;
287+
`,
288+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
289+
},
290+
{
291+
code: `
292+
/**
293+
* @deprecate
294+
*/
295+
declare function f(x: string): void;
296+
/**
297+
* @async
298+
*/
299+
declare function f(x: boolean): void;
300+
declare function f(x: number): void;
301+
`,
302+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
303+
},
304+
{
305+
code: `
306+
/**
307+
* This signature does something.
308+
*/
309+
declare function f(x: number): void;
310+
311+
/**
312+
* This signature does something else.
313+
*/
314+
declare function f(x: string): void;
315+
`,
316+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
317+
},
318+
{
319+
code: `
320+
/** @deprecated */
321+
export function f(x: number): unknown;
322+
export function f(x: boolean): unknown;
323+
`,
324+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
325+
},
326+
{
327+
code: `
328+
/**
329+
* This signature does something.
330+
*/
331+
332+
// some other comment
333+
export function f(x: number): void;
334+
335+
/**
336+
* This signature does something else.
337+
*/
338+
export function f(x: string): void;
339+
`,
340+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
341+
},
342+
{
343+
code: `
344+
interface I {
345+
/**
346+
* This signature does something else.
347+
*/
348+
f(x: number): void;
349+
f(x: string): void;
350+
}
351+
`,
352+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
353+
},
354+
// invalid jsdoc comments
355+
{
356+
code: `
357+
/* @deprecated */
358+
declare function f(x: number): unknown;
359+
declare function f(x: boolean): unknown;
360+
`,
361+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
362+
},
363+
{
364+
code: `
365+
/*
366+
* This signature does something.
367+
*/
368+
declare function f(x: number): unknown;
369+
declare function f(x: boolean): unknown;
370+
`,
371+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
372+
},
373+
{
374+
code: `
375+
/**
376+
* This signature does something.
377+
**/
378+
declare function f(x: number): unknown;
379+
declare function f(x: boolean): unknown;
380+
`,
381+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
382+
},
253383
],
254384
invalid: [
255385
{
@@ -820,5 +950,191 @@ export default function (foo: number, bar?: string): string[];
820950
},
821951
],
822952
},
953+
{
954+
code: `
955+
/**
956+
* @deprecate
957+
*/
958+
declare function f(x: string): void;
959+
declare function f(x: number): void;
960+
declare function f(x: boolean): void;
961+
`,
962+
errors: [
963+
{
964+
column: 20,
965+
data: {
966+
failureStringStart:
967+
'This overload and the one on line 6 can be combined into one signature',
968+
type1: 'number',
969+
type2: 'boolean',
970+
},
971+
line: 7,
972+
messageId: 'singleParameterDifference',
973+
},
974+
],
975+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
976+
},
977+
{
978+
code: `
979+
/**
980+
* @deprecate
981+
*/
982+
declare function f(x: string): void;
983+
/**
984+
* @deprecate
985+
*/
986+
declare function f(x: number): void;
987+
declare function f(x: boolean): void;
988+
`,
989+
errors: [
990+
{
991+
column: 20,
992+
data: {
993+
failureStringStart:
994+
'This overload and the one on line 5 can be combined into one signature',
995+
type1: 'string',
996+
type2: 'number',
997+
},
998+
line: 9,
999+
messageId: 'singleParameterDifference',
1000+
},
1001+
],
1002+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
1003+
},
1004+
{
1005+
code: `
1006+
declare function f(x: string): void;
1007+
/**
1008+
* @deprecate
1009+
*/
1010+
declare function f(x: number): void;
1011+
/**
1012+
* @deprecate
1013+
*/
1014+
declare function f(x: boolean): void;
1015+
`,
1016+
errors: [
1017+
{
1018+
column: 20,
1019+
data: {
1020+
failureStringStart:
1021+
'This overload and the one on line 6 can be combined into one signature',
1022+
type1: 'number',
1023+
type2: 'boolean',
1024+
},
1025+
line: 10,
1026+
messageId: 'singleParameterDifference',
1027+
},
1028+
],
1029+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
1030+
},
1031+
{
1032+
code: `
1033+
export function f(x: string): void;
1034+
/**
1035+
* @deprecate
1036+
*/
1037+
export function f(x: number): void;
1038+
/**
1039+
* @deprecate
1040+
*/
1041+
export function f(x: boolean): void;
1042+
`,
1043+
errors: [
1044+
{
1045+
column: 19,
1046+
data: {
1047+
failureStringStart:
1048+
'This overload and the one on line 6 can be combined into one signature',
1049+
type1: 'number',
1050+
type2: 'boolean',
1051+
},
1052+
line: 10,
1053+
messageId: 'singleParameterDifference',
1054+
},
1055+
],
1056+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
1057+
},
1058+
{
1059+
code: `
1060+
/**
1061+
* This signature does something.
1062+
*/
1063+
1064+
/**
1065+
* This signature does something else.
1066+
*/
1067+
function f(x: number): void;
1068+
1069+
/**
1070+
* This signature does something else.
1071+
*/
1072+
function f(x: string): void;
1073+
`,
1074+
errors: [
1075+
{
1076+
column: 12,
1077+
data: {
1078+
failureStringStart:
1079+
'These overloads can be combined into one signature',
1080+
type1: 'number',
1081+
type2: 'string',
1082+
},
1083+
line: 14,
1084+
messageId: 'singleParameterDifference',
1085+
},
1086+
],
1087+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
1088+
},
1089+
{
1090+
code: `
1091+
interface I {
1092+
f(x: string): void;
1093+
/**
1094+
* @deprecate
1095+
*/
1096+
f(x: number): void;
1097+
/**
1098+
* @deprecate
1099+
*/
1100+
f(x: boolean): void;
1101+
}
1102+
`,
1103+
errors: [
1104+
{
1105+
column: 5,
1106+
data: {
1107+
failureStringStart:
1108+
'This overload and the one on line 7 can be combined into one signature',
1109+
type1: 'number',
1110+
type2: 'boolean',
1111+
},
1112+
line: 11,
1113+
messageId: 'singleParameterDifference',
1114+
},
1115+
],
1116+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
1117+
},
1118+
{
1119+
code: `
1120+
// a line comment
1121+
declare function f(x: number): unknown;
1122+
declare function f(x: boolean): unknown;
1123+
`,
1124+
errors: [
1125+
{
1126+
column: 20,
1127+
data: {
1128+
failureStringStart:
1129+
'These overloads can be combined into one signature',
1130+
type1: 'number',
1131+
type2: 'boolean',
1132+
},
1133+
line: 4,
1134+
messageId: 'singleParameterDifference',
1135+
},
1136+
],
1137+
options: [{ ignoreOverloadsWithDifferentJSDoc: true }],
1138+
},
8231139
],
8241140
});

‎packages/eslint-plugin/tests/schema-snapshots/unified-signatures.shot

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
Please sign in to comment.