Fix hover and qualified-symbol resolution for jdtls

cmd/generate/tables.go

@@ -67,8 +67,11 @@ var renameProp = map[prop]string{
 
 	{"ExecuteCommandParams", "arguments"}: "[]json.RawMessage",
 	{"FoldingRange", "kind"}:              "string",
-	{"Hover", "contents"}:                 "MarkupContent",
-	{"InlayHint", "label"}:                "[]InlayHintLabelPart",
+	// Hover.contents intentionally has no override: the generator emits the
+	// spec union (Or_Hover_contents) so MarkedString / []MarkedString (both
+	// deprecated, still sent by e.g. jdtls) decode alongside MarkupContent.
+	// Forcing "MarkupContent" here would make jdtls hover decode to empty.
+	{"InlayHint", "label"}: "[]InlayHintLabelPart",
 
 	{"RelatedFullDocumentDiagnosticReport", "relatedDocuments"}:      "map[DocumentUri]interface{}",
 	{"RelatedUnchangedDocumentDiagnosticReport", "relatedDocuments"}: "map[DocumentUri]interface{}",

go.mod

@@ -3,16 +3,16 @@ module github.com/isaacphi/mcp-language-server
 go 1.24.0
 
 require (
-	github.com/davecgh/go-spew v1.1.1
 	github.com/fsnotify/fsnotify v1.9.0
 	github.com/mark3labs/mcp-go v0.25.0
 	github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06
 	github.com/stretchr/testify v1.10.0
-	golang.org/x/text v0.25.0
+	golang.org/x/text v0.24.0
 )
 
 require (
 	github.com/BurntSushi/toml v1.4.1-0.20240526193622-a339e1f7089c // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/google/go-cmp v0.7.0 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/kisielk/errcheck v1.9.0 // indirect
@@ -22,7 +22,7 @@ require (
 	github.com/yosida95/uritemplate/v3 v3.0.2 // indirect
 	golang.org/x/exp/typeparams v0.0.0-20250210185358-939b2ce775ac // indirect
 	golang.org/x/mod v0.24.0 // indirect
-	golang.org/x/sync v0.14.0 // indirect
+	golang.org/x/sync v0.13.0 // indirect
 	golang.org/x/sys v0.31.0 // indirect
 	golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7 // indirect
 	golang.org/x/tools v0.31.0 // indirect

go.sum

@@ -44,14 +44,14 @@ golang.org/x/exp/typeparams v0.0.0-20250210185358-939b2ce775ac h1:TSSpLIG4v+p0rP
 golang.org/x/exp/typeparams v0.0.0-20250210185358-939b2ce775ac/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
 golang.org/x/mod v0.24.0 h1:ZfthKaKaT4NrhGVZHO1/WDTwGES4De8KtWO0SIbNJMU=
 golang.org/x/mod v0.24.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww=
-golang.org/x/sync v0.14.0 h1:woo0S4Yywslg6hp4eUFjTVOyKt0RookbpAHG4c1HmhQ=
-golang.org/x/sync v0.14.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
+golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610=
+golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
 golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik=
 golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
 golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7 h1:FemxDzfMUcK2f3YY4H+05K9CDzbSVr2+q/JKN45pey0=
 golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7/go.mod h1:pRgIJT+bRLFKnoM1ldnzKoxTIn14Yxz928LQRYYgIN0=
-golang.org/x/text v0.25.0 h1:qVyWApTSYLk/drJRO5mDlNYskwQznZmkpV2c8q9zls4=
-golang.org/x/text v0.25.0/go.mod h1:WEdwpYrmk1qmdHvhkSTNPm3app7v4rsT8F2UD6+VHIA=
+golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0=
+golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU=
 golang.org/x/tools v0.31.0 h1:0EedkvKDbh+qistFTd0Bcwe/YLh4vHwWEkiI0toFIBU=
 golang.org/x/tools v0.31.0/go.mod h1:naFTU+Cev749tSJRXJlna0T3WxKvb1kWEx15xA4SdmQ=
 golang.org/x/vuln v1.1.4 h1:Ju8QsuyhX3Hk8ma3CesTbO8vfJD9EvUBgHvkxHBzj0I=

internal/lsp/client.go

@@ -151,6 +151,11 @@ func (c *Client) InitializeLSPClient(ctx context.Context, workspaceDir string) (
 					Completion: protocol.CompletionClientCapabilities{
 						CompletionItem: protocol.ClientCompletionItemOptions{},
 					},
+					Hover: &protocol.HoverClientCapabilities{
+						// Prefer MarkupContent over deprecated MarkedString
+						// responses (e.g. from jdtls).
+						ContentFormat: []protocol.MarkupKind{protocol.Markdown, protocol.PlainText},
+					},
 					CodeLens: &protocol.CodeLensClientCapabilities{
 						DynamicRegistration: true,
 					},

internal/tools/definition.go

@@ -10,33 +10,17 @@ import (
 )
 
 func ReadDefinition(ctx context.Context, client *lsp.Client, symbolName string) (string, error) {
-	symbolResult, err := client.Symbol(ctx, protocol.WorkspaceSymbolParams{
-		Query: symbolName,
-	})
+	results, err := workspaceSymbols(ctx, client, symbolName)
 	if err != nil {
-		return "", fmt.Errorf("failed to fetch symbol: %v", err)
-	}
-
-	results, err := symbolResult.Results()
-	if err != nil {
-		return "", fmt.Errorf("failed to parse results: %v", err)
+		return "", err
 	}
 
 	var definitions []string
 	for _, symbol := range results {
-		kind := ""
-		container := ""
-
 		// Skip symbols that we are not looking for. workspace/symbol may return
 		// a large number of fuzzy matches.
 		switch v := symbol.(type) {
 		case *protocol.SymbolInformation:
-			// SymbolInformation results have richer data.
-			kind = fmt.Sprintf("Kind: %s\n", protocol.TableKindMap[v.Kind])
-			if v.ContainerName != "" {
-				container = fmt.Sprintf("Container Name: %s\n", v.ContainerName)
-			}
-
 			// Handle different matching strategies based on the search term
 			if strings.Contains(symbolName, ".") {
 				// For qualified names like "Type.Method", require exact match
@@ -62,38 +46,53 @@ func ReadDefinition(ctx context.Context, client *lsp.Client, symbolName string)
 		}
 
 		toolsLogger.Debug("Found symbol: %s", symbol.GetName())
-		loc := symbol.GetLocation()
 
-		err := client.OpenFile(ctx, loc.URI.Path())
+		symPath, err := uriToPath(symbol.GetLocation().URI)
 		if err != nil {
+			toolsLogger.Error("Error resolving symbol path: %v", err)
+			continue
+		}
+		if err := client.OpenFile(ctx, symPath); err != nil {
 			toolsLogger.Error("Error opening file: %v", err)
 			continue
 		}
 
-		banner := "---\n\n"
-		definition, loc, err := GetFullDefinition(ctx, client, loc)
-		locationInfo := fmt.Sprintf(
-			"Symbol: %s\n"+
-				"File: %s\n"+
-				kind+
-				container+
-				"Range: L%d:C%d - L%d:C%d\n\n",
-			symbol.GetName(),
-			strings.TrimPrefix(string(loc.URI), "file://"),
-			loc.Range.Start.Line+1,
-			loc.Range.Start.Character+1,
-			loc.Range.End.Line+1,
-			loc.Range.End.Character+1,
-		)
-
+		definition, loc, err := GetFullDefinition(ctx, client, symbol.GetLocation())
 		if err != nil {
 			toolsLogger.Error("Error getting definition: %v", err)
 			continue
 		}
 
-		definition = addLineNumbers(definition, int(loc.Range.Start.Line)+1)
+		// Extract kind/container for both SymbolInformation and WorkspaceSymbol
+		// results so the header is consistent regardless of which shape the
+		// server returns.
+		kind, container := symbolKindAndContainer(symbol)
+		definitions = append(definitions, formatDefinitionEntry(symbol.GetName(), kind, container, definition, loc))
+	}
 
-		definitions = append(definitions, banner+locationInfo+definition+"\n")
+	// Fallback for qualified names that workspace/symbol could not match
+	// directly. Some servers (e.g. jdtls) only index types, so fully
+	// qualified names ("com.example.Foo") and member symbols
+	// ("Class.method") never appear in workspace/symbol results.
+	if len(definitions) == 0 && strings.Contains(symbolName, ".") {
+		entries, err := resolveQualifiedEntries(ctx, client, symbolName, func(sym resolvedSymbol) ([]string, error) {
+			symPath, err := uriToPath(sym.Location.URI)
+			if err != nil {
+				return nil, err
+			}
+			if err := client.OpenFile(ctx, symPath); err != nil {
+				return nil, fmt.Errorf("error opening file: %v", err)
+			}
+			definition, loc, err := GetFullDefinition(ctx, client, sym.Location)
+			if err != nil {
+				return nil, fmt.Errorf("error getting definition: %v", err)
+			}
+			return []string{formatDefinitionEntry(sym.Name, sym.Kind, sym.ContainerName, definition, loc)}, nil
+		})
+		if err != nil {
+			return "", err
+		}
+		definitions = append(definitions, entries...)
 	}
 
 	if len(definitions) == 0 {
@@ -102,3 +101,35 @@ func ReadDefinition(ctx context.Context, client *lsp.Client, symbolName string)
 
 	return strings.Join(definitions, ""), nil
 }
+
+// formatDefinitionEntry renders a single definition block. It is shared by the
+// workspace/symbol path and the qualified-name fallback so the header format
+// stays in one place. kindName/containerName are raw names; their labels are
+// emitted only when non-empty.
+func formatDefinitionEntry(name, kindName, containerName, definition string, loc protocol.Location) string {
+	var b strings.Builder
+	fmt.Fprintf(&b, "Symbol: %s\n", name)
+	// Display the filesystem path for file:// URIs; for anything else (e.g. a
+	// jdt:// URI from jdtls) fall back to the raw URI rather than panicking.
+	displayPath, err := uriToPath(loc.URI)
+	if err != nil {
+		displayPath = string(loc.URI)
+	}
+	fmt.Fprintf(&b, "File: %s\n", displayPath)
+	// kindName/containerName are server-provided; pass them as args (never as
+	// part of the format string) so a '%' in a name is not read as a verb.
+	if kindName != "" {
+		fmt.Fprintf(&b, "Kind: %s\n", kindName)
+	}
+	if containerName != "" {
+		fmt.Fprintf(&b, "Container Name: %s\n", containerName)
+	}
+	fmt.Fprintf(&b, "Range: L%d:C%d - L%d:C%d\n\n",
+		loc.Range.Start.Line+1,
+		loc.Range.Start.Character+1,
+		loc.Range.End.Line+1,
+		loc.Range.End.Character+1,
+	)
+
+	return "---\n\n" + b.String() + addLineNumbers(definition, int(loc.Range.Start.Line)+1) + "\n"
+}

internal/tools/hover.go

@@ -38,29 +38,54 @@ func GetHoverInfo(ctx context.Context, client *lsp.Client, filePath string, line
 
 	var result strings.Builder
 
-	// Process the hover contents based on Markup content
-	if hoverResult.Contents.Value == "" {
-		// Extract the line where the hover was requested
-		lineText, err := ExtractTextFromLocation(protocol.Location{
-			URI: uri,
-			Range: protocol.Range{
-				Start: protocol.Position{
-					Line:      position.Line,
-					Character: 0,
-				},
-				End: protocol.Position{
-					Line:      position.Line + 1,
-					Character: 0,
-				},
-			},
-		})
+	// Process the hover contents: MarkedString | []MarkedString | MarkupContent
+	contents := renderHoverContents(hoverResult.Contents)
+	if contents == "" {
+		// Extract the line where the hover was requested. Use a single-line
+		// range (start and end on the same line) so this also works when the
+		// symbol is on the file's last line; spanning to the start of the next
+		// line would push End.Line past the last index and fail the bounds
+		// check in ExtractTextFromLocation.
+		lineText, err := extractLineText(uri, position.Line)
 		if err != nil {
 			toolsLogger.Warn("failed to extract line at position: %v", err)
 		}
 		result.WriteString(fmt.Sprintf("No hover information available for this position on the following line:\n%s", lineText))
 	} else {
-		result.WriteString(hoverResult.Contents.Value)
+		result.WriteString(contents)
 	}
 
 	return result.String(), nil
 }
+
+// renderHoverContents converts any of the three hover content shapes
+// (MarkupContent, MarkedString, []MarkedString) to a markdown string.
+func renderHoverContents(contents protocol.Or_Hover_contents) string {
+	switch v := contents.Value.(type) {
+	case protocol.MarkupContent:
+		return v.Value
+	case protocol.MarkedString:
+		return renderMarkedString(v)
+	case []protocol.MarkedString:
+		parts := make([]string, 0, len(v))
+		for _, ms := range v {
+			if s := renderMarkedString(ms); s != "" {
+				parts = append(parts, s)
+			}
+		}
+		return strings.Join(parts, "\n\n")
+	}
+	return ""
+}
+
+// renderMarkedString converts a MarkedString (plain string or
+// {language, value} code block) to markdown.
+func renderMarkedString(ms protocol.MarkedString) string {
+	switch v := ms.Value.(type) {
+	case string:
+		return v
+	case protocol.MarkedStringWithLanguage:
+		return fmt.Sprintf("```%s\n%s\n```", v.Language, v.Value)
+	}
+	return ""
+}

internal/tools/hover_test.go

@@ -0,0 +1,92 @@
+package tools
+
+import (
+	"testing"
+
+	"github.com/isaacphi/mcp-language-server/internal/protocol"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestRenderHoverContents covers the three hover content shapes the renderer
+// must handle: MarkupContent ({kind, value}), a plain-string MarkedString, and
+// a []MarkedString containing a MarkedStringWithLanguage (which must be wrapped
+// in a language-tagged code fence). These are the shapes jdtls and other
+// servers emit, so the fix's formatting is asserted here without a live LSP.
+func TestRenderHoverContents(t *testing.T) {
+	tests := []struct {
+		name     string
+		contents protocol.Or_Hover_contents
+		want     string
+	}{
+		{
+			name: "MarkupContent returns its value verbatim",
+			contents: protocol.Or_Hover_contents{Value: protocol.MarkupContent{
+				Kind:  protocol.Markdown,
+				Value: "# Title\nbody",
+			}},
+			want: "# Title\nbody",
+		},
+		{
+			name: "plain-string MarkedString returns the string",
+			contents: protocol.Or_Hover_contents{Value: protocol.MarkedString{
+				Value: "just a string",
+			}},
+			want: "just a string",
+		},
+		{
+			name: "[]MarkedString with MarkedStringWithLanguage is code-fenced",
+			contents: protocol.Or_Hover_contents{Value: []protocol.MarkedString{
+				{Value: protocol.MarkedStringWithLanguage{Language: "java", Value: "int x"}},
+				{Value: "documentation text"},
+			}},
+			want: "```java\nint x\n```\n\ndocumentation text",
+		},
+		{
+			// A top-level MarkedStringWithLanguage (not inside a slice) goes
+			// through the distinct `case protocol.MarkedString` branch and must
+			// still be wrapped in a language-tagged code fence.
+			name: "top-level MarkedStringWithLanguage is code-fenced",
+			contents: protocol.Or_Hover_contents{Value: protocol.MarkedString{
+				Value: protocol.MarkedStringWithLanguage{Language: "java", Value: "int x"},
+			}},
+			want: "```java\nint x\n```",
+		},
+		{
+			// Empty []MarkedString entries are dropped so they do not produce
+			// stray blank separators in the joined output.
+			name: "[]MarkedString drops empty entries",
+			contents: protocol.Or_Hover_contents{Value: []protocol.MarkedString{
+				{Value: "first"},
+				{Value: ""},
+				{Value: "second"},
+			}},
+			want: "first\n\nsecond",
+		},
+		{
+			name:     "unknown shape returns empty string",
+			contents: protocol.Or_Hover_contents{Value: 42},
+			want:     "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := renderHoverContents(tt.contents)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// TestRenderMarkedString covers the two MarkedString variants directly: a plain
+// string passes through unchanged, and a MarkedStringWithLanguage becomes a
+// language-tagged code fence.
+func TestRenderMarkedString(t *testing.T) {
+	assert.Equal(t, "plain", renderMarkedString(protocol.MarkedString{Value: "plain"}))
+	assert.Equal(t,
+		"```go\nfunc f()\n```",
+		renderMarkedString(protocol.MarkedString{
+			Value: protocol.MarkedStringWithLanguage{Language: "go", Value: "func f()"},
+		}),
+	)
+	assert.Equal(t, "", renderMarkedString(protocol.MarkedString{Value: 0}))
+}