Skip to content

Commit 6126dab

Browse files
ahejlsbergahejlsberg
authored
Multiple fixes to JSDoc display in quick info and signature help (#2389)
1 parent 51d6a11 commit 6126dab

27 files changed

+933
-812
lines changed

‎internal/ls/hover.go‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,21 +68,21 @@ func (l *LanguageService) getQuickInfoAndDocumentationForSymbol(c *checker.Check
6868
if quickInfo == "" {
6969
return "", ""
7070
}
71-
return quickInfo, l.getDocumentationFromDeclaration(c, declaration, contentFormat)
71+
return quickInfo, l.getDocumentationFromDeclaration(c, declaration, contentFormat, false /*commentOnly*/)
7272
}
7373

74-
func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, declaration *ast.Node, contentFormat lsproto.MarkupKind) string {
74+
func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, declaration *ast.Node, contentFormat lsproto.MarkupKind, commentOnly bool) string {
7575
if declaration == nil {
7676
return ""
7777
}
7878
isMarkdown := contentFormat == lsproto.MarkupKindMarkdown
7979
var b strings.Builder
80-
if jsdoc := getJSDocOrTag(c, declaration); jsdoc != nil && !containsTypedefTag(jsdoc) {
80+
if jsdoc := getJSDocOrTag(c, declaration); jsdoc != nil && !(declaration.Flags&ast.NodeFlagsReparsed == 0 && containsTypedefTag(jsdoc)) {
8181
l.writeComments(&b, c, jsdoc.Comments(), isMarkdown)
82-
if jsdoc.Kind == ast.KindJSDoc {
82+
if jsdoc.Kind == ast.KindJSDoc && !commentOnly {
8383
if tags := jsdoc.AsJSDoc().Tags; tags != nil {
8484
for _, tag := range tags.Nodes {
85-
if tag.Kind == ast.KindJSDocTypeTag {
85+
if tag.Kind == ast.KindJSDocTypeTag || tag.Kind == ast.KindJSDocTypedefTag || tag.Kind == ast.KindJSDocCallbackTag {
8686
continue
8787
}
8888
b.WriteString("\n\n")
@@ -142,7 +142,7 @@ func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, de
142142
}
143143
} else if len(comments) != 0 {
144144
b.WriteString(" ")
145-
if !commentHasPrefix(comments, "-") {
145+
if comments[0].Kind != ast.KindJSDocText || !strings.HasPrefix(comments[0].Text(), "-") {
146146
b.WriteString("— ")
147147
}
148148
l.writeComments(&b, c, comments, isMarkdown)
@@ -307,7 +307,7 @@ func getQuickInfoAndDeclarationAtLocation(c *checker.Checker, symbol *ast.Symbol
307307
b.WriteString(" = ")
308308
b.WriteString(c.TypeToStringEx(c.GetDeclaredTypeOfSymbol(symbol), container, typeFormatFlags|checker.TypeFormatFlagsInTypeAlias))
309309
}
310-
declaration = core.Find(symbol.Declarations, ast.IsTypeAliasDeclaration)
310+
declaration = core.Find(symbol.Declarations, ast.IsTypeOrJSTypeAliasDeclaration)
311311
default:
312312
b.WriteString(c.TypeToStringEx(c.GetTypeOfSymbol(symbol), container, typeFormatFlags))
313313
}
@@ -442,10 +442,6 @@ func containsTypedefTag(jsdoc *ast.Node) bool {
442442
return false
443443
}
444444

445-
func commentHasPrefix(comments []*ast.Node, prefix string) bool {
446-
return comments[0].Kind == ast.KindJSDocText && strings.HasPrefix(comments[0].Text(), prefix)
447-
}
448-
449445
func getJSDoc(node *ast.Node) *ast.Node {
450446
return core.LastOrNil(node.JSDoc(nil))
451447
}

‎internal/ls/signaturehelp.go‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -418,17 +418,17 @@ func (l *LanguageService) computeActiveParameter(sig signatureInformation, argum
418418
func (l *LanguageService) getSignatureHelpItem(candidate *checker.Signature, isTypeParameterList bool, callTargetSymbol string, enclosingDeclaration *ast.Node, sourceFile *ast.SourceFile, c *checker.Checker, docFormat lsproto.MarkupKind) []signatureInformation {
419419
var infos []*signatureHelpItemInfo
420420
if isTypeParameterList {
421-
infos = itemInfoForTypeParameters(candidate, c, enclosingDeclaration, sourceFile)
421+
infos = l.itemInfoForTypeParameters(candidate, c, enclosingDeclaration, sourceFile, docFormat)
422422
} else {
423-
infos = itemInfoForParameters(candidate, c, enclosingDeclaration, sourceFile)
423+
infos = l.itemInfoForParameters(candidate, c, enclosingDeclaration, sourceFile, docFormat)
424424
}
425425

426426
suffixDisplayParts := returnTypeToDisplayParts(candidate, c)
427427

428428
// Generate documentation from the signature's declaration
429429
var documentation *string
430430
if declaration := candidate.Declaration(); declaration != nil {
431-
doc := l.getDocumentationFromDeclaration(c, declaration, docFormat)
431+
doc := l.getDocumentationFromDeclaration(c, declaration, docFormat, true /*commentOnly*/)
432432
if doc != "" {
433433
documentation = &doc
434434
}
@@ -462,7 +462,7 @@ func returnTypeToDisplayParts(candidateSignature *checker.Signature, c *checker.
462462
return returnType.String()
463463
}
464464

465-
func itemInfoForTypeParameters(candidateSignature *checker.Signature, c *checker.Checker, enclosingDeclaration *ast.Node, sourceFile *ast.SourceFile) []*signatureHelpItemInfo {
465+
func (l *LanguageService) itemInfoForTypeParameters(candidateSignature *checker.Signature, c *checker.Checker, enclosingDeclaration *ast.Node, sourceFile *ast.SourceFile, docFormat lsproto.MarkupKind) []*signatureHelpItemInfo {
466466
printer := printer.NewPrinter(printer.PrinterOptions{NewLine: core.NewLineKindLF}, printer.PrintHandlers{}, nil)
467467

468468
var typeParameters []*checker.Type
@@ -478,7 +478,7 @@ func itemInfoForTypeParameters(candidateSignature *checker.Signature, c *checker
478478

479479
thisParameter := []signatureHelpParameter{}
480480
if candidateSignature.ThisParameter() != nil {
481-
thisParameter = []signatureHelpParameter{createSignatureHelpParameterForParameter(candidateSignature.ThisParameter(), enclosingDeclaration, printer, sourceFile, c)}
481+
thisParameter = []signatureHelpParameter{l.createSignatureHelpParameterForParameter(candidateSignature.ThisParameter(), enclosingDeclaration, printer, sourceFile, c, docFormat)}
482482
}
483483

484484
// Creating type parameter display label
@@ -504,7 +504,7 @@ func itemInfoForTypeParameters(candidateSignature *checker.Signature, c *checker
504504
displayParameters.WriteString(displayParts.String())
505505
parameters := thisParameter
506506
for j, param := range parameterList {
507-
parameter := createSignatureHelpParameterForParameter(param, enclosingDeclaration, printer, sourceFile, c)
507+
parameter := l.createSignatureHelpParameterForParameter(param, enclosingDeclaration, printer, sourceFile, c, docFormat)
508508
parameters = append(parameters, parameter)
509509
if j > 0 {
510510
displayParameters.WriteString(", ")
@@ -522,7 +522,7 @@ func itemInfoForTypeParameters(candidateSignature *checker.Signature, c *checker
522522
return result
523523
}
524524

525-
func itemInfoForParameters(candidateSignature *checker.Signature, c *checker.Checker, enclosingDeclaratipn *ast.Node, sourceFile *ast.SourceFile) []*signatureHelpItemInfo {
525+
func (l *LanguageService) itemInfoForParameters(candidateSignature *checker.Signature, c *checker.Checker, enclosingDeclaratipn *ast.Node, sourceFile *ast.SourceFile, docFormat lsproto.MarkupKind) []*signatureHelpItemInfo {
526526
printer := printer.NewPrinter(printer.PrinterOptions{NewLine: core.NewLineKindLF}, printer.PrintHandlers{}, nil)
527527

528528
signatureHelpTypeParameters := make([]signatureHelpParameter, len(candidateSignature.TypeParameters()))
@@ -564,7 +564,7 @@ func itemInfoForParameters(candidateSignature *checker.Signature, c *checker.Che
564564
var displayParameters strings.Builder
565565
displayParameters.WriteString(displayParts.String())
566566
for j, param := range parameterList {
567-
parameter := createSignatureHelpParameterForParameter(param, enclosingDeclaratipn, printer, sourceFile, c)
567+
parameter := l.createSignatureHelpParameterForParameter(param, enclosingDeclaratipn, printer, sourceFile, c, docFormat)
568568
parameters[j] = parameter
569569
if j > 0 {
570570
displayParameters.WriteString(", ")
@@ -585,14 +585,26 @@ func itemInfoForParameters(candidateSignature *checker.Signature, c *checker.Che
585585

586586
const signatureHelpNodeBuilderFlags = nodebuilder.FlagsOmitParameterModifiers | nodebuilder.FlagsIgnoreErrors | nodebuilder.FlagsUseAliasDefinedOutsideCurrentScope
587587

588-
func createSignatureHelpParameterForParameter(parameter *ast.Symbol, enclosingDeclaratipn *ast.Node, p *printer.Printer, sourceFile *ast.SourceFile, c *checker.Checker) signatureHelpParameter {
588+
func (l *LanguageService) createSignatureHelpParameterForParameter(parameter *ast.Symbol, enclosingDeclaratipn *ast.Node, p *printer.Printer, sourceFile *ast.SourceFile, c *checker.Checker, docFormat lsproto.MarkupKind) signatureHelpParameter {
589589
display := p.Emit(checker.NewNodeBuilder(c, printer.NewEmitContext()).SymbolToParameterDeclaration(parameter, enclosingDeclaratipn, signatureHelpNodeBuilderFlags, nodebuilder.InternalFlagsNone, nil), sourceFile)
590590
isOptional := parameter.CheckFlags&ast.CheckFlagsOptionalParameter != 0
591591
isRest := parameter.CheckFlags&ast.CheckFlagsRestParameter != 0
592+
var documentation *lsproto.StringOrMarkupContent
593+
if parameter.ValueDeclaration != nil {
594+
doc := l.getDocumentationFromDeclaration(c, parameter.ValueDeclaration, docFormat, true /*commentOnly*/)
595+
if doc != "" {
596+
documentation = &lsproto.StringOrMarkupContent{
597+
MarkupContent: &lsproto.MarkupContent{
598+
Kind: docFormat,
599+
Value: doc,
600+
},
601+
}
602+
}
603+
}
592604
return signatureHelpParameter{
593605
parameterInfo: &lsproto.ParameterInformation{
594606
Label: lsproto.StringOrTuple{String: &display},
595-
Documentation: nil,
607+
Documentation: documentation,
596608
},
597609
isRest: isRest,
598610
isOptional: isOptional,

‎internal/ls/symbols.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,14 @@ func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, nod
125125
if ctx.Err() != nil {
126126
return true
127127
}
128-
if jsdocs := node.JSDoc(file); len(jsdocs) > 0 {
129-
for _, jsdoc := range jsdocs {
130-
if tagList := jsdoc.AsJSDoc().Tags; tagList != nil {
131-
for _, tag := range tagList.Nodes {
132-
if ast.IsJSDocTypedefTag(tag) || ast.IsJSDocCallbackTag(tag) {
133-
addSymbolForNode(tag, nil /*children*/)
128+
if node.Flags&ast.NodeFlagsReparsed == 0 {
129+
if jsdocs := node.JSDoc(file); len(jsdocs) > 0 {
130+
for _, jsdoc := range jsdocs {
131+
if tagList := jsdoc.AsJSDoc().Tags; tagList != nil {
132+
for _, tag := range tagList.Nodes {
133+
if ast.IsJSDocTypedefTag(tag) || ast.IsJSDocCallbackTag(tag) {
134+
addSymbolForNode(tag, nil /*children*/)
135+
}
134136
}
135137
}
136138
}

‎internal/parser/jsdoc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ loop:
298298

299299
func removeLeadingNewlines(comments []string) []string {
300300
i := 0
301-
for i < len(comments) && (comments[i] == "\n" || comments[i] == "\r") {
301+
for i < len(comments) && strings.TrimLeft(comments[i], "\r\n") == "" {
302302
i++
303303
}
304304
return comments[i:]

‎internal/parser/reparser.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ func (p *Parser) reparseUnhosted(tag *ast.Node, parent *ast.Node, jsDoc *ast.Nod
8484
}
8585
typeAlias.AsTypeAliasDeclaration().Type = t
8686
p.finishReparsedNode(typeAlias, tag)
87+
p.jsdocCache[typeAlias] = []*ast.Node{jsDoc}
88+
typeAlias.Flags |= ast.NodeFlagsHasJSDoc
8789
p.reparseList = append(p.reparseList, typeAlias)
8890
case ast.KindJSDocCallbackTag:
8991
callbackTag := tag.AsJSDocCallbackTag()
@@ -94,6 +96,8 @@ func (p *Parser) reparseUnhosted(tag *ast.Node, parent *ast.Node, jsDoc *ast.Nod
9496
typeAlias := p.factory.NewJSTypeAliasDeclaration(nil, p.factory.DeepCloneReparse(callbackTag.FullName), nil, functionType)
9597
typeAlias.AsTypeAliasDeclaration().TypeParameters = p.gatherTypeParameters(jsDoc, tag)
9698
p.finishReparsedNode(typeAlias, tag)
99+
p.jsdocCache[typeAlias] = []*ast.Node{jsDoc}
100+
typeAlias.Flags |= ast.NodeFlagsHasJSDoc
97101
p.reparseList = append(p.reparseList, typeAlias)
98102
case ast.KindJSDocImportTag:
99103
importTag := tag.AsJSDocImportTag()
@@ -171,6 +175,7 @@ func (p *Parser) reparseJSDocSignature(jsSignature *ast.Node, fun *ast.Node, jsD
171175
}
172176
p.finishReparsedNode(parameter, param)
173177
parameters = append(parameters, parameter)
178+
p.reparseJSDocComment(parameter, param)
174179
}
175180
signature.FunctionLikeData().Parameters = p.newNodeList(jsSignature.AsJSDocSignature().Parameters.Loc, parameters)
176181

@@ -205,17 +210,28 @@ func (p *Parser) reparseJSDocTypeLiteral(t *ast.TypeNode) *ast.Node {
205210
}
206211
p.finishReparsedNode(property, prop)
207212
properties = append(properties, property)
213+
p.reparseJSDocComment(property, prop)
208214
}
209215
t = p.factory.NewTypeLiteralNode(p.newNodeList(jstypeliteral.Loc, properties))
210216
if isArrayType {
211217
p.finishReparsedNode(t, jstypeliteral.AsNode())
212218
t = p.factory.NewArrayTypeNode(t)
213219
}
214220
p.finishReparsedNode(t, jstypeliteral.AsNode())
221+
return t
215222
}
216223
return p.factory.DeepCloneReparse(t)
217224
}
218225

226+
func (p *Parser) reparseJSDocComment(node *ast.Node, tag *ast.Node) {
227+
if comment := tag.CommentList(); comment != nil {
228+
propJSDoc := p.factory.NewJSDoc(comment, nil)
229+
p.finishReparsedNode(propJSDoc, tag)
230+
p.jsdocCache[node] = []*ast.Node{propJSDoc}
231+
node.Flags |= ast.NodeFlagsHasJSDoc
232+
}
233+
}
234+
219235
func (p *Parser) gatherTypeParameters(j *ast.Node, tagWithTypeParameters *ast.Node) *ast.NodeList {
220236
var typeParameters []*ast.Node
221237
pos := -1

‎testdata/baselines/reference/fourslash/quickInfo/quickInfoForJSDocWithHttpLinks.baseline‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
// | ```tsx
1818
// | (property) https: number
1919
// | ```
20+
// | ://wass
2021
// |
2122
// | ----------------------------------------------------------------------
2223
// */
@@ -105,7 +106,7 @@
105106
"item": {
106107
"contents": {
107108
"kind": "markdown",
108-
"value": "```tsx\n(property) https: number\n```\n"
109+
"value": "```tsx\n(property) https: number\n```\n://wass\n"
109110
},
110111
"range": {
111112
"start": {

‎testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocTagsTypedef.baseline‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// | ```tsx
99
// | type Bar = { baz: string; qux: string; }
1010
// | ```
11-
// |
11+
// | Bar comment
1212
// | ----------------------------------------------------------------------
1313
// * @property {string} baz - baz comment
1414
// * @property {string} qux - qux comment
@@ -22,7 +22,7 @@
2222
// | ```tsx
2323
// | type Bar = { baz: string; qux: string; }
2424
// | ```
25-
// |
25+
// | Bar comment
2626
// | ----------------------------------------------------------------------
2727
// * @returns {Bar}
2828
// */
@@ -43,7 +43,7 @@
4343
"item": {
4444
"contents": {
4545
"kind": "markdown",
46-
"value": "```tsx\ntype Bar = { baz: string; qux: string; }\n```\n"
46+
"value": "```tsx\ntype Bar = { baz: string; qux: string; }\n```\nBar comment"
4747
},
4848
"range": {
4949
"start": {
@@ -70,7 +70,7 @@
7070
"item": {
7171
"contents": {
7272
"kind": "markdown",
73-
"value": "```tsx\ntype Bar = { baz: string; qux: string; }\n```\n"
73+
"value": "```tsx\ntype Bar = { baz: string; qux: string; }\n```\nBar comment"
7474
},
7575
"range": {
7676
"start": {

‎testdata/baselines/reference/fourslash/signatureHelp/jsDocDontBreakWithNamespaces.baseline‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@
88
// ^
99
// | ----------------------------------------------------------------------
1010
// | foo(): module
11-
// |
12-
// |
13-
// | *@returns* — :@nodefuel/web~Webserver~wsServer#hello} Websocket server object
14-
// |
1511
// | ----------------------------------------------------------------------
1612
//
1713
// /**
@@ -46,10 +42,6 @@
4642
"signatures": [
4743
{
4844
"label": "foo(): module",
49-
"documentation": {
50-
"kind": "markdown",
51-
"value": "\n\n*@returns* — :@nodefuel/web~Webserver~wsServer#hello} Websocket server object\n"
52-
},
5345
"parameters": []
5446
}
5547
],

‎testdata/baselines/reference/fourslash/signatureHelp/jsDocFunctionSignatures5.baseline‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,9 @@
1515
// ^
1616
// | ----------------------------------------------------------------------
1717
// | pathFilter(**basePath: String**, pattern: String, type: String, options: Object): any[]
18+
// | - `basePath: String`: The base path where the search will be performed.
19+
1820
// | Filters a path based on a regexp or glob pattern.
19-
// |
20-
// | *@param* `basePath` — The base path where the search will be performed.
21-
// |
22-
// |
23-
// | *@param* `pattern` — A string defining a regexp of a glob pattern.
24-
// |
25-
// |
26-
// | *@param* `type` — The search pattern type, can be a regexp or a glob.
27-
// |
28-
// |
29-
// | *@param* `options` — A object containing options to the search.
30-
// |
31-
// |
32-
// | *@return* — A list containing the filtered paths.
33-
// |
3421
// | ----------------------------------------------------------------------
3522
[
3623
{
@@ -49,20 +36,36 @@
4936
"label": "pathFilter(basePath: String, pattern: String, type: String, options: Object): any[]",
5037
"documentation": {
5138
"kind": "markdown",
52-
"value": "Filters a path based on a regexp or glob pattern.\n\n*@param* `basePath` — The base path where the search will be performed.\n\n\n*@param* `pattern` — A string defining a regexp of a glob pattern.\n\n\n*@param* `type` — The search pattern type, can be a regexp or a glob.\n\n\n*@param* `options` — A object containing options to the search.\n\n\n*@return* — A list containing the filtered paths.\n"
39+
"value": "Filters a path based on a regexp or glob pattern."
5340
},
5441
"parameters": [
5542
{
56-
"label": "basePath: String"
43+
"label": "basePath: String",
44+
"documentation": {
45+
"kind": "markdown",
46+
"value": "The base path where the search will be performed.\n"
47+
}
5748
},
5849
{
59-
"label": "pattern: String"
50+
"label": "pattern: String",
51+
"documentation": {
52+
"kind": "markdown",
53+
"value": "A string defining a regexp of a glob pattern.\n"
54+
}
6055
},
6156
{
62-
"label": "type: String"
57+
"label": "type: String",
58+
"documentation": {
59+
"kind": "markdown",
60+
"value": "The search pattern type, can be a regexp or a glob.\n"
61+
}
6362
},
6463
{
65-
"label": "options: Object"
64+
"label": "options: Object",
65+
"documentation": {
66+
"kind": "markdown",
67+
"value": "A object containing options to the search.\n"
68+
}
6669
}
6770
],
6871
"activeParameter": 0

0 commit comments

Comments
 (0)