Don't inherit jsdoc tags from overloaded signatures (microsoft#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 <git@orta.io> Co-authored-by: Orta Therox <git@orta.io>
orta · Mar 10, 2021 · 5a98be3 · 5a98be3
1 parent 822cb3a
commit 5a98be3
Show file tree

Hide file tree

Showing 3 changed files with 179 additions and 11 deletions.
diff --git 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<T>(checker: TypeChecker, declaration: Declaration, cb: (symbol: Symbol) => T[]): T[] | undefined {
+    function findBaseOfDeclaration<T>(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
@@ -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": []
+    }
+  }
+]
diff --git a/tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts b/tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts
@@ -0,0 +1,30 @@
+
+//// interface PartialObserver<T> {}
+//// interface Subscription {}
+//// interface Unsubscribable {}
+//// 
+//// export interface Subscribable<T> {
+////   subscribe(observer?: PartialObserver<T>): 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<T> extends Subscribable<T> {
+////    subscribe(observer?: PartialObserver<T>): 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<void>
+//// a.subscribe/**/(() => {
+////   console.log('something happened');
+//// });
+
+verify.baselineQuickInfo();