From c261fb8c76a26df6f338ed84b89c71eebe54f1ae Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 19 Jun 2026 18:34:18 +0100 Subject: [PATCH] fix(parser): detect ambiguous symbol_name reads FindSymbolRange returned the first AST node matching a name, so file_read with symbol_name silently read the wrong symbol when a name collided (e.g. two String() methods on different receivers, or a func and type sharing a name). New FindSymbolCandidates returns all matches; the file_read handler now errors on ambiguity, listing each candidate's node type and line range so the caller can disambiguate via symbol_kind or line_start/line_end. FindSymbolRange is kept (first match) for callers that don't need ambiguity detection. Adds TestFindSymbolCandidates. --- internal/parser/symbols.go | 71 +++++++++++++++++++++----------- internal/parser/symbols_test.go | 58 ++++++++++++++++++++++++++ internal/server/handlers_file.go | 36 ++++++++++++---- 3 files changed, 133 insertions(+), 32 deletions(-) diff --git a/internal/parser/symbols.go b/internal/parser/symbols.go index 494ab20..9bcb524 100644 --- a/internal/parser/symbols.go +++ b/internal/parser/symbols.go @@ -910,17 +910,33 @@ func kindMatchesNode(kind protocol.SymbolKind, nodeType string) bool { return true } -// FindSymbolRange finds a named symbol in the AST and returns its line range (1-indexed, inclusive). -// symbolKind filters by symbol kind (e.g. "function", "struct"); empty string matches any kind. -// Returns (0, 0, false) if the symbol is not found. -func FindSymbolRange(tree *sitter.Tree, content []byte, filename, symbolName string, symbolKind protocol.SymbolKind) (startLine, endLine int, found bool) { +// SymbolCandidate is one symbol matching a name (and optional kind) lookup, +// with its declaration node type and 1-indexed inclusive line range. +type SymbolCandidate struct { + NodeType string + StartLine int + EndLine int +} + +// symbolDeclNodeTypes is the set of declaration node types FindSymbolCandidates +// treats as nameable symbols across supported languages. +var symbolDeclNodeTypes = map[string]bool{ + "function_declaration": true, "method_declaration": true, "type_declaration": true, + "function_definition": true, "class_definition": true, "class_declaration": true, + "interface_declaration": true, "type_alias_declaration": true, + "function_item": true, "struct_item": true, "enum_item": true, "trait_item": true, + "type_item": true, "const_item": true, "mod_item": true, +} + +// FindSymbolCandidates returns every declaration whose name equals symbolName and +// whose kind matches symbolKind (empty matches any). Multiple results indicate an +// ambiguous lookup (e.g. two methods named String on different receivers). +func FindSymbolCandidates(tree *sitter.Tree, content []byte, filename, symbolName string, symbolKind protocol.SymbolKind) []SymbolCandidate { if tree == nil || symbolName == "" { - return + return nil } + var candidates []SymbolCandidate WalkTree(tree.RootNode(), func(n *sitter.Node) bool { - if found { - return false - } nameNode := n.ChildByFieldName("name") if nameNode == nil { return true @@ -928,22 +944,31 @@ func FindSymbolRange(tree *sitter.Tree, content []byte, filename, symbolName str if GetNodeText(nameNode, content) != symbolName { return true } - switch n.Type() { - case "function_declaration", "method_declaration", "type_declaration", - "function_definition", "class_definition", "class_declaration", - "interface_declaration", "type_alias_declaration", - "function_item", "struct_item", "enum_item", "trait_item", - "type_item", "const_item", "mod_item": - if !kindMatchesNode(symbolKind, n.Type()) { - return true // kind mismatch, keep searching - } - r := NodeRange(n, filename) - startLine = r.Start.Line - endLine = r.End.Line - found = true - return false + if !symbolDeclNodeTypes[n.Type()] { + return true } + if !kindMatchesNode(symbolKind, n.Type()) { + return true // kind mismatch, keep searching + } + r := NodeRange(n, filename) + candidates = append(candidates, SymbolCandidate{ + NodeType: n.Type(), + StartLine: r.Start.Line, + EndLine: r.End.Line, + }) return true }) - return + return candidates +} + +// FindSymbolRange finds a named symbol in the AST and returns its line range (1-indexed, inclusive). +// symbolKind filters by symbol kind (e.g. "function", "struct"); empty string matches any kind. +// Returns (0, 0, false) if the symbol is not found. On ambiguity it returns the first match; +// callers that need to detect ambiguity should use FindSymbolCandidates. +func FindSymbolRange(tree *sitter.Tree, content []byte, filename, symbolName string, symbolKind protocol.SymbolKind) (startLine, endLine int, found bool) { + candidates := FindSymbolCandidates(tree, content, filename, symbolName, symbolKind) + if len(candidates) == 0 { + return 0, 0, false + } + return candidates[0].StartLine, candidates[0].EndLine, true } diff --git a/internal/parser/symbols_test.go b/internal/parser/symbols_test.go index ef61246..1bf796b 100644 --- a/internal/parser/symbols_test.go +++ b/internal/parser/symbols_test.go @@ -7,6 +7,64 @@ import ( "github.com/lukaszraczylo/mcp-filepuff/pkg/protocol" ) +func TestFindSymbolCandidates(t *testing.T) { + r := NewRegistry() + defer r.Close() + + // Two methods named String on different receivers, plus a unique func. + content := `package main + +type A struct{} + +func (a A) String() string { return "a" } + +type B struct{} + +func (b B) String() string { return "b" } + +func Unique() {} +` + ctx := context.Background() + result, err := r.Parse(ctx, "test.go", []byte(content)) + if err != nil { + t.Fatalf("parse failed: %v", err) + } + + t.Run("ambiguous name returns all candidates", func(t *testing.T) { + got := FindSymbolCandidates(result.Tree, []byte(content), "test.go", "String", "") + if len(got) != 2 { + t.Fatalf("got %d candidates for String, want 2: %+v", len(got), got) + } + if got[0].StartLine >= got[1].StartLine { + t.Errorf("candidates not in source order: %+v", got) + } + }) + + t.Run("unique name returns one candidate", func(t *testing.T) { + got := FindSymbolCandidates(result.Tree, []byte(content), "test.go", "Unique", "") + if len(got) != 1 { + t.Fatalf("got %d candidates for Unique, want 1: %+v", len(got), got) + } + }) + + t.Run("missing name returns none", func(t *testing.T) { + got := FindSymbolCandidates(result.Tree, []byte(content), "test.go", "Missing", "") + if len(got) != 0 { + t.Fatalf("got %d candidates for Missing, want 0", len(got)) + } + }) + + t.Run("FindSymbolRange returns first match", func(t *testing.T) { + start, end, found := FindSymbolRange(result.Tree, []byte(content), "test.go", "String", "") + if !found { + t.Fatal("expected String to be found") + } + if start <= 0 || end < start { + t.Errorf("invalid range: start=%d end=%d", start, end) + } + }) +} + func TestExtractGoSymbols(t *testing.T) { r := NewRegistry() defer r.Close() diff --git a/internal/server/handlers_file.go b/internal/server/handlers_file.go index 374f054..1b28240 100644 --- a/internal/server/handlers_file.go +++ b/internal/server/handlers_file.go @@ -440,12 +440,19 @@ func (s *Server) readOneFile(ctx context.Context, request mcp.CallToolRequest, p // Symbol-based line range: find the symbol and use its exact bounds. if symbolName != "" { symbolKind := protocol.SymbolKind(request.GetString("symbol_kind", "")) - start, end, found := s.resolveSymbolLines(ctx, path, content, symbolName, symbolKind) - if !found { + candidates := s.resolveSymbolCandidates(ctx, path, content, symbolName, symbolKind) + switch len(candidates) { + case 0: return "", fmt.Errorf("symbol %q not found in %s", symbolName, path) + case 1: + lineStart = candidates[0].StartLine + lineEnd = candidates[0].EndLine + default: + // Ambiguous: returning the first match risks reading the wrong + // symbol. List candidates so the caller can disambiguate. + return "", fmt.Errorf("symbol %q is ambiguous in %s (%d matches): %s — disambiguate with symbol_kind, or read a specific one with line_start/line_end", + symbolName, path, len(candidates), formatSymbolCandidates(candidates)) } - lineStart = start - lineEnd = end } // Clamp to valid range. @@ -500,14 +507,25 @@ func (s *Server) readOneFile(ctx context.Context, request mcp.CallToolRequest, p return output.String(), nil } -// resolveSymbolLines parses the AST and returns the line range of the named symbol. -// symbolKind optionally filters by kind (empty = any). -func (s *Server) resolveSymbolLines(ctx context.Context, path string, content []byte, symbolName string, symbolKind protocol.SymbolKind) (startLine, endLine int, found bool) { +// resolveSymbolCandidates parses the AST and returns every symbol matching the +// name (and optional kind). Zero results means not found; more than one means an +// ambiguous lookup. Returns nil on parse failure (treated as not found). +func (s *Server) resolveSymbolCandidates(ctx context.Context, path string, content []byte, symbolName string, symbolKind protocol.SymbolKind) []parser.SymbolCandidate { result, err := s.parser.Parse(ctx, path, content) if err != nil { - return + return nil } - return parser.FindSymbolRange(result.Tree, content, path, symbolName, symbolKind) + return parser.FindSymbolCandidates(result.Tree, content, path, symbolName, symbolKind) +} + +// formatSymbolCandidates renders ambiguous symbol matches as a compact, +// comma-separated list, e.g. "method_declaration L10-24, method_declaration L42-58". +func formatSymbolCandidates(candidates []parser.SymbolCandidate) string { + parts := make([]string, len(candidates)) + for i, c := range candidates { + parts[i] = fmt.Sprintf("%s L%d-%d", c.NodeType, c.StartLine, c.EndLine) + } + return strings.Join(parts, ", ") } // writeLines writes the selected line range into output, applying all formatting options.