Correctly resolve tags for function overloads (#30253)

* 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
This commit is contained in:
Jean Pierre 2020-01-10 11:54:26 -05:00 committed by Nathan Shively-Sanders
parent 5fc917be2e
commit 79dcd3dba1
7 changed files with 370 additions and 41 deletions

View File

@ -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;
@ -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 };
@ -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
@ -268,6 +270,7 @@ namespace ts.SymbolDisplay {
addSignatureDisplayParts(signature, allSignatures);
hasAddedSymbolInfo = true;
hasMultipleSignatures = allSignatures.length > 1;
}
}
}
@ -495,6 +498,7 @@ namespace ts.SymbolDisplay {
const allSignatures = type.getNonNullableType().getCallSignatures();
if (allSignatures.length) {
addSignatureDisplayParts(allSignatures[0], allSignatures);
hasMultipleSignatures = allSignatures.length > 1;
}
}
}
@ -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) {
@ -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) {

View File

@ -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(
@ -277,7 +277,7 @@ 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" },
],
@ -285,7 +285,7 @@ verify.completions(
{
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)" },
@ -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");
@ -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" },
],
@ -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");
@ -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)");
@ -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" },
],
@ -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");
@ -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)");
@ -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"],
@ -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"],

View File

@ -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" }]);

View File

@ -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" }]);

View File

@ -0,0 +1,63 @@
/// <reference path='fourslash.ts'/>
// @Filename: quickInfoJsDocTagsFunctionOverload03.ts
////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: 17, 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"}
],
[]);
goTo.marker("2");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 95, 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" }]);

View File

@ -0,0 +1,61 @@
/// <reference path='fourslash.ts'/>
// @Filename: quickInfoJsDocTagsFunctionOverload04.ts
////declare function /*1*/foo(): void;
////
/////**
//// * Doc foo overloaded
//// */
////declare function /*2*/foo(x: number): void
goTo.marker("1");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 17, 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"}
],
[]);
goTo.marker("2");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 78, 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" }]);

View File

@ -0,0 +1,62 @@
/// <reference path='fourslash.ts'/>
// @Filename: quickInfoJsDocTagsFunctionOverload05.ts
////declare function /*1*/foo(): void;
////
/////**
//// * @tag Tag text
//// */
////declare function /*2*/foo(x: number): void
goTo.marker("1");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 17, 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"}
],
[]);
goTo.marker("2");
verify.verifyQuickInfoDisplayParts(
"function",
"declare",
{ start: 73, 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"}
],
[],
[{ name: "tag", text: "Tag text" }]);