Skip to content

Commit

Permalink
Correctly resolve tags for function overloads (#30253)
Browse files Browse the repository at this point in the history 
* Correctly resolve tags for function overloads. Fixes #30181

* Better fix for #30181. Added more unit tests

* Fix commentsOverloads tests

* Fallback to first signature when doc and tags are empty
  • Loading branch information
@jeanp413 @sandersn
jeanp413 authored and sandersn committed Jan 10, 2020
1 parent 5fc917b commit 79dcd3d
Show file tree
Hide file tree
Showing 7 changed files with 370 additions and 41 deletions.
65 changes: 39 additions & 26 deletions src/services/symbolDisplay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ namespace ts.SymbolDisplay {
}

const displayParts: SymbolDisplayPart[] = [];
let documentation: SymbolDisplayPart[] | undefined;
let tags: JSDocTagInfo[] | undefined;
let documentation: SymbolDisplayPart[] = [];
let tags: JSDocTagInfo[] = [];
const symbolFlags = getCombinedLocalAndExportSymbolFlags(symbol);
let symbolKind = semanticMeaning & SemanticMeaning.Value ? getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(typeChecker, symbol, location) : ScriptElementKind.unknown;
let hasAddedSymbolInfo = false;
Expand All @@ -141,6 +141,7 @@ namespace ts.SymbolDisplay {
let printer: Printer;
let documentationFromAlias: SymbolDisplayPart[] | undefined;
let tagsFromAlias: JSDocTagInfo[] | undefined;
let hasMultipleSignatures = false;

if (location.kind === SyntaxKind.ThisKeyword && !isThisExpression) {
return { displayParts: [keywordPart(SyntaxKind.ThisKeyword)], documentation: [], symbolKind: ScriptElementKind.primitiveType, tags: undefined };
Expand Down Expand Up @@ -236,6 +237,7 @@ namespace ts.SymbolDisplay {
addSignatureDisplayParts(signature, allSignatures);
}
hasAddedSymbolInfo = true;
hasMultipleSignatures = allSignatures.length > 1;
}
}
else if ((isNameOfFunctionDeclaration(location) && !(symbolFlags & SymbolFlags.Accessor)) || // name of function declaration
Expand Down Expand Up @@ -268,6 +270,7 @@ namespace ts.SymbolDisplay {

addSignatureDisplayParts(signature, allSignatures);
hasAddedSymbolInfo = true;
hasMultipleSignatures = allSignatures.length > 1;
}
}
}
Expand Down Expand Up @@ -495,6 +498,7 @@ namespace ts.SymbolDisplay {
const allSignatures = type.getNonNullableType().getCallSignatures();
if (allSignatures.length) {
addSignatureDisplayParts(allSignatures[0], allSignatures);
hasMultipleSignatures = allSignatures.length > 1;
}
}
}
Expand All @@ -504,42 +508,47 @@ namespace ts.SymbolDisplay {
}
}

if (!documentation) {
if (documentation.length === 0 && !hasMultipleSignatures) {
documentation = symbol.getDocumentationComment(typeChecker);
tags = symbol.getJsDocTags();
if (documentation.length === 0 && symbolFlags & SymbolFlags.Property) {
// For some special property access expressions like `exports.foo = foo` or `module.exports.foo = foo`
// there documentation comments might be attached to the right hand side symbol of their declarations.
// The pattern of such special property access is that the parent symbol is the symbol of the file.
if (symbol.parent && forEach(symbol.parent.declarations, declaration => declaration.kind === SyntaxKind.SourceFile)) {
for (const declaration of symbol.declarations) {
if (!declaration.parent || declaration.parent.kind !== SyntaxKind.BinaryExpression) {
continue;
}
}

const rhsSymbol = typeChecker.getSymbolAtLocation((<BinaryExpression>declaration.parent).right);
if (!rhsSymbol) {
continue;
}
if (documentation.length === 0 && symbolFlags & SymbolFlags.Property) {
// For some special property access expressions like `exports.foo = foo` or `module.exports.foo = foo`
// there documentation comments might be attached to the right hand side symbol of their declarations.
// The pattern of such special property access is that the parent symbol is the symbol of the file.
if (symbol.parent && forEach(symbol.parent.declarations, declaration => declaration.kind === SyntaxKind.SourceFile)) {
for (const declaration of symbol.declarations) {
if (!declaration.parent || declaration.parent.kind !== SyntaxKind.BinaryExpression) {
continue;
}

documentation = rhsSymbol.getDocumentationComment(typeChecker);
tags = rhsSymbol.getJsDocTags();
if (documentation.length > 0) {
break;
}
const rhsSymbol = typeChecker.getSymbolAtLocation((<BinaryExpression>declaration.parent).right);
if (!rhsSymbol) {
continue;
}

documentation = rhsSymbol.getDocumentationComment(typeChecker);
tags = rhsSymbol.getJsDocTags();
if (documentation.length > 0) {
break;
}
}
}
}

if (tags.length === 0 && !hasMultipleSignatures) {
tags = symbol.getJsDocTags();
}

if (documentation.length === 0 && documentationFromAlias) {
documentation = documentationFromAlias;
}
if (tags!.length === 0 && tagsFromAlias) {

if (tags.length === 0 && tagsFromAlias) {
tags = tagsFromAlias;
}

return { displayParts, documentation, symbolKind, tags: tags!.length === 0 ? undefined : tags };
return { displayParts, documentation, symbolKind, tags: tags.length === 0 ? undefined : tags };

function getPrinter() {
if (!printer) {
Expand Down Expand Up @@ -620,9 +629,13 @@ namespace ts.SymbolDisplay {
displayParts.push(textPart(allSignatures.length === 2 ? "overload" : "overloads"));
displayParts.push(punctuationPart(SyntaxKind.CloseParenToken));
}
const docComment = signature.getDocumentationComment(typeChecker);
documentation = docComment.length === 0 ? undefined : docComment;
documentation = signature.getDocumentationComment(typeChecker);
tags = signature.getJsDocTags();

if (allSignatures.length > 1 && documentation.length === 0 && tags.length === 0) {
documentation = allSignatures[0].getDocumentationComment(typeChecker);
tags = allSignatures[0].getJsDocTags();
}
}

function writeTypeParametersOfSymbol(symbol: Symbol, enclosingDeclaration: Node | undefined) {
Expand Down
30 changes: 15 additions & 15 deletions tests/cases/fourslash/commentsOverloads.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,11 @@ verify.signatureHelp({ marker: "4", overloadsCount: 2 });
verify.signatureHelp({ marker: "o4", overloadsCount: 2, docComment: "this is signature 1", parameterDocComment: "param a" });

verify.quickInfos({
5: ["function f2(a: number): number (+1 overload)", "this is signature 2\nthis is f2 var comment"],
5: "function f2(a: number): number (+1 overload)",
6: ["function f2(b: string): number (+1 overload)", "this is signature 2"],
7: ["function f2(a: number): number (+1 overload)", "this is signature 2\nthis is f2 var comment"],
7: "function f2(a: number): number (+1 overload)",
"8q": ["function f2(b: string): number (+1 overload)", "this is signature 2"],
o8q: ["function f2(a: number): number (+1 overload)", "this is signature 2\nthis is f2 var comment"],
o8q: "function f2(a: number): number (+1 overload)",
});

verify.signatureHelp(
Expand Down Expand Up @@ -277,15 +277,15 @@ verify.completions(
marker: "17",
includes: [
{ name: "f1", text: "function f1(a: number): number (+1 overload)", documentation: "this is signature 1" },
{ name: "f2", text: "function f2(a: number): number (+1 overload)", documentation: "this is signature 2\nthis is f2 var comment" },
{ name: "f2", text: "function f2(a: number): number (+1 overload)" },
{ name: "f3", text: "function f3(a: number): number (+1 overload)" },
{ name: "f4", text: "function f4(a: number): number (+1 overload)", documentation: "this is signature 4 - with number parameter" },
],
},
{
marker: "18",
includes: [
{ name: "i1_i", text: "var i1_i: i1\nnew (b: number) => any (+1 overload)" },
{ name: "i1_i", text: "var i1_i: i1\nnew (b: number) => any (+1 overload)", documentation: "new 1" },
{ name: "i2_i", text: "var i2_i: i2\nnew (a: string) => any (+1 overload)" },
{ name: "i3_i", text: "var i3_i: i3\nnew (a: string) => any (+1 overload)", documentation: "new 1" },
{ name: "i4_i", text: "var i4_i: i4\nnew (a: string) => any (+1 overload)" },
Expand All @@ -295,7 +295,7 @@ verify.completions(
);

verify.signatureHelp({ marker: "19", overloadsCount: 2 });
verify.quickInfoAt("19q", "var i1_i: i1\nnew (b: number) => any (+1 overload)");
verify.quickInfoAt("19q", "var i1_i: i1\nnew (b: number) => any (+1 overload)", "new 1");

verify.signatureHelp({ marker: "20", overloadsCount: 2, docComment: "new 1" });
verify.quickInfoAt("20q", "var i1_i: i1\nnew (a: string) => any (+1 overload)", "new 1");
Expand All @@ -311,7 +311,7 @@ verify.completions({
marker: "23",
includes: [
{ name: "foo" , text: "(method) i1.foo(a: number): number (+1 overload)", documentation: "foo 1" },
{ name: "foo2", text: "(method) i1.foo2(a: number): number (+1 overload)", documentation: "foo2 2" },
{ name: "foo2", text: "(method) i1.foo2(a: number): number (+1 overload)" },
{ name: "foo3", text: "(method) i1.foo3(a: number): number (+1 overload)" },
{ name: "foo4", text: "(method) i1.foo4(a: number): number (+1 overload)", documentation: "foo4 1" },
],
Expand All @@ -324,7 +324,7 @@ verify.signatureHelp({ marker: "25", overloadsCount: 2, docComment: "foo 2" });
verify.quickInfoAt("25q", "(method) i1.foo(b: string): number (+1 overload)", "foo 2");

verify.signatureHelp({ marker: "26", overloadsCount: 2 });
verify.quickInfoAt("26q", "(method) i1.foo2(a: number): number (+1 overload)", "foo2 2");
verify.quickInfoAt("26q", "(method) i1.foo2(a: number): number (+1 overload)");

verify.signatureHelp({ marker: "27", overloadsCount: 2, docComment: "foo2 2" });
verify.quickInfoAt("27q", "(method) i1.foo2(b: string): number (+1 overload)", "foo2 2");
Expand Down Expand Up @@ -363,7 +363,7 @@ verify.signatureHelp({ marker: "38", overloadsCount: 2, docComment: "this is sig
verify.quickInfoAt("38q", "var i3_i: i3\n(a: number) => number (+1 overload)", "this is signature 1");

verify.signatureHelp({ marker: "39", overloadsCount: 2 });
verify.quickInfoAt("39q", "var i3_i: i3\n(b: string) => number (+1 overload)");
verify.quickInfoAt("39q", "var i3_i: i3\n(b: string) => number (+1 overload)", "this is signature 1");

verify.signatureHelp({ marker: "40", overloadsCount: 2 });
verify.quickInfoAt("40q", "var i4_i: i4\nnew (b: number) => any (+1 overload)");
Expand All @@ -382,7 +382,7 @@ verify.completions({
exact: [
{ name: "prop1", text: "(method) c.prop1(a: number): number (+1 overload)" },
{ name: "prop2", text: "(method) c.prop2(a: number): number (+1 overload)", documentation: "prop2 1" },
{ name: "prop3", text: "(method) c.prop3(a: number): number (+1 overload)", documentation: "prop3 2" },
{ name: "prop3", text: "(method) c.prop3(a: number): number (+1 overload)" },
{ name: "prop4", text: "(method) c.prop4(a: number): number (+1 overload)", documentation: "prop4 1" },
{ name: "prop5", text: "(method) c.prop5(a: number): number (+1 overload)", documentation: "prop5 1" },
],
Expand All @@ -401,7 +401,7 @@ verify.signatureHelp({ marker: "48", overloadsCount: 2 });
verify.quickInfoAt("48q", "(method) c.prop2(b: string): number (+1 overload)", "prop2 1");

verify.signatureHelp({ marker: "49", overloadsCount: 2 });
verify.quickInfoAt("49q", "(method) c.prop3(a: number): number (+1 overload)", "prop3 2");
verify.quickInfoAt("49q", "(method) c.prop3(a: number): number (+1 overload)");

verify.signatureHelp({ marker: "50", overloadsCount: 2, docComment: "prop3 2" });
verify.quickInfoAt("50q", "(method) c.prop3(b: string): number (+1 overload)", "prop3 2");
Expand All @@ -428,7 +428,7 @@ verify.signatureHelp({ marker: "57", overloadsCount: 2, docComment: "c2 1" });
verify.quickInfoAt("57q", "constructor c2(a: number): c2 (+1 overload)", "c2 1");

verify.signatureHelp({ marker: "58", overloadsCount: 2 });
verify.quickInfoAt("58q", "constructor c2(b: string): c2 (+1 overload)");
verify.quickInfoAt("58q", "constructor c2(b: string): c2 (+1 overload)", "c2 1");

verify.signatureHelp({ marker: "59", overloadsCount: 2 });
verify.quickInfoAt("59q", "constructor c3(a: number): c3 (+1 overload)");
Expand Down Expand Up @@ -490,7 +490,7 @@ verify.quickInfos({
79: "constructor c1(b: string): c1 (+1 overload)",
80: "constructor c1(a: number): c1 (+1 overload)",
81: ["constructor c2(a: number): c2 (+1 overload)", "c2 1"],
82: "constructor c2(b: string): c2 (+1 overload)",
82: ["constructor c2(b: string): c2 (+1 overload)", "c2 1"],
83: ["constructor c2(a: number): c2 (+1 overload)", "c2 1"],
84: "constructor c3(a: number): c3 (+1 overload)",
85: ["constructor c3(b: string): c3 (+1 overload)", "c3 2"],
Expand All @@ -507,9 +507,9 @@ verify.quickInfos({
96: ["(method) c.prop2(a: number): number (+1 overload)", "prop2 1"],
97: ["(method) c.prop2(b: string): number (+1 overload)", "prop2 1"],
98: ["(method) c.prop2(a: number): number (+1 overload)", "prop2 1"],
99: ["(method) c.prop3(a: number): number (+1 overload)", "prop3 2"],
99: "(method) c.prop3(a: number): number (+1 overload)",
100: ["(method) c.prop3(b: string): number (+1 overload)", "prop3 2"],
101: ["(method) c.prop3(a: number): number (+1 overload)", "prop3 2"],
101: "(method) c.prop3(a: number): number (+1 overload)",
102: ["(method) c.prop4(a: number): number (+1 overload)", "prop4 1"],
103: ["(method) c.prop4(b: string): number (+1 overload)", "prop4 2"],
104: ["(method) c.prop4(a: number): number (+1 overload)", "prop4 1"],
Expand Down
66 changes: 66 additions & 0 deletions tests/cases/fourslash/quickInfoJsDocTagsFunctionOverload01.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/// <reference path='fourslash.ts'/>

// @Filename: quickInfoJsDocTagsFunctionOverload01.ts
/////**
//// * Doc foo
//// */
////declare function /*1*/foo(): void;
////
/////**
//// * Doc foo overloaded
//// * @tag Tag text
//// */
////declare function /*2*/foo(x: number): void

goTo.marker("1");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 36, length: 3 },
[
{text:"function",kind:"keyword"},
{text:" ",kind:"space"},
{text:"foo",kind:"functionName"},
{text:"(",kind:"punctuation"},
{text:")",kind:"punctuation"},
{text:":",kind:"punctuation"},
{text:" ",kind:"space"},
{text:"void",kind:"keyword"},
{text:" ",kind:"space"},
{text:"(",kind:"punctuation"},
{text:"+",kind:"operator"},
{text:"1",kind:"numericLiteral"},
{text:" ",kind:"space"},
{text:"overload",kind:"text"},
{text:")",kind:"punctuation"}
],
[{ text: "Doc foo", kind: "text" }]);

goTo.marker("2");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 114, length: 3 },
[
{text:"function",kind:"keyword"},
{text:" ",kind:"space"},
{text:"foo",kind:"functionName"},
{text:"(",kind:"punctuation"},
{text:"x",kind:"parameterName"},
{text:":",kind:"punctuation"},
{text:" ",kind:"space"},
{text:"number",kind:"keyword"},
{text:")",kind:"punctuation"},
{text:":",kind:"punctuation"},
{text:" ",kind:"space"},
{text:"void",kind:"keyword"},
{text:" ",kind:"space"},
{text:"(",kind:"punctuation"},
{text:"+",kind:"operator"},
{text:"1",kind:"numericLiteral"},
{text:" ",kind:"space"},
{text:"overload",kind:"text"},
{text:")",kind:"punctuation"}
],
[{ text: "Doc foo overloaded", kind: "text" }],
[{ name: "tag", text: "Tag text" }]);
64 changes: 64 additions & 0 deletions tests/cases/fourslash/quickInfoJsDocTagsFunctionOverload02.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/// <reference path='fourslash.ts'/>

// @Filename: quickInfoJsDocTagsFunctionOverload02.ts
/////**
//// * Doc foo
//// */
////declare function /*1*/foo(): void;
////
/////**
//// * Doc foo overloaded
//// */
////declare function /*2*/foo(x: number): void

goTo.marker("1");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 36, length: 3 },
[
{text:"function",kind:"keyword"},
{text:" ",kind:"space"},
{text:"foo",kind:"functionName"},
{text:"(",kind:"punctuation"},
{text:")",kind:"punctuation"},
{text:":",kind:"punctuation"},
{text:" ",kind:"space"},
{text:"void",kind:"keyword"},
{text:" ",kind:"space"},
{text:"(",kind:"punctuation"},
{text:"+",kind:"operator"},
{text:"1",kind:"numericLiteral"},
{text:" ",kind:"space"},
{text:"overload",kind:"text"},
{text:")",kind:"punctuation"}
],
[{ text: "Doc foo", kind: "text" }]);

goTo.marker("2");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 97, length: 3 },
[
{text:"function",kind:"keyword"},
{text:" ",kind:"space"},
{text:"foo",kind:"functionName"},
{text:"(",kind:"punctuation"},
{text:"x",kind:"parameterName"},
{text:":",kind:"punctuation"},
{text:" ",kind:"space"},
{text:"number",kind:"keyword"},
{text:")",kind:"punctuation"},
{text:":",kind:"punctuation"},
{text:" ",kind:"space"},
{text:"void",kind:"keyword"},
{text:" ",kind:"space"},
{text:"(",kind:"punctuation"},
{text:"+",kind:"operator"},
{text:"1",kind:"numericLiteral"},
{text:" ",kind:"space"},
{text:"overload",kind:"text"},
{text:")",kind:"punctuation"}
],
[{ text: "Doc foo overloaded", kind: "text" }]);
Loading

0 comments on commit 79dcd3d

Please sign in to comment.