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.
2026-10-12 04:54:27 +00:00 · 2026-06-20 00:33:31 +01:00
parent ff37be1f17
commit c261fb8c76
3 changed files with 133 additions and 32 deletions
@@ -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
 }
@@ -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()
@@ -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.