Fix and simplify JSDoc handling for binding elements (#4029)

Author: ahejlsberg

internal/fourslash/tests/destructuredIntersectionJSDoc_test.go

@@ -0,0 +1,45 @@
+package fourslash_test
+
+import (
+	"testing"
+
+	"github.com/microsoft/typescript-go/internal/fourslash"
+	"github.com/microsoft/typescript-go/internal/testutil"
+)
+
+func TestDestructuredIntersectionJSDoc(t *testing.T) {
+	t.Parallel()
+	defer testutil.RecoverAndFail(t, "Panic on fourslash test")
+	const content = `
+type X = {
+    /** Description of a. */
+    a: {}
+}
+
+type Y = X & { a: {} }
+
+declare function f({ /*1*/a }: Y): void
+`
+	f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
+	defer done()
+	f.VerifyBaselineHover(t)
+}
+
+func TestDestructuredIntersectionJSDocVariable(t *testing.T) {
+	t.Parallel()
+	defer testutil.RecoverAndFail(t, "Panic on fourslash test")
+	const content = `
+type X = {
+    /** Description of a. */
+    a: {}
+}
+
+type Y = X & { a: {} }
+
+declare const y: Y;
+const { /*1*/a } = y;
+`
+	f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
+	defer done()
+	f.VerifyBaselineHover(t)
+}

internal/ls/hover.go

@@ -155,37 +155,9 @@ func (l *LanguageService) getDocumentationFromDeclaration(c *checker.Checker, sy
 	if declaration == nil {
 		return ""
 	}
-
 	isMarkdown := contentFormat == lsproto.MarkupKindMarkdown
 	var b strings.Builder
-	jsdoc := getJSDocOrTag(c, declaration)
-
-	// Handle binding elements specially (variables created from destructuring) - we need to get the documentation from the property type
-	// If the binding element doesn't have its own JSDoc, fall back to the property's JSDoc
-	if jsdoc == nil && symbol != nil && symbol.ValueDeclaration != nil && ast.IsBindingElement(symbol.ValueDeclaration) && ast.IsIdentifier(location) {
-		bindingElement := symbol.ValueDeclaration
-		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 {
-					jsdoc = getJSDocOrTag(c, propertySymbol.ValueDeclaration)
-					if jsdoc != nil {
-						// Use property declaration for typedef check
-						declaration = propertySymbol.ValueDeclaration
-					}
-				}
-			}
-		}
-	}
-
-	if jsdoc != nil && !(declaration.Flags&ast.NodeFlagsReparsed == 0 && containsTypedefTag(jsdoc)) {
+	if jsdoc := getJSDocOrTag(c, declaration); jsdoc != nil && !(declaration.Flags&ast.NodeFlagsReparsed == 0 && containsTypedefTag(jsdoc)) {
 		l.writeComments(&b, c, jsdoc.Comments(), isMarkdown)
 		if jsdoc.Kind == ast.KindJSDoc && !commentOnly {
 			if tags := jsdoc.AsJSDoc().Tags; tags != nil {
@@ -909,6 +881,18 @@ func getJSDocOrTag(c *checker.Checker, node *ast.Node) *ast.Node {
 	case (ast.IsFunctionExpressionOrArrowFunction(node) || ast.IsClassExpression(node)) &&
 		(ast.IsVariableDeclaration(node.Parent) || ast.IsPropertyDeclaration(node.Parent) || ast.IsPropertyAssignment(node.Parent)) && node.Parent.Initializer() == node:
 		return getJSDocOrTag(c, node.Parent)
+	case ast.IsBindingElement(node) && ast.IsObjectBindingPattern(node.Parent):
+		if name := node.PropertyNameOrName(); ast.IsIdentifier(name) {
+			if objectType := c.GetTypeAtLocation(node.Parent); objectType != nil {
+				if prop := c.GetPropertyOfType(objectType, name.Text()); prop != nil {
+					for _, d := range prop.Declarations {
+						if jsdoc := getJSDoc(d); jsdoc != nil {
+							return jsdoc
+						}
+					}
+				}
+			}
+		}
 	}
 	if symbol := node.Symbol(); symbol != nil && node.Parent != nil {
 		if ast.IsFunctionDeclaration(node) || ast.IsMethodDeclaration(node) || ast.IsMethodSignatureDeclaration(node) || ast.IsConstructorDeclaration(node) || ast.IsConstructSignatureDeclaration(node) {
@@ -1135,20 +1119,6 @@ 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)

testdata/baselines/reference/fourslash/quickInfo/destructuredIntersectionJSDoc.baseline

@@ -0,0 +1,48 @@
+// === QuickInfo ===
+=== /destructuredIntersectionJSDoc.ts ===
+// 
+// type X = {
+//     /** Description of a. */
+//     a: {}
+// }
+// 
+// type Y = X & { a: {} }
+// 
+// declare function f({ a }: Y): void
+//                      ^
+// | ----------------------------------------------------------------------
+// | ```typescript
+// | (parameter) a: {}
+// | ```
+// | Description of a.
+// | ----------------------------------------------------------------------
+// 
+[
+  {
+    "marker": {
+      "Position": 99,
+      "LSPosition": {
+        "line": 8,
+        "character": 21
+      },
+      "Name": "1",
+      "Data": {}
+    },
+    "item": {
+      "contents": {
+        "kind": "markdown",
+        "value": "```typescript\n(parameter) a: {}\n```\nDescription of a."
+      },
+      "range": {
+        "start": {
+          "line": 8,
+          "character": 21
+        },
+        "end": {
+          "line": 8,
+          "character": 22
+        }
+      }
+    }
+  }
+]

testdata/baselines/reference/fourslash/quickInfo/destructuredIntersectionJSDocVariable.baseline

@@ -0,0 +1,49 @@
+// === QuickInfo ===
+=== /destructuredIntersectionJSDocVariable.ts ===
+// 
+// type X = {
+//     /** Description of a. */
+//     a: {}
+// }
+// 
+// type Y = X & { a: {} }
+// 
+// declare const y: Y;
+// const { a } = y;
+//         ^
+// | ----------------------------------------------------------------------
+// | ```typescript
+// | const a: {}
+// | ```
+// | Description of a.
+// | ----------------------------------------------------------------------
+// 
+[
+  {
+    "marker": {
+      "Position": 106,
+      "LSPosition": {
+        "line": 9,
+        "character": 8
+      },
+      "Name": "1",
+      "Data": {}
+    },
+    "item": {
+      "contents": {
+        "kind": "markdown",
+        "value": "```typescript\nconst a: {}\n```\nDescription of a."
+      },
+      "range": {
+        "start": {
+          "line": 9,
+          "character": 8
+        },
+        "end": {
+          "line": 9,
+          "character": 9
+        }
+      }
+    }
+  }
+]