From eb24bb48a6797fef00a2ce4afffa10f2e6ba94a3 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk Date: Sun, 7 Dec 2025 02:26:57 +0200 Subject: [PATCH 1/2] fix(2260): preserve inherited jsdoc details --- internal/ls/hover.go | 131 ++++++++++++------ .../quickInfo/quickInfoInheritDoc.baseline | 17 ++- .../quickInfo/quickInfoInheritDoc2.baseline | 4 +- .../quickInfo/quickInfoInheritDoc3.baseline | 4 +- .../quickInfo/quickInfoJsDocTags6.baseline | 17 ++- 5 files changed, 121 insertions(+), 52 deletions(-) diff --git a/internal/ls/hover.go b/internal/ls/hover.go index 6c8c644adc..3a313b19e2 100644 --- a/internal/ls/hover.go +++ b/internal/ls/hover.go @@ -75,51 +75,76 @@ func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, de return "" } isMarkdown := contentFormat == lsproto.MarkupKindMarkdown + + jsdoc := getJSDocOrTag(c, declaration) + if jsdoc == nil { + return l.getDocumentation(c, getInheritedJSDocOrTag(c, declaration), isMarkdown) + } + if containsTypedefTag(jsdoc) { + return "" + } + var b strings.Builder - if jsdoc := getJSDocOrTag(c, declaration); jsdoc != nil && !containsTypedefTag(jsdoc) { - l.writeComments(&b, c, jsdoc.Comments(), isMarkdown) - if jsdoc.Kind == ast.KindJSDoc { - if tags := jsdoc.AsJSDoc().Tags; tags != nil { - for _, tag := range tags.Nodes { - if tag.Kind == ast.KindJSDocTypeTag { - continue - } - b.WriteString("\n\n") - if isMarkdown { - b.WriteString("*@") - b.WriteString(tag.TagName().Text()) - b.WriteString("*") - } else { - b.WriteString("@") - b.WriteString(tag.TagName().Text()) - } - switch tag.Kind { - case ast.KindJSDocParameterTag, ast.KindJSDocPropertyTag: - writeOptionalEntityName(&b, tag.Name()) - case ast.KindJSDocAugmentsTag: - writeOptionalEntityName(&b, tag.ClassName()) - case ast.KindJSDocSeeTag: - writeOptionalEntityName(&b, tag.AsJSDocSeeTag().NameExpression) - case ast.KindJSDocTemplateTag: - for i, tp := range tag.TypeParameters() { - if i != 0 { - b.WriteString(",") - } - writeOptionalEntityName(&b, tp.Name()) + + if jsdoc.Kind == ast.KindJSDoc { + tags := jsdoc.AsJSDoc().Tags + if len(jsdoc.AsJSDoc().Comments()) == 0 || tags == nil || len(tags.Nodes) == 0 || core.Some(tags.Nodes, isInheritDocTag) { + b.WriteString(l.getDocumentation(c, getInheritedJSDocOrTag(c, declaration), isMarkdown)) + } + } + + b.WriteString(l.getDocumentation(c, jsdoc, isMarkdown)) + return b.String() +} + +func (l *LanguageService) getDocumentation(c *checker.Checker, jsdoc *ast.Node, isMarkdown bool) string { + if jsdoc == nil { + return "" + } + + var b strings.Builder + l.writeComments(&b, c, jsdoc.Comments(), isMarkdown) + if jsdoc.Kind == ast.KindJSDoc { + if tags := jsdoc.AsJSDoc().Tags; tags != nil { + for _, tag := range tags.Nodes { + if tag.Kind == ast.KindJSDocTypeTag { + continue + } + b.WriteString("\n\n") + if isMarkdown { + b.WriteString("*@") + b.WriteString(tag.TagName().Text()) + b.WriteString("*") + } else { + b.WriteString("@") + b.WriteString(tag.TagName().Text()) + } + switch tag.Kind { + case ast.KindJSDocParameterTag, ast.KindJSDocPropertyTag: + writeOptionalEntityName(&b, tag.Name()) + case ast.KindJSDocAugmentsTag: + writeOptionalEntityName(&b, tag.ClassName()) + case ast.KindJSDocSeeTag: + writeOptionalEntityName(&b, tag.AsJSDocSeeTag().NameExpression) + case ast.KindJSDocTemplateTag: + for i, tp := range tag.TypeParameters() { + if i != 0 { + b.WriteString(",") } + writeOptionalEntityName(&b, tp.Name()) } - comments := tag.Comments() - if len(comments) != 0 { - if commentHasPrefix(comments, "```") { - b.WriteString("\n") - } else { - b.WriteString(" ") - if !commentHasPrefix(comments, "-") { - b.WriteString("— ") - } + } + comments := tag.Comments() + if len(comments) != 0 { + if commentHasPrefix(comments, "```") { + b.WriteString("\n") + } else { + b.WriteString(" ") + if !commentHasPrefix(comments, "-") { + b.WriteString("— ") } - l.writeComments(&b, c, comments, isMarkdown) } + l.writeComments(&b, c, comments, isMarkdown) } } } @@ -127,6 +152,10 @@ func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, de return b.String() } +func isInheritDocTag(tag *ast.JSDocTag) bool { + return tag.TagName().Text() == "inheritDoc" || tag.TagName().Text() == "inheritdoc" +} + func formatQuickInfo(quickInfo string) string { var b strings.Builder b.Grow(32) @@ -425,17 +454,33 @@ func getJSDocOrTag(c *checker.Checker, node *ast.Node) *ast.Node { (ast.IsVariableDeclaration(node.Parent) || ast.IsPropertyDeclaration(node.Parent) || ast.IsPropertyAssignment(node.Parent)) && node.Parent.Initializer() == node: return getJSDocOrTag(c, node.Parent) } + return nil +} + +func getInheritedJSDocOrTag(c *checker.Checker, node *ast.Node) *ast.Node { + if ast.IsGetAccessorDeclaration(node) || ast.IsSetAccessorDeclaration(node) { + return nil + } if symbol := node.Symbol(); symbol != nil && node.Parent != nil && ast.IsClassOrInterfaceLike(node.Parent) { isStatic := ast.HasStaticModifier(node) for _, baseType := range c.GetBaseTypes(c.GetDeclaredTypeOfSymbol(node.Parent.Symbol())) { t := baseType - if isStatic { + if isStatic && baseType.Symbol() != nil { t = c.GetTypeOfSymbol(baseType.Symbol()) } if prop := c.GetPropertyOfType(t, symbol.Name); prop != nil && prop.ValueDeclaration != nil { - if jsDoc := getJSDocOrTag(c, prop.ValueDeclaration); jsDoc != nil { - return jsDoc + jsdoc := getJSDocOrTag(c, prop.ValueDeclaration) + if jsdoc == nil { + return getInheritedJSDocOrTag(c, prop.ValueDeclaration) + } + tags := jsdoc.AsJSDoc().Tags + if tags == nil { + return jsdoc + } + if containsTypedefTag(jsdoc) { + return nil } + return jsdoc } } } diff --git a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline index c490bd95dc..3605e7d9b6 100644 --- a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline +++ b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline @@ -40,6 +40,9 @@ // | ```tsx // | (method) SubClass.doSomethingUseful(mySpecificStuff?: { tiger: string; lion: string; }): string // | ``` +// | Useful description always applicable +// | +// | *@returns* — Useful description of return value always applicable. // | // | // | *@inheritDoc* @@ -65,6 +68,12 @@ // | ```tsx // | (method) SubClass.func1(stuff1: any): void // | ``` +// | BaseClass.func1 +// | +// | *@param* `stuff1` — BaseClass.func1.stuff1 +// | +// | +// | *@returns* — BaseClass.func1.returns // | // | // | *@inheritDoc* @@ -88,7 +97,7 @@ // | ```tsx // | (property) SubClass.someProperty: string // | ``` -// | text over tag +// | Applicable description always.text over tag // | // | *@inheritDoc* — text after tag // | @@ -108,7 +117,7 @@ "item": { "contents": { "kind": "markdown", - "value": "```tsx\n(method) SubClass.doSomethingUseful(mySpecificStuff?: { tiger: string; lion: string; }): string\n```\n\n\n*@inheritDoc*\n\n*@param* `mySpecificStuff` — Description of my specific parameter.\n" + "value": "```tsx\n(method) SubClass.doSomethingUseful(mySpecificStuff?: { tiger: string; lion: string; }): string\n```\nUseful description always applicable\n\n*@returns* — Useful description of return value always applicable.\n\n\n*@inheritDoc*\n\n*@param* `mySpecificStuff` — Description of my specific parameter.\n" }, "range": { "start": { @@ -135,7 +144,7 @@ "item": { "contents": { "kind": "markdown", - "value": "```tsx\n(method) SubClass.func1(stuff1: any): void\n```\n\n\n*@inheritDoc*\n\n*@param* `stuff1` — SubClass.func1.stuff1\n\n\n*@returns* — SubClass.func1.returns\n" + "value": "```tsx\n(method) SubClass.func1(stuff1: any): void\n```\nBaseClass.func1\n\n*@param* `stuff1` — BaseClass.func1.stuff1\n\n\n*@returns* — BaseClass.func1.returns\n\n\n*@inheritDoc*\n\n*@param* `stuff1` — SubClass.func1.stuff1\n\n\n*@returns* — SubClass.func1.returns\n" }, "range": { "start": { @@ -162,7 +171,7 @@ "item": { "contents": { "kind": "markdown", - "value": "```tsx\n(property) SubClass.someProperty: string\n```\ntext over tag\n\n*@inheritDoc* — text after tag\n" + "value": "```tsx\n(property) SubClass.someProperty: string\n```\nApplicable description always.text over tag\n\n*@inheritDoc* — text after tag\n" }, "range": { "start": { diff --git a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc2.baseline b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc2.baseline index 4e8510c723..423d8c20c2 100644 --- a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc2.baseline +++ b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc2.baseline @@ -18,7 +18,7 @@ // | ```tsx // | (property) SubClass.prop: T // | ``` -// | +// | Base.prop // | // | *@inheritdoc* — SubClass.prop // | @@ -38,7 +38,7 @@ "item": { "contents": { "kind": "markdown", - "value": "```tsx\n(property) SubClass.prop: T\n```\n\n\n*@inheritdoc* — SubClass.prop\n" + "value": "```tsx\n(property) SubClass.prop: T\n```\nBase.prop\n\n*@inheritdoc* — SubClass.prop\n" }, "range": { "start": { diff --git a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc3.baseline b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc3.baseline index 47214ef747..46f832d9a7 100644 --- a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc3.baseline +++ b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc3.baseline @@ -19,7 +19,7 @@ // | ```tsx // | (property) SubClass.prop: string // | ``` -// | +// | Base.prop // | // | *@inheritdoc* — SubClass.prop // | @@ -39,7 +39,7 @@ "item": { "contents": { "kind": "markdown", - "value": "```tsx\n(property) SubClass.prop: string\n```\n\n\n*@inheritdoc* — SubClass.prop\n" + "value": "```tsx\n(property) SubClass.prop: string\n```\nBase.prop\n\n*@inheritdoc* — SubClass.prop\n" }, "range": { "start": { diff --git a/testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocTags6.baseline b/testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocTags6.baseline index 63fb95c127..b0af392199 100644 --- a/testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocTags6.baseline +++ b/testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocTags6.baseline @@ -22,6 +22,21 @@ // | ```tsx // | (method) Bar.method(x: any, y: any): number // | ``` +// | comment +// | +// | *@author* — Me +// | +// | +// | *@see* `x` — (the parameter) +// | +// | +// | *@param* `x` - x comment +// | +// | +// | *@param* `y` - y comment +// | +// | +// | *@returns* — The result // | // | // | *@inheritDoc* @@ -44,7 +59,7 @@ "item": { "contents": { "kind": "markdown", - "value": "```tsx\n(method) Bar.method(x: any, y: any): number\n```\n\n\n*@inheritDoc*" + "value": "```tsx\n(method) Bar.method(x: any, y: any): number\n```\ncomment\n\n*@author* — Me \n\n\n*@see* `x` — (the parameter)\n\n\n*@param* `x` - x comment\n\n\n*@param* `y` - y comment\n\n\n*@returns* — The result\n\n\n*@inheritDoc*" }, "range": { "start": { From 9065573fd576625562c783ca271ec903679b2483 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk Date: Tue, 9 Dec 2025 00:58:35 +0200 Subject: [PATCH 2/2] combine inherited and local jsdoc comments --- .../tests/quickInfoJsDocInheritDocTag_test.go | 39 +++++++++++ internal/ls/hover.go | 65 ++++++++++++------- .../quickInfo/quickInfoInheritDoc.baseline | 4 +- .../quickInfoJsDocInheritDocTag.baseline | 65 +++++++++++++++++++ 4 files changed, 146 insertions(+), 27 deletions(-) create mode 100644 internal/fourslash/tests/quickInfoJsDocInheritDocTag_test.go create mode 100644 testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocInheritDocTag.baseline diff --git a/internal/fourslash/tests/quickInfoJsDocInheritDocTag_test.go b/internal/fourslash/tests/quickInfoJsDocInheritDocTag_test.go new file mode 100644 index 0000000000..d2d95c2bd8 --- /dev/null +++ b/internal/fourslash/tests/quickInfoJsDocInheritDocTag_test.go @@ -0,0 +1,39 @@ +package fourslash_test + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/fourslash" + "github.com/microsoft/typescript-go/internal/testutil" +) + +func TestQuickInfoJsDocInheritDocTag(t *testing.T) { + t.Parallel() + + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = `// @noEmit: true +// @allowJs: true +// @Filename: quickInfoJsDocInheritDocTag.js +abstract class A { + /** + * A.f description + * @returns {string} A.f return value. + */ + public static f(props?: any): string { + throw new Error("Must be implemented by subclass"); + } +} + +class B extends A { + /** + * B.f description + * @inheritDoc + * @param {{ a: string; b: string; }} [props] description of props + */ + public static /**/f(props?: { a: string; b: string }): string {} +} +` + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) + defer done() + f.VerifyBaselineHover(t) +} diff --git a/internal/ls/hover.go b/internal/ls/hover.go index 3a313b19e2..60a723066a 100644 --- a/internal/ls/hover.go +++ b/internal/ls/hover.go @@ -78,78 +78,93 @@ func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, de jsdoc := getJSDocOrTag(c, declaration) if jsdoc == nil { - return l.getDocumentation(c, getInheritedJSDocOrTag(c, declaration), isMarkdown) + docComments, docTags := l.getDocumentation(c, getInheritedJSDocOrTag(c, declaration), isMarkdown) + return docComments + docTags } + if containsTypedefTag(jsdoc) { return "" } - var b strings.Builder + var commentsBuilder strings.Builder + var tagsBuilder strings.Builder if jsdoc.Kind == ast.KindJSDoc { tags := jsdoc.AsJSDoc().Tags if len(jsdoc.AsJSDoc().Comments()) == 0 || tags == nil || len(tags.Nodes) == 0 || core.Some(tags.Nodes, isInheritDocTag) { - b.WriteString(l.getDocumentation(c, getInheritedJSDocOrTag(c, declaration), isMarkdown)) + docComments, docTags := l.getDocumentation(c, getInheritedJSDocOrTag(c, declaration), isMarkdown) + commentsBuilder.WriteString(docComments) + tagsBuilder.WriteString(docTags) } } - b.WriteString(l.getDocumentation(c, jsdoc, isMarkdown)) - return b.String() + docComments, docTags := l.getDocumentation(c, jsdoc, isMarkdown) + if commentsBuilder.Len() > 0 && len(docComments) > 0 { + commentsBuilder.WriteString(" ") + } + commentsBuilder.WriteString(docComments) + tagsBuilder.WriteString(docTags) + + commentsBuilder.WriteString(tagsBuilder.String()) + return commentsBuilder.String() } -func (l *LanguageService) getDocumentation(c *checker.Checker, jsdoc *ast.Node, isMarkdown bool) string { +func (l *LanguageService) getDocumentation(c *checker.Checker, jsdoc *ast.Node, isMarkdown bool) (comments, tags string) { if jsdoc == nil { - return "" + return "", "" } - var b strings.Builder - l.writeComments(&b, c, jsdoc.Comments(), isMarkdown) + var commnetsBuilder strings.Builder + var tagsBuilder strings.Builder + + l.writeComments(&commnetsBuilder, c, jsdoc.Comments(), isMarkdown) + if jsdoc.Kind == ast.KindJSDoc { if tags := jsdoc.AsJSDoc().Tags; tags != nil { for _, tag := range tags.Nodes { if tag.Kind == ast.KindJSDocTypeTag { continue } - b.WriteString("\n\n") + tagsBuilder.WriteString("\n\n") if isMarkdown { - b.WriteString("*@") - b.WriteString(tag.TagName().Text()) - b.WriteString("*") + tagsBuilder.WriteString("*@") + tagsBuilder.WriteString(tag.TagName().Text()) + tagsBuilder.WriteString("*") } else { - b.WriteString("@") - b.WriteString(tag.TagName().Text()) + tagsBuilder.WriteString("@") + tagsBuilder.WriteString(tag.TagName().Text()) } switch tag.Kind { case ast.KindJSDocParameterTag, ast.KindJSDocPropertyTag: - writeOptionalEntityName(&b, tag.Name()) + writeOptionalEntityName(&tagsBuilder, tag.Name()) case ast.KindJSDocAugmentsTag: - writeOptionalEntityName(&b, tag.ClassName()) + writeOptionalEntityName(&tagsBuilder, tag.ClassName()) case ast.KindJSDocSeeTag: - writeOptionalEntityName(&b, tag.AsJSDocSeeTag().NameExpression) + writeOptionalEntityName(&tagsBuilder, tag.AsJSDocSeeTag().NameExpression) case ast.KindJSDocTemplateTag: for i, tp := range tag.TypeParameters() { if i != 0 { - b.WriteString(",") + tagsBuilder.WriteString(",") } - writeOptionalEntityName(&b, tp.Name()) + writeOptionalEntityName(&tagsBuilder, tp.Name()) } } comments := tag.Comments() if len(comments) != 0 { if commentHasPrefix(comments, "```") { - b.WriteString("\n") + tagsBuilder.WriteString("\n") } else { - b.WriteString(" ") + tagsBuilder.WriteString(" ") if !commentHasPrefix(comments, "-") { - b.WriteString("— ") + tagsBuilder.WriteString("— ") } } - l.writeComments(&b, c, comments, isMarkdown) + l.writeComments(&tagsBuilder, c, comments, isMarkdown) } } } } - return b.String() + return commnetsBuilder.String(), tagsBuilder.String() } func isInheritDocTag(tag *ast.JSDocTag) bool { diff --git a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline index 3605e7d9b6..dd82364dec 100644 --- a/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline +++ b/testdata/baselines/reference/fourslash/quickInfo/quickInfoInheritDoc.baseline @@ -97,7 +97,7 @@ // | ```tsx // | (property) SubClass.someProperty: string // | ``` -// | Applicable description always.text over tag +// | Applicable description always. text over tag // | // | *@inheritDoc* — text after tag // | @@ -171,7 +171,7 @@ "item": { "contents": { "kind": "markdown", - "value": "```tsx\n(property) SubClass.someProperty: string\n```\nApplicable description always.text over tag\n\n*@inheritDoc* — text after tag\n" + "value": "```tsx\n(property) SubClass.someProperty: string\n```\nApplicable description always. text over tag\n\n*@inheritDoc* — text after tag\n" }, "range": { "start": { diff --git a/testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocInheritDocTag.baseline b/testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocInheritDocTag.baseline new file mode 100644 index 0000000000..1770a3e729 --- /dev/null +++ b/testdata/baselines/reference/fourslash/quickInfo/quickInfoJsDocInheritDocTag.baseline @@ -0,0 +1,65 @@ +// === QuickInfo === +=== /quickInfoJsDocInheritDocTag.js === +// abstract class A { +// /** +// * A.f description +// * @returns {string} A.f return value. +// */ +// public static f(props?: any): string { +// throw new Error("Must be implemented by subclass"); +// } +// } +// +// class B extends A { +// /** +// * B.f description +// * @inheritDoc +// * @param {{ a: string; b: string; }} [props] description of props +// */ +// public static f(props?: { a: string; b: string }): string {} +// ^ +// | ---------------------------------------------------------------------- +// | ```tsx +// | (method) B.f(props?: { a: string; b: string; }): string +// | ``` +// | A.f description B.f description +// | +// | *@returns* — A.f return value. +// | +// | +// | *@inheritDoc* +// | +// | *@param* `props` — description of props +// | +// | ---------------------------------------------------------------------- +// } +// +[ + { + "marker": { + "Position": 352, + "LSPosition": { + "line": 16, + "character": 16 + }, + "Name": "", + "Data": {} + }, + "item": { + "contents": { + "kind": "markdown", + "value": "```tsx\n(method) B.f(props?: { a: string; b: string; }): string\n```\nA.f description B.f description\n\n*@returns* — A.f return value.\n\n\n*@inheritDoc*\n\n*@param* `props` — description of props\n" + }, + "range": { + "start": { + "line": 16, + "character": 16 + }, + "end": { + "line": 16, + "character": 17 + } + } + } + } +] \ No newline at end of file