diff --git a/packages/eslint-plugin/docs/rules/unified-signatures.mdx b/packages/eslint-plugin/docs/rules/unified-signatures.mdx index 6813957a5e80..593af8594126 100644 --- a/packages/eslint-plugin/docs/rules/unified-signatures.mdx +++ b/packages/eslint-plugin/docs/rules/unified-signatures.mdx @@ -78,6 +78,50 @@ function f(b: string): void; +### `ignoreOverloadsWithDifferentJSDoc` + +{/* insert option description */} + +Examples of code for this rule with `ignoreOverloadsWithDifferentJSDoc`: + + + + +```ts option='{ "ignoreOverloadsWithDifferentJSDoc": true }' +declare function f(x: string): void; +declare function f(x: boolean): void; +/** + * @deprecate + */ +declare function f(x: number): void; +/** + * @deprecate + */ +declare function f(x: null): void; +``` + + + + +```ts option='{ "ignoreOverloadsWithDifferentJSDoc": true }' +declare function f(x: string): void; +/** + * This signature does something else. + */ +declare function f(x: boolean): void; +/** + * @async + */ +declare function f(x: number): void; +/** + * @deprecate + */ +declare function f(x: null): void; +``` + + + + ## When Not To Use It This is purely a stylistic rule to help with readability of function signature overloads. diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index af8bd281da51..7c0ac6cd161d 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -1,6 +1,6 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; import type { Equal } from '../util'; @@ -61,6 +61,7 @@ export type MessageIds = export type Options = [ { ignoreDifferentlyNamedParameters?: boolean; + ignoreOverloadsWithDifferentJSDoc?: boolean; }, ]; @@ -91,6 +92,11 @@ export default createRule({ description: 'Whether two parameters with different names at the same index should be considered different even if their types are the same.', }, + ignoreOverloadsWithDifferentJSDoc: { + type: 'boolean', + description: + 'Whether two overloads with different JSDoc comments should be considered different even if their parameter and return types are the same.', + }, }, }, ], @@ -98,9 +104,13 @@ export default createRule({ defaultOptions: [ { ignoreDifferentlyNamedParameters: false, + ignoreOverloadsWithDifferentJSDoc: false, }, ], - create(context, [{ ignoreDifferentlyNamedParameters }]) { + create( + context, + [{ ignoreDifferentlyNamedParameters, ignoreOverloadsWithDifferentJSDoc }], + ) { //---------------------------------------------------------------------- // Helpers //---------------------------------------------------------------------- @@ -230,6 +240,15 @@ export default createRule({ } } + if (ignoreOverloadsWithDifferentJSDoc) { + const aComment = getBlockCommentForNode(getExportingNode(a) ?? a); + const bComment = getBlockCommentForNode(getExportingNode(b) ?? b); + + if (aComment?.value !== bComment?.value) { + return false; + } + } + return ( typesAreEqual(a.returnType, b.returnType) && // Must take the same type parameters. @@ -522,6 +541,18 @@ export default createRule({ currentScope = scopes.pop(); } + /** + * @returns the first valid JSDoc comment annotating `node` + */ + function getBlockCommentForNode( + node: TSESTree.Node, + ): TSESTree.Comment | undefined { + return context.sourceCode + .getCommentsBefore(node) + .reverse() + .find(comment => comment.type === AST_TOKEN_TYPES.Block); + } + function addOverload( signature: OverloadNode, key?: string, @@ -590,7 +621,7 @@ export default createRule({ }); function getExportingNode( - node: TSESTree.TSDeclareFunction, + node: SignatureDefinition, ): | TSESTree.ExportDefaultDeclaration | TSESTree.ExportNamedDeclaration diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/unified-signatures.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/unified-signatures.shot index c35fb2cc7545..222058e486b1 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/unified-signatures.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/unified-signatures.shot @@ -60,3 +60,42 @@ function f(a: number): void; function f(b: string): void; " `; + +exports[`Validating rule docs unified-signatures.mdx code examples ESLint output 8`] = ` +"Incorrect +Options: { "ignoreOverloadsWithDifferentJSDoc": true } + +declare function f(x: string): void; +declare function f(x: boolean): void; + ~~~~~~~~~~ This overload and the one on line 1 can be combined into one signature taking \`string | boolean\`. +/** + * @deprecate + */ +declare function f(x: number): void; +/** + * @deprecate + */ +declare function f(x: null): void; + ~~~~~~~ This overload and the one on line 6 can be combined into one signature taking \`number | null\`. +" +`; + +exports[`Validating rule docs unified-signatures.mdx code examples ESLint output 9`] = ` +"Correct +Options: { "ignoreOverloadsWithDifferentJSDoc": true } + +declare function f(x: string): void; +/** + * This signature does something else. + */ +declare function f(x: boolean): void; +/** + * @async + */ +declare function f(x: number): void; +/** + * @deprecate + */ +declare function f(x: null): void; +" +`; diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index f88ba1e89fd8..16e48d02ca5d 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -238,6 +238,136 @@ class C { `, options: [{ ignoreDifferentlyNamedParameters: true }], }, + { + code: ` +/** @deprecated */ +declare function f(x: number): unknown; +declare function f(x: boolean): unknown; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +declare function f(x: number): unknown; +/** @deprecated */ +declare function f(x: boolean): unknown; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +declare function f(x: number): unknown; +/** @deprecated */ declare function f(x: boolean): unknown; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +declare function f(x: string): void; +/** + * @async + */ +declare function f(x: boolean): void; +/** + * @deprecate + */ +declare function f(x: number): void; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/** + * @deprecate + */ +declare function f(x: string): void; +/** + * @async + */ +declare function f(x: boolean): void; +declare function f(x: number): void; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/** + * This signature does something. + */ +declare function f(x: number): void; + +/** + * This signature does something else. + */ +declare function f(x: string): void; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/** @deprecated */ +export function f(x: number): unknown; +export function f(x: boolean): unknown; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/** + * This signature does something. + */ + +// some other comment +export function f(x: number): void; + +/** + * This signature does something else. + */ +export function f(x: string): void; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +interface I { + /** + * This signature does something else. + */ + f(x: number): void; + f(x: string): void; +} + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + // invalid jsdoc comments + { + code: ` +/* @deprecated */ +declare function f(x: number): unknown; +declare function f(x: boolean): unknown; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/* + * This signature does something. + */ +declare function f(x: number): unknown; +declare function f(x: boolean): unknown; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/** + * This signature does something. + **/ +declare function f(x: number): unknown; +declare function f(x: boolean): unknown; + `, + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, ], invalid: [ { @@ -789,5 +919,191 @@ export default function (foo: number, bar?: string): string[]; }, ], }, + { + code: ` +/** + * @deprecate + */ +declare function f(x: string): void; +declare function f(x: number): void; +declare function f(x: boolean): void; + `, + errors: [ + { + column: 20, + data: { + failureStringStart: + 'This overload and the one on line 6 can be combined into one signature', + type1: 'number', + type2: 'boolean', + }, + line: 7, + messageId: 'singleParameterDifference', + }, + ], + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/** + * @deprecate + */ +declare function f(x: string): void; +/** + * @deprecate + */ +declare function f(x: number): void; +declare function f(x: boolean): void; + `, + errors: [ + { + column: 20, + data: { + failureStringStart: + 'This overload and the one on line 5 can be combined into one signature', + type1: 'string', + type2: 'number', + }, + line: 9, + messageId: 'singleParameterDifference', + }, + ], + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +declare function f(x: string): void; +/** + * @deprecate + */ +declare function f(x: number): void; +/** + * @deprecate + */ +declare function f(x: boolean): void; + `, + errors: [ + { + column: 20, + data: { + failureStringStart: + 'This overload and the one on line 6 can be combined into one signature', + type1: 'number', + type2: 'boolean', + }, + line: 10, + messageId: 'singleParameterDifference', + }, + ], + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +export function f(x: string): void; +/** + * @deprecate + */ +export function f(x: number): void; +/** + * @deprecate + */ +export function f(x: boolean): void; + `, + errors: [ + { + column: 19, + data: { + failureStringStart: + 'This overload and the one on line 6 can be combined into one signature', + type1: 'number', + type2: 'boolean', + }, + line: 10, + messageId: 'singleParameterDifference', + }, + ], + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +/** + * This signature does something. + */ + +/** + * This signature does something else. + */ +function f(x: number): void; + +/** + * This signature does something else. + */ +function f(x: string): void; + `, + errors: [ + { + column: 12, + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, + line: 14, + messageId: 'singleParameterDifference', + }, + ], + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +interface I { + f(x: string): void; + /** + * @deprecate + */ + f(x: number): void; + /** + * @deprecate + */ + f(x: boolean): void; +} + `, + errors: [ + { + column: 5, + data: { + failureStringStart: + 'This overload and the one on line 7 can be combined into one signature', + type1: 'number', + type2: 'boolean', + }, + line: 11, + messageId: 'singleParameterDifference', + }, + ], + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, + { + code: ` +// a line comment +declare function f(x: number): unknown; +declare function f(x: boolean): unknown; + `, + errors: [ + { + column: 20, + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'boolean', + }, + line: 4, + messageId: 'singleParameterDifference', + }, + ], + options: [{ ignoreOverloadsWithDifferentJSDoc: true }], + }, ], }); diff --git a/packages/eslint-plugin/tests/schema-snapshots/unified-signatures.shot b/packages/eslint-plugin/tests/schema-snapshots/unified-signatures.shot index 39b61b6dde7d..9f16f8aa1da6 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/unified-signatures.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/unified-signatures.shot @@ -11,6 +11,10 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos "ignoreDifferentlyNamedParameters": { "description": "Whether two parameters with different names at the same index should be considered different even if their types are the same.", "type": "boolean" + }, + "ignoreOverloadsWithDifferentJSDoc": { + "description": "Whether two overloads with different JSDoc comments should be considered different even if their parameter and return types are the same.", + "type": "boolean" } }, "type": "object" @@ -24,6 +28,8 @@ type Options = [ { /** Whether two parameters with different names at the same index should be considered different even if their types are the same. */ ignoreDifferentlyNamedParameters?: boolean; + /** Whether two overloads with different JSDoc comments should be considered different even if their parameter and return types are the same. */ + ignoreOverloadsWithDifferentJSDoc?: boolean; }, ]; "