Skip to content

Commit

Permalink
Don't inherit jsdoc tags from overloaded signatures (#43165) (#43180)
Browse files Browse the repository at this point in the history
* 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 <git@orta.io>

Co-authored-by: Orta Therox <git@orta.io>

* Update baseline

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
  • Loading branch information
Orta Therox and sandersn authored Mar 24, 2021
1 parent e462dfa commit db13113
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 11 deletions.
20 changes: 9 additions & 11 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down
139 changes: 139 additions & 0 deletions tests/baselines/reference/deprecatedInheritedJSDocOverload.baseline
Original file line number Diff line number Diff line change
@@ -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": []
}
}
]
30 changes: 30 additions & 0 deletions tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts
Original file line number Diff line number Diff line change
@@ -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();

0 comments on commit db13113

Please sign in to comment.