Fix missing JSDoc comments on destructured interface properties #2195
+133
−14
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package fourslash_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/typescript-go/internal/fourslash" | ||
| "github.com/microsoft/typescript-go/internal/testutil" | ||
| ) | ||
|
|
||
| func TestDestructuredInterfaceJSDoc(t *testing.T) { | ||
| t.Parallel() | ||
| defer testutil.RecoverAndFail(t, "Panic on fourslash test") | ||
| const content = ` | ||
| interface FooBar { | ||
| /** foo comment */ | ||
| foo: number; | ||
| /** bar comment */ | ||
| bar: string; | ||
| /** baz comment */ | ||
| baz: string; | ||
| } | ||
|
|
||
| declare const fubar: FooBar; | ||
|
|
||
| const {/*1*/foo, /*2*/bar, /*3*/baz: /*4*/biz} = fubar; | ||
| ` | ||
| f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) | ||
| defer done() | ||
| f.VerifyQuickInfoAt(t, "1", "const foo: number", "foo comment") | ||
| f.VerifyQuickInfoAt(t, "2", "const bar: string", "bar comment") | ||
| f.VerifyQuickInfoAt(t, "3", "(property) FooBar.baz: string", "baz comment") | ||
| f.VerifyQuickInfoAt(t, "4", "const biz: string", "baz comment") | ||
| } | ||
|
|
||
| func TestDestructuredInterfaceJSDocWithRename(t *testing.T) { | ||
| t.Parallel() | ||
| defer testutil.RecoverAndFail(t, "Panic on fourslash test") | ||
| const content = ` | ||
| interface FooBar { | ||
| /** foo comment */ | ||
| foo: number; | ||
| /** bar comment */ | ||
| bar: string; | ||
| } | ||
|
|
||
| declare const fubar: FooBar; | ||
|
|
||
| const {foo: /*1*/myFoo, bar: /*2*/myBar} = fubar; | ||
| ` | ||
| f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) | ||
| defer done() | ||
| f.VerifyQuickInfoAt(t, "1", "const myFoo: number", "foo comment") | ||
| f.VerifyQuickInfoAt(t, "2", "const myBar: string", "bar comment") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,10 +67,37 @@ func (l *LanguageService) getQuickInfoAndDocumentationForSymbol(c *checker.Check | |
| if quickInfo == "" { | ||
| return "", "" | ||
| } | ||
| return quickInfo, l.getDocumentationFromDeclaration(c, symbol, declaration, node, contentFormat) | ||
| } | ||
|
|
||
| func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, symbol *ast.Symbol, declaration *ast.Node, location *ast.Node, contentFormat lsproto.MarkupKind) string { | ||
| // Handle binding elements specially (variables created from destructuring) - we need to get the documentation from the property type | ||
| // The declaration passed in might be the binding element itself, but we need the interface property declaration | ||
| // Check all declarations to see if any is a binding element | ||
| if symbol != nil && ast.IsIdentifier(location) { | ||
| for _, decl := range symbol.Declarations { | ||
| if decl != nil && ast.IsBindingElement(decl) { | ||
| bindingElement := decl | ||
| parent := bindingElement.Parent | ||
| name := bindingElement.PropertyName() | ||
| if name == nil { | ||
| name = bindingElement.Name() | ||
| } | ||
| if ast.IsIdentifier(name) && ast.IsObjectBindingPattern(parent) { | ||
| propertyName := name.Text() | ||
| objectType := c.GetTypeAtLocation(parent) | ||
| if objectType != nil { | ||
| propertySymbol := findPropertyInType(c, objectType, propertyName) | ||
| if propertySymbol != nil && propertySymbol.ValueDeclaration != nil { | ||
| declaration = propertySymbol.ValueDeclaration | ||
|
||
| break | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if declaration == nil { | ||
| return "" | ||
| } | ||
|
|
@@ -582,6 +609,20 @@ func writeQuotedString(b *strings.Builder, str string, quote bool) { | |
| } | ||
| } | ||
|
|
||
| // findPropertyInType finds a property in a type, handling union types by searching constituent types | ||
| func findPropertyInType(c *checker.Checker, objectType *checker.Type, propertyName string) *ast.Symbol { | ||
| // For union types, try to find the property in any of the constituent types | ||
| if objectType.IsUnion() { | ||
| for _, t := range objectType.Types() { | ||
| if prop := c.GetPropertyOfType(t, propertyName); prop != nil { | ||
| return prop | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
| return c.GetPropertyOfType(objectType, propertyName) | ||
| } | ||
|
|
||
| func getEntityNameString(name *ast.Node) string { | ||
| var b strings.Builder | ||
| writeEntityNameParts(&b, name) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not so familiar with this, so it could be right, but what I would say is that maybe this needs to be more general and look at all declarations nodes from a given symbol like
getDocumentationCommentdid in the original codebase.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in f6b3006. Made the implementation more general by iterating through all
symbol.Declarations(instead of just checkingsymbol.ValueDeclaration) to find binding elements. This matches the pattern in TypeScript wheregetDocumentationCommentprocesses all declarations from a symbol.