From 3d536616055ad7ac8cbfd4944156c6e36b3f08f2 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 9 Mar 2021 12:28:36 -0800 Subject: [PATCH] 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 --- src/services/services.ts | 20 ++- .../deprecatedInheritedJSDocOverload.baseline | 140 ++++++++++++++++++ .../deprecatedInheritedJSDocOverload.ts | 30 ++++ 3 files changed, 179 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 2d482fe5ae9e6..ccc66fe2e2a5d 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..5bfdc82fecaa8 --- /dev/null +++ b/tests/baselines/reference/deprecatedInheritedJSDocOverload.baseline @@ -0,0 +1,140 @@ +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts", + "position": 1183, + "name": "" + }, + "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