Fix hover range calculation: eliminate duplicate calls and remove whitespace from ranges

internal/ls/hover.go

@@ -11,6 +11,7 @@ import (
 	"github.com/microsoft/typescript-go/internal/checker"
 	"github.com/microsoft/typescript-go/internal/core"
 	"github.com/microsoft/typescript-go/internal/lsp/lsproto"
+	"github.com/microsoft/typescript-go/internal/scanner"
 )
 
 const (
@@ -27,10 +28,22 @@ func (l *LanguageService) ProvideHover(ctx context.Context, documentURI lsproto.
 	}
 	c, done := program.GetTypeCheckerForFile(ctx, file)
 	defer done()
-	quickInfo, documentation := getQuickInfoAndDocumentation(c, node)
+
+	// Calculate the applicable range for the hover
+	rangeNode := getNodeForQuickInfo(node)
+	quickInfo, documentation := getQuickInfoAndDocumentationWithNode(c, node, rangeNode)
 	if quickInfo == "" {
 		return lsproto.HoverOrNull{}, nil
 	}
+
+	// Calculate range without leading trivia to avoid including whitespace
+	sourceFile := ast.GetSourceFileOfNode(rangeNode)
+	sourceText := sourceFile.Text()
+	start := scanner.SkipTrivia(sourceText, rangeNode.Pos())
+	end := rangeNode.End()
+	textRange := core.NewTextRange(start, end)
+	hoverRange := l.converters.ToLSPRange(file, textRange)
+
 	return lsproto.HoverOrNull{
 		Hover: &lsproto.Hover{
 			Contents: lsproto.MarkupContentOrStringOrMarkedStringWithLanguageOrMarkedStrings{
@@ -39,12 +52,17 @@ func (l *LanguageService) ProvideHover(ctx context.Context, documentURI lsproto.
 					Value: formatQuickInfo(quickInfo) + documentation,
 				},
 			},
+			Range: &hoverRange,
 		},
 	}, nil
 }
 
 func getQuickInfoAndDocumentation(c *checker.Checker, node *ast.Node) (string, string) {
-	return getQuickInfoAndDocumentationForSymbol(c, c.GetSymbolAtLocation(node), getNodeForQuickInfo(node))
+	return getQuickInfoAndDocumentationWithNode(c, node, getNodeForQuickInfo(node))
+}
+
+func getQuickInfoAndDocumentationWithNode(c *checker.Checker, node *ast.Node, rangeNode *ast.Node) (string, string) {
+	return getQuickInfoAndDocumentationForSymbol(c, c.GetSymbolAtLocation(node), rangeNode)
 }
 
 func getQuickInfoAndDocumentationForSymbol(c *checker.Checker, symbol *ast.Symbol, node *ast.Node) (string, string) {

internal/ls/hover_test.go

@@ -47,6 +47,10 @@ function myFunction() {
 							Value: "```tsx\nfunction myFunction(): string\n```\nA function with JSDoc links that previously caused panic\n`console.log` and `Array.from` and `Object.keys`",
 						},
 					},
+					Range: &lsproto.Range{
+						Start: lsproto.Position{Line: 8, Character: 0},
+						End:   lsproto.Position{Line: 8, Character: 10},
+					},
 				},
 			},
 		},
@@ -70,6 +74,10 @@ myFunction();`,
 							Value: "```tsx\nfunction myFunction(param: string): string\n```\n\n\n*@param* `param` - the greatest of days\n",
 						},
 					},
+					Range: &lsproto.Range{
+						Start: lsproto.Position{Line: 3, Character: 9},
+						End:   lsproto.Position{Line: 3, Character: 19},
+					},
 				},
 			},
 		},
@@ -93,6 +101,10 @@ function myFunction(param) {
 							Value: "```tsx\nfunction myFunction(param: string): string\n```\n\n\n*@param* `param` - the greatest of days\n",
 						},
 					},
+					Range: &lsproto.Range{
+						Start: lsproto.Position{Line: 7, Character: 0},
+						End:   lsproto.Position{Line: 7, Character: 10},
+					},
 				},
 			},
 		},
@@ -116,6 +128,10 @@ myFunction();`,
 							Value: "```tsx\n(parameter) param: string\n```\n- the greatest of days\n",
 						},
 					},
+					Range: &lsproto.Range{
+						Start: lsproto.Position{Line: 3, Character: 20},
+						End:   lsproto.Position{Line: 3, Character: 25},
+					},
 				},
 			},
 		},
@@ -136,6 +152,28 @@ myFunction();`,
 				"marker": nil,
 			},
 		},
+		{
+			title: "HoverRangeTest",
+			input: `
+// @filename: index.ts
+function /*marker*/testFunction(param: string): string {
+    return param;
+}`,
+			expected: map[string]*lsproto.Hover{
+				"marker": {
+					Contents: lsproto.MarkupContentOrStringOrMarkedStringWithLanguageOrMarkedStrings{
+						MarkupContent: &lsproto.MarkupContent{
+							Kind:  lsproto.MarkupKindMarkdown,
+							Value: "```tsx\nfunction testFunction(param: string): string\n```\n",
+						},
+					},
+					Range: &lsproto.Range{
+						Start: lsproto.Position{Line: 0, Character: 9},
+						End:   lsproto.Position{Line: 0, Character: 21},
+					},
+				},
+			},
+		},
 	}
 
 	for _, testCase := range testCases {

testdata/baselines/reference/fourslash/hover/CompletionDetailsOfContextSensitiveParameterNoCrash.baseline

@@ -83,7 +83,7 @@
 //         ...args
 //     ): StylingFunction => {
 //         return curry(getStylingByKeys, 2)(mergedStyling, ...args);
-//                                                             ^
+//                                                             ^^^^
 // | ----------------------------------------------------------------------
 // | ```tsx
 // | (parameter) args: []
@@ -108,6 +108,16 @@
       "contents": {
         "kind": "markdown",
         "value": "```tsx\n(parameter) args: []\n```\n"
+      },
+      "range": {
+        "start": {
+          "line": 82,
+          "character": 60
+        },
+        "end": {
+          "line": 82,
+          "character": 64
+        }
       }
     }
   }

testdata/baselines/reference/fourslash/hover/DeprecatedInheritedJSDocOverload.baseline

@@ -23,7 +23,7 @@
 // }
 // declare const a: ThingWithDeprecations<void>
 // a.subscribe(() => {
-//            ^
+//   ^^^^^^^^^
 // | ----------------------------------------------------------------------
 // | ```tsx
 // | (method) ThingWithDeprecations.subscribe(observer?: PartialObserver<void>): Subscription
@@ -47,6 +47,16 @@
       "contents": {
         "kind": "markdown",
         "value": "```tsx\n(method) ThingWithDeprecations.subscribe(observer?: PartialObserver<void>): Subscription\n```\n"
+      },
+      "range": {
+        "start": {
+          "line": 22,
+          "character": 2
+        },
+        "end": {
+          "line": 22,
+          "character": 11
+        }
       }
     }
   }