From ff37be1f17fd7ab6af414d47b464b56793150db4 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 19 Jun 2026 18:34:09 +0100 Subject: [PATCH] fix(lsp): parse LocationLink in definition results We advertise textDocument.definition.linkSupport=true, so gopls and rust-analyzer return the LocationLink shape (targetUri/targetRange). The old code only tried []Location then a single Location; unmarshaling LocationLink JSON into []Location silently succeeds with zero-value URIs/ranges, producing garbage locations (or a false 'No definition found'). parseDefinitionResult now decodes a unified shape capturing both field sets and normalizes to Location, and surfaces a real parse error instead of swallowing it. Adds TestParseDefinitionResult covering all shapes. --- internal/lsp/definition_parse_test.go | 78 +++++++++++++++++++++++++++ internal/lsp/manager.go | 54 ++++++++++++++++--- 2 files changed, 124 insertions(+), 8 deletions(-) create mode 100644 internal/lsp/definition_parse_test.go diff --git a/internal/lsp/definition_parse_test.go b/internal/lsp/definition_parse_test.go new file mode 100644 index 0000000..39b6050 --- /dev/null +++ b/internal/lsp/definition_parse_test.go @@ -0,0 +1,78 @@ +package lsp + +import ( + "encoding/json" + "testing" +) + +// TestParseDefinitionResult covers every LSP definition result shape: single +// Location, Location[], single LocationLink, LocationLink[], plus empty/error +// cases. The LocationLink shapes are the regression target — we advertise +// LinkSupport, so gopls/rust-analyzer return targetUri/targetRange, which a +// naive []Location unmarshal would silently turn into zero-value locations. +func TestParseDefinitionResult(t *testing.T) { + tests := []struct { + name string + raw string + wantURIs []string + wantErr bool + }{ + { + name: "single Location", + raw: `{"uri":"file:///a.go","range":{"start":{"line":4,"character":2},"end":{"line":4,"character":8}}}`, + wantURIs: []string{"file:///a.go"}, + }, + { + name: "Location array", + raw: `[{"uri":"file:///a.go","range":{"start":{"line":1,"character":0},"end":{"line":1,"character":3}}},{"uri":"file:///b.go","range":{"start":{"line":2,"character":0},"end":{"line":2,"character":3}}}]`, + wantURIs: []string{"file:///a.go", "file:///b.go"}, + }, + { + name: "single LocationLink", + raw: `{"targetUri":"file:///c.go","targetRange":{"start":{"line":9,"character":0},"end":{"line":12,"character":1}}}`, + wantURIs: []string{"file:///c.go"}, + }, + { + name: "LocationLink array", + raw: `[{"targetUri":"file:///d.go","targetRange":{"start":{"line":0,"character":0},"end":{"line":0,"character":5}}}]`, + wantURIs: []string{"file:///d.go"}, + }, + { + name: "empty array", + raw: `[]`, + wantURIs: []string{}, + }, + { + name: "garbage", + raw: `"not-a-location"`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + locs, err := parseDefinitionResult(json.RawMessage(tt.raw)) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got locations %+v", locs) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(locs) != len(tt.wantURIs) { + t.Fatalf("got %d locations, want %d (%+v)", len(locs), len(tt.wantURIs), locs) + } + for i, want := range tt.wantURIs { + if locs[i].URI != want { + t.Errorf("location[%d].URI = %q, want %q", i, locs[i].URI, want) + } + } + // LocationLink ranges must be carried over, not zeroed. + if tt.name == "single LocationLink" && locs[0].Range.Start.Line != 9 { + t.Errorf("LocationLink targetRange not preserved: got start line %d, want 9", locs[0].Range.Start.Line) + } + }) + } +} diff --git a/internal/lsp/manager.go b/internal/lsp/manager.go index 50dbdcc..2db0309 100644 --- a/internal/lsp/manager.go +++ b/internal/lsp/manager.go @@ -358,17 +358,55 @@ func (m *Manager) Definition(ctx context.Context, file string, line, col int) ([ return nil, nil } - // Result can be Location, []Location, or []LocationLink - var locations []Location - if err := json.Unmarshal(resp.Result, &locations); err != nil { - // Try single location - var single Location - if err := json.Unmarshal(resp.Result, &single); err == nil { - locations = []Location{single} + return parseDefinitionResult(resp.Result) +} + +// parseDefinitionResult decodes a textDocument/definition result. Per LSP the +// result may be Location, Location[], LocationLink, or LocationLink[]. Because +// we advertise LinkSupport (see initialize), servers such as gopls and +// rust-analyzer return the LocationLink shape (targetUri/targetRange). A naive +// unmarshal into []Location silently succeeds with zero-value URIs/ranges, so +// decode into a unified shape that captures both and normalize to Location. +func parseDefinitionResult(raw json.RawMessage) ([]Location, error) { + // locOrLink captures both the Location (uri/range) and LocationLink + // (targetUri/targetRange) field sets; whichever is populated wins. + type locOrLink struct { + URI string `json:"uri"` + Range Range `json:"range"` + TargetURI string `json:"targetUri"` + TargetRange Range `json:"targetRange"` + } + toLocation := func(l locOrLink) (Location, bool) { + if l.URI != "" { + return Location{URI: l.URI, Range: l.Range}, true } + if l.TargetURI != "" { + return Location{URI: l.TargetURI, Range: l.TargetRange}, true + } + return Location{}, false } - return locations, nil + // Array form (Location[] or LocationLink[]). + var arr []locOrLink + if err := json.Unmarshal(raw, &arr); err == nil { + locations := make([]Location, 0, len(arr)) + for _, l := range arr { + if loc, ok := toLocation(l); ok { + locations = append(locations, loc) + } + } + return locations, nil + } + + // Single object form (Location or LocationLink). + var single locOrLink + if err := json.Unmarshal(raw, &single); err != nil { + return nil, fmt.Errorf("failed to parse definition result: %w", err) + } + if loc, ok := toLocation(single); ok { + return []Location{loc}, nil + } + return nil, nil } // References finds all references to the symbol at the given position.