From db131130f7535fd6a87b0351f2e5c9f82c6eb943 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Wed, 24 Mar 2021 22:01:45 +0000 Subject: [PATCH] Don't inherit jsdoc tags from overloaded signatures (#43165) (#43180) * Don't inherit jsdoc tags from overloaded signatures (#43165) Previously, when getting jsdoc for signatures, the services layer would get the jsdoc tags for the base symbol of a signature if it was present. This is fine except when the base was overloaded. In that case, the multiple signatures of the overload would all contribute jsdoc, which is not correct. A more correct fix would be to resolve overloads to the base, but the compiler doesn't have this capability and adding it or jury-rigging it seems like it would be complex, inappropriate for a fix to ship in a patch version. Co-authored-by: Orta Therox Co-authored-by: Orta Therox * Update baseline Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> --- src/services/services.ts | 20 ++- .../deprecatedInheritedJSDocOverload.baseline | 139 ++++++++++++++++++ .../deprecatedInheritedJSDocOverload.ts | 30 ++++ 3 files changed, 178 insertions(+), 11 deletions(-) create mode 100644 tests/baselines/reference/deprecatedInheritedJSDocOverload.baseline create mode 100644 tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts diff --git a/src/services/services.ts b/src/services/services.ts index 1463d81dfdecc..82d3b6c9818c7 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -550,7 +550,7 @@ namespace ts { getJsDocTags(): JSDocTagInfo[] { if (this.jsDocTags === undefined) { - this.jsDocTags = this.declaration ? getJsDocTags([this.declaration], this.checker) : []; + this.jsDocTags = this.declaration ? getJsDocTagsOfSignature(this.declaration, this.checker) : []; } return this.jsDocTags; } @@ -565,15 +565,13 @@ namespace ts { return getJSDocTags(node).some(tag => tag.tagName.text === "inheritDoc"); } - function getJsDocTags(declarations: Declaration[], checker: TypeChecker): JSDocTagInfo[] { - let tags = JsDoc.getJsDocTagsFromDeclarations(declarations); - if (tags.length === 0 || declarations.some(hasJSDocInheritDocTag)) { - forEachUnique(declarations, declaration => { - const inheritedTags = findBaseOfDeclaration(checker, declaration, symbol => symbol.getJsDocTags()); - if (inheritedTags) { - tags = [...inheritedTags, ...tags]; - } - }); + function getJsDocTagsOfSignature(declaration: Declaration, checker: TypeChecker): JSDocTagInfo[] { + let tags = JsDoc.getJsDocTagsFromDeclarations([declaration]); + if (tags.length === 0 || hasJSDocInheritDocTag(declaration)) { + const inheritedTags = findBaseOfDeclaration(checker, declaration, symbol => symbol.declarations?.length === 1 ? symbol.getJsDocTags() : undefined); + if (inheritedTags) { + tags = [...inheritedTags, ...tags]; + } } return tags; } @@ -592,7 +590,7 @@ namespace ts { return doc; } - function findBaseOfDeclaration(checker: TypeChecker, declaration: Declaration, cb: (symbol: Symbol) => T[]): T[] | undefined { + function findBaseOfDeclaration(checker: TypeChecker, declaration: Declaration, cb: (symbol: Symbol) => T[] | undefined): T[] | undefined { return firstDefined(declaration.parent ? getAllSuperTypeNodes(declaration.parent) : emptyArray, superTypeNode => { const symbol = checker.getPropertyOfType(checker.getTypeAtLocation(superTypeNode), declaration.symbol.name); return symbol ? cb(symbol) : undefined; diff --git a/tests/baselines/reference/deprecatedInheritedJSDocOverload.baseline b/tests/baselines/reference/deprecatedInheritedJSDocOverload.baseline new file mode 100644 index 0000000000000..685957e653d58 --- /dev/null +++ b/tests/baselines/reference/deprecatedInheritedJSDocOverload.baseline @@ -0,0 +1,139 @@ +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts", + "position": 1183 + }, + "quickInfo": { + "kind": "method", + "kindModifiers": "", + "textSpan": { + "start": 1174, + "length": 9 + }, + "displayParts": [ + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "method", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "ThingWithDeprecations", + "kind": "interfaceName" + }, + { + "text": "<", + "kind": "punctuation" + }, + { + "text": "void", + "kind": "keyword" + }, + { + "text": ">", + "kind": "punctuation" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "subscribe", + "kind": "methodName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "observer", + "kind": "parameterName" + }, + { + "text": "?", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "PartialObserver", + "kind": "interfaceName" + }, + { + "text": "<", + "kind": "punctuation" + }, + { + "text": "void", + "kind": "keyword" + }, + { + "text": ">", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "Subscription", + "kind": "interfaceName" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "+", + "kind": "operator" + }, + { + "text": "2", + "kind": "numericLiteral" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "overloads", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + } + ], + "documentation": [] + } + } +] \ No newline at end of file diff --git a/tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts b/tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts new file mode 100644 index 0000000000000..838d984bae0a9 --- /dev/null +++ b/tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts @@ -0,0 +1,30 @@ + +//// interface PartialObserver {} +//// interface Subscription {} +//// interface Unsubscribable {} +//// +//// export interface Subscribable { +//// subscribe(observer?: PartialObserver): Unsubscribable; +//// /** @deprecated Base deprecation 1 */ +//// subscribe(next: null | undefined, error: null | undefined, complete: () => void): Unsubscribable; +//// /** @deprecated Base deprecation 2 */ +//// subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Unsubscribable; +//// /** @deprecated Base deprecation 3 */ +//// subscribe(next: (value: T) => void, error: null | undefined, complete: () => void): Unsubscribable; +//// subscribe(next?: (value: T) => void, error?: (error: any) => void, complete?: () => void): Unsubscribable; +//// } + +//// interface ThingWithDeprecations extends Subscribable { +//// subscribe(observer?: PartialObserver): Subscription; +//// /** @deprecated 'real' deprecation */ +//// subscribe(next: null | undefined, error: null | undefined, complete: () => void): Subscription; +//// /** @deprecated 'real' deprecation */ +//// subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Subscription; +//// } + +//// declare const a: ThingWithDeprecations +//// a.subscribe/**/(() => { +//// console.log('something happened'); +//// }); + +verify.baselineQuickInfo(); \ No newline at end of file