Skip to content

Commit 0673f02

Browse files
ShuiRuTianShuiRuTian
authored
Fix JsDocTags inheritage and setter/getter quickInfo (microsoft#46801)
* tmp * fix jsdoc inheritage for property and setter/getter * fix test * fix test * fix mirrors * add more tests * add tests of jsdoc for intance of class
1 parent f84a67f commit 0673f02

16 files changed

+3281
-82
lines changed

‎src/harness/fourslashImpl.ts‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,11 +1397,11 @@ namespace FourSlash {
13971397
}));
13981398
}
13991399

1400-
public verifyQuickInfoAt(markerName: string | Range, expectedText: string, expectedDocumentation?: string) {
1400+
public verifyQuickInfoAt(markerName: string | Range, expectedText: string, expectedDocumentation?: string, expectedTags?: {name: string; text: string;}[]) {
14011401
if (typeof markerName === "string") this.goToMarker(markerName);
14021402
else this.goToRangeStart(markerName);
14031403

1404-
this.verifyQuickInfoString(expectedText, expectedDocumentation);
1404+
this.verifyQuickInfoString(expectedText, expectedDocumentation, expectedTags);
14051405
}
14061406

14071407
public verifyQuickInfos(namesAndTexts: { [name: string]: string | [string, string] }) {
@@ -1420,16 +1420,29 @@ namespace FourSlash {
14201420
}
14211421
}
14221422

1423-
public verifyQuickInfoString(expectedText: string, expectedDocumentation?: string) {
1423+
public verifyQuickInfoString(expectedText: string, expectedDocumentation?: string, expectedTags?: { name: string; text: string; }[]) {
14241424
if (expectedDocumentation === "") {
1425-
throw new Error("Use 'undefined' instead");
1425+
throw new Error("Use 'undefined' instead of empty string for `expectedDocumentation`");
14261426
}
14271427
const actualQuickInfo = this.languageService.getQuickInfoAtPosition(this.activeFile.fileName, this.currentCaretPosition);
1428-
const actualQuickInfoText = actualQuickInfo ? ts.displayPartsToString(actualQuickInfo.displayParts) : "";
1429-
const actualQuickInfoDocumentation = actualQuickInfo ? ts.displayPartsToString(actualQuickInfo.documentation) : "";
1428+
const actualQuickInfoText = ts.displayPartsToString(actualQuickInfo?.displayParts);
1429+
const actualQuickInfoDocumentation = ts.displayPartsToString(actualQuickInfo?.documentation);
1430+
const actualQuickInfoTags = actualQuickInfo?.tags?.map(tag => ({ name: tag.name, text: ts.displayPartsToString(tag.text) }));
14301431

14311432
assert.equal(actualQuickInfoText, expectedText, this.messageAtLastKnownMarker("quick info text"));
14321433
assert.equal(actualQuickInfoDocumentation, expectedDocumentation || "", this.assertionMessageAtLastKnownMarker("quick info doc"));
1434+
if (!expectedTags) {
1435+
// Skip if `expectedTags` is not given
1436+
}
1437+
else if (!actualQuickInfoTags) {
1438+
assert.equal(actualQuickInfoTags, expectedTags, this.messageAtLastKnownMarker("QuickInfo tags"));
1439+
}
1440+
else {
1441+
ts.zipWith(expectedTags, actualQuickInfoTags, (expectedTag, actualTag) => {
1442+
assert.equal(expectedTag.name, actualTag.name);
1443+
assert.equal(expectedTag.text, actualTag.text, this.messageAtLastKnownMarker("QuickInfo tag " + actualTag.name));
1444+
});
1445+
}
14331446
}
14341447

14351448
public verifyQuickInfoDisplayParts(kind: string, kindModifiers: string, textSpan: TextSpan,

‎src/harness/fourslashInterfaceImpl.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,12 @@ namespace FourSlashInterface {
259259
this.state.verifyInlayHints(expected, span, preference);
260260
}
261261

262-
public quickInfoIs(expectedText: string, expectedDocumentation?: string) {
263-
this.state.verifyQuickInfoString(expectedText, expectedDocumentation);
262+
public quickInfoIs(expectedText: string, expectedDocumentation?: string, expectedTags?: { name: string; text: string; }[]) {
263+
this.state.verifyQuickInfoString(expectedText, expectedDocumentation, expectedTags);
264264
}
265265

266-
public quickInfoAt(markerName: string | FourSlash.Range, expectedText: string, expectedDocumentation?: string) {
267-
this.state.verifyQuickInfoAt(markerName, expectedText, expectedDocumentation);
266+
public quickInfoAt(markerName: string | FourSlash.Range, expectedText: string, expectedDocumentation?: string, expectedTags?: { name: string; text: string; }[]) {
267+
this.state.verifyQuickInfoAt(markerName, expectedText, expectedDocumentation, expectedTags);
268268
}
269269

270270
public quickInfos(namesAndTexts: { [name: string]: string }) {

‎src/services/services.ts‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,9 @@ namespace ts {
299299
contextualGetAccessorDocumentationComment?: SymbolDisplayPart[];
300300
contextualSetAccessorDocumentationComment?: SymbolDisplayPart[];
301301

302+
contextualGetAccessorTags?: JSDocTagInfo[];
303+
contextualSetAccessorTags?: JSDocTagInfo[];
304+
302305
constructor(flags: SymbolFlags, name: __String) {
303306
this.flags = flags;
304307
this.escapedName = name;
@@ -343,13 +346,11 @@ namespace ts {
343346
switch (context?.kind) {
344347
case SyntaxKind.GetAccessor:
345348
if (!this.contextualGetAccessorDocumentationComment) {
346-
this.contextualGetAccessorDocumentationComment = emptyArray;
347349
this.contextualGetAccessorDocumentationComment = getDocumentationComment(filter(this.declarations, isGetAccessor), checker);
348350
}
349351
return this.contextualGetAccessorDocumentationComment;
350352
case SyntaxKind.SetAccessor:
351353
if (!this.contextualSetAccessorDocumentationComment) {
352-
this.contextualSetAccessorDocumentationComment = emptyArray;
353354
this.contextualSetAccessorDocumentationComment = getDocumentationComment(filter(this.declarations, isSetAccessor), checker);
354355
}
355356
return this.contextualSetAccessorDocumentationComment;
@@ -360,11 +361,28 @@ namespace ts {
360361

361362
getJsDocTags(checker?: TypeChecker): JSDocTagInfo[] {
362363
if (this.tags === undefined) {
363-
this.tags = JsDoc.getJsDocTagsFromDeclarations(this.declarations, checker);
364+
this.tags = getJsDocTagsOfDeclarations(this.declarations, checker);
364365
}
365366

366367
return this.tags;
367368
}
369+
370+
getContextualJsDocTags(context: Node | undefined, checker: TypeChecker | undefined): JSDocTagInfo[] {
371+
switch (context?.kind) {
372+
case SyntaxKind.GetAccessor:
373+
if (!this.contextualGetAccessorTags) {
374+
this.contextualGetAccessorTags = getJsDocTagsOfDeclarations(filter(this.declarations, isGetAccessor), checker);
375+
}
376+
return this.contextualGetAccessorTags;
377+
case SyntaxKind.SetAccessor:
378+
if (!this.contextualSetAccessorTags) {
379+
this.contextualSetAccessorTags = getJsDocTagsOfDeclarations(filter(this.declarations, isSetAccessor), checker);
380+
}
381+
return this.contextualSetAccessorTags;
382+
default:
383+
return this.getJsDocTags(checker);
384+
}
385+
}
368386
}
369387

370388
class TokenObject<TKind extends SyntaxKind> extends TokenOrIdentifierObject implements Token<TKind> {
@@ -564,10 +582,7 @@ namespace ts {
564582
}
565583

566584
getJsDocTags(): JSDocTagInfo[] {
567-
if (this.jsDocTags === undefined) {
568-
this.jsDocTags = this.declaration ? getJsDocTagsOfSignature(this.declaration, this.checker) : [];
569-
}
570-
return this.jsDocTags;
585+
return this.jsDocTags || (this.jsDocTags = getJsDocTagsOfDeclarations(singleElementArray(this.declaration), this.checker));
571586
}
572587
}
573588

@@ -580,12 +595,25 @@ namespace ts {
580595
return getJSDocTags(node).some(tag => tag.tagName.text === "inheritDoc");
581596
}
582597

583-
function getJsDocTagsOfSignature(declaration: Declaration, checker: TypeChecker): JSDocTagInfo[] {
584-
let tags = JsDoc.getJsDocTagsFromDeclarations([declaration], checker);
585-
if (tags.length === 0 || hasJSDocInheritDocTag(declaration)) {
586-
const inheritedTags = findBaseOfDeclaration(checker, declaration, symbol => symbol.declarations?.length === 1 ? symbol.getJsDocTags() : undefined);
587-
if (inheritedTags) {
588-
tags = [...inheritedTags, ...tags];
598+
function getJsDocTagsOfDeclarations(declarations: Declaration[] | undefined, checker: TypeChecker | undefined): JSDocTagInfo[] {
599+
if (!declarations) return emptyArray;
600+
601+
let tags = JsDoc.getJsDocTagsFromDeclarations(declarations, checker);
602+
if (checker && (tags.length === 0 || declarations.some(hasJSDocInheritDocTag))) {
603+
const seenSymbols = new Set<Symbol>();
604+
for (const declaration of declarations) {
605+
const inheritedTags = findBaseOfDeclaration(checker, declaration, symbol => {
606+
if (!seenSymbols.has(symbol)) {
607+
seenSymbols.add(symbol);
608+
if (declaration.kind === SyntaxKind.GetAccessor || declaration.kind === SyntaxKind.SetAccessor) {
609+
return symbol.getContextualJsDocTags(declaration, checker);
610+
}
611+
return symbol.declarations?.length === 1 ? symbol.getJsDocTags() : undefined;
612+
}
613+
});
614+
if (inheritedTags) {
615+
tags = [...inheritedTags, ...tags];
616+
}
589617
}
590618
}
591619
return tags;
@@ -601,6 +629,9 @@ namespace ts {
601629
const inheritedDocs = findBaseOfDeclaration(checker, declaration, symbol => {
602630
if (!seenSymbols.has(symbol)) {
603631
seenSymbols.add(symbol);
632+
if (declaration.kind === SyntaxKind.GetAccessor || declaration.kind === SyntaxKind.SetAccessor) {
633+
return symbol.getContextualDocumentationComment(declaration, checker);
634+
}
604635
return symbol.getDocumentationComment(checker);
605636
}
606637
});

‎src/services/symbolDisplay.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ namespace ts.SymbolDisplay {
5858
return isLocalVariableOrFunction(symbol) ? ScriptElementKind.localVariableElement : ScriptElementKind.variableElement;
5959
}
6060
if (flags & SymbolFlags.Function) return isLocalVariableOrFunction(symbol) ? ScriptElementKind.localFunctionElement : ScriptElementKind.functionElement;
61+
// FIXME: getter and setter use the same symbol. And it is rare to use only setter without getter, so in most cases the symbol always has getter flag.
62+
// So, even when the location is just on the declaration of setter, this function returns getter.
6163
if (flags & SymbolFlags.GetAccessor) return ScriptElementKind.memberGetAccessorElement;
6264
if (flags & SymbolFlags.SetAccessor) return ScriptElementKind.memberSetAccessorElement;
6365
if (flags & SymbolFlags.Method) return ScriptElementKind.memberFunctionElement;
@@ -154,9 +156,24 @@ namespace ts.SymbolDisplay {
154156

155157
// Class at constructor site need to be shown as constructor apart from property,method, vars
156158
if (symbolKind !== ScriptElementKind.unknown || symbolFlags & SymbolFlags.Class || symbolFlags & SymbolFlags.Alias) {
157-
// If it is accessor they are allowed only if location is at name of the accessor
159+
// If symbol is accessor, they are allowed only if location is at declaration identifier of the accessor
158160
if (symbolKind === ScriptElementKind.memberGetAccessorElement || symbolKind === ScriptElementKind.memberSetAccessorElement) {
159-
symbolKind = ScriptElementKind.memberVariableElement;
161+
const declaration = find(symbol.declarations as ((GetAccessorDeclaration | SetAccessorDeclaration)[]), declaration => declaration.name === location);
162+
if (declaration) {
163+
switch(declaration.kind){
164+
case SyntaxKind.GetAccessor:
165+
symbolKind = ScriptElementKind.memberGetAccessorElement;
166+
break;
167+
case SyntaxKind.SetAccessor:
168+
symbolKind = ScriptElementKind.memberSetAccessorElement;
169+
break;
170+
default:
171+
Debug.assertNever(declaration);
172+
}
173+
}
174+
else {
175+
symbolKind = ScriptElementKind.memberVariableElement;
176+
}
160177
}
161178

162179
let signature: Signature | undefined;
@@ -492,6 +509,8 @@ namespace ts.SymbolDisplay {
492509

493510
// For properties, variables and local vars: show the type
494511
if (symbolKind === ScriptElementKind.memberVariableElement ||
512+
symbolKind === ScriptElementKind.memberGetAccessorElement ||
513+
symbolKind === ScriptElementKind.memberSetAccessorElement ||
495514
symbolKind === ScriptElementKind.jsxAttribute ||
496515
symbolFlags & SymbolFlags.Variable ||
497516
symbolKind === ScriptElementKind.localVariableElement ||
@@ -566,7 +585,7 @@ namespace ts.SymbolDisplay {
566585
}
567586

568587
if (tags.length === 0 && !hasMultipleSignatures) {
569-
tags = symbol.getJsDocTags(typeChecker);
588+
tags = symbol.getContextualJsDocTags(enclosingDeclaration, typeChecker);
570589
}
571590

572591
if (documentation.length === 0 && documentationFromAlias) {

‎src/services/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ namespace ts {
4444
/* @internal */
4545
getContextualDocumentationComment(context: Node | undefined, checker: TypeChecker | undefined): SymbolDisplayPart[]
4646
getJsDocTags(checker?: TypeChecker): JSDocTagInfo[];
47+
/* @internal */
48+
getContextualJsDocTags(context: Node | undefined, checker: TypeChecker | undefined): JSDocTagInfo[];
4749
}
4850

4951
export interface Type {

0 commit comments

Comments
 (0)