From cc1661d295a187946340a709d9ea465dbfcdd02f Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 19 Jun 2026 21:07:57 +0100 Subject: [PATCH] feat(lsp): query by symbol_name; document ast_query match limits Two ast_query/lsp_query ergonomics improvements that share the tool registration and help files: - lsp_query accepts symbol_name (+symbol_kind) and resolves the position via AST (resolveLSPSymbolPosition), so callers need not know line:col; line/column are no longer required. SymbolCandidate gains NameLine/NameColumn for the precise identifier position. Ambiguous/unknown names error with candidates / a 'did you mean' hint. Adds TestResolveLSPSymbolPosition. - ast_query help + tool description now document that matching is keyword/kind-based, not structural: pattern arg/return constraints are not enforced; use name_exact/name_matches/kind_in to filter. Fixes a misleading 'functions returning error' example. --- internal/parser/symbols.go | 21 ++++++---- internal/server/handlers_lsp.go | 56 ++++++++++++++++++++----- internal/server/handlers_lsp_test.go | 63 ++++++++++++++++++++++++++++ internal/server/help_content.go | 16 ++++++- internal/server/server.go | 16 ++++--- 5 files changed, 149 insertions(+), 23 deletions(-) diff --git a/internal/parser/symbols.go b/internal/parser/symbols.go index 6b3538b..819e86b 100644 --- a/internal/parser/symbols.go +++ b/internal/parser/symbols.go @@ -919,11 +919,15 @@ func kindMatchesNode(kind protocol.SymbolKind, nodeType string) bool { } // SymbolCandidate is one symbol matching a name (and optional kind) lookup, -// with its declaration node type and 1-indexed inclusive line range. +// with its declaration node type and 1-indexed inclusive line range. NameLine +// and NameColumn point at the name identifier itself (1-indexed), which is the +// position LSP queries (hover/definition/references) expect. type SymbolCandidate struct { - NodeType string - StartLine int - EndLine int + NodeType string + StartLine int + EndLine int + NameLine int + NameColumn int } // symbolDeclNodeTypes is the set of declaration node types FindSymbolCandidates @@ -959,10 +963,13 @@ func FindSymbolCandidates(tree *sitter.Tree, content []byte, filename, symbolNam return true // kind mismatch, keep searching } r := NodeRange(n, filename) + nameLoc := NodeLocation(nameNode, filename) candidates = append(candidates, SymbolCandidate{ - NodeType: n.Type(), - StartLine: r.Start.Line, - EndLine: r.End.Line, + NodeType: n.Type(), + StartLine: r.Start.Line, + EndLine: r.End.Line, + NameLine: nameLoc.Line, + NameColumn: nameLoc.Column, }) return true }) diff --git a/internal/server/handlers_lsp.go b/internal/server/handlers_lsp.go index 32b505e..3e5a99d 100644 --- a/internal/server/handlers_lsp.go +++ b/internal/server/handlers_lsp.go @@ -10,6 +10,7 @@ import ( "github.com/lukaszraczylo/mcp-filepuff/internal/lsp" "github.com/lukaszraczylo/mcp-filepuff/internal/parser" "github.com/lukaszraczylo/mcp-filepuff/pkg/errors" + "github.com/lukaszraczylo/mcp-filepuff/pkg/protocol" "github.com/mark3labs/mcp-go/mcp" ) @@ -26,20 +27,30 @@ func (s *Server) handleLSPQuery(ctx context.Context, request mcp.CallToolRequest return mcp.NewToolResultError("file is required"), nil } - line := request.GetInt("line", 0) - if line <= 0 { - return mcp.NewToolResultError("line must be positive"), nil - } - - col := request.GetInt("column", 0) - if col <= 0 { - return mcp.NewToolResultError("column must be positive"), nil - } - if !s.cfg.IsPathAllowed(file) { return mcp.NewToolResultError("file is outside workspace root"), nil } + // Position can be given directly (line+column) or resolved from a symbol name. + var line, col int + if symbolName := request.GetString("symbol_name", ""); symbolName != "" { + symbolKind := protocol.SymbolKind(request.GetString("symbol_kind", "")) + resolvedLine, resolvedCol, errResult := s.resolveLSPSymbolPosition(ctx, file, symbolName, symbolKind) + if errResult != nil { + return errResult, nil + } + line, col = resolvedLine, resolvedCol + } else { + line = request.GetInt("line", 0) + if line <= 0 { + return mcp.NewToolResultError("line must be positive (or provide symbol_name)"), nil + } + col = request.GetInt("column", 0) + if col <= 0 { + return mcp.NewToolResultError("column must be positive (or provide symbol_name)"), nil + } + } + verbose := request.GetBool("verbose", false) switch action { @@ -76,6 +87,31 @@ func (s *Server) handleLSPQuery(ctx context.Context, request mcp.CallToolRequest } } +// resolveLSPSymbolPosition resolves a symbol name to the 1-indexed line:column of +// its name identifier, for use as an LSP position. Returns an error result when the +// file can't be read/parsed, the symbol is not found (with a "did you mean" hint), +// or the name is ambiguous (listing candidates). +func (s *Server) resolveLSPSymbolPosition(ctx context.Context, file, symbolName string, symbolKind protocol.SymbolKind) (line, col int, errResult *mcp.CallToolResult) { + content, err := os.ReadFile(file) + if err != nil { + return 0, 0, mcp.NewToolResultError(fmt.Sprintf("failed to read file: %s", errors.SanitizeError(err))) + } + result, err := s.parser.Parse(ctx, file, content) + if err != nil { + return 0, 0, mcp.NewToolResultError(fmt.Sprintf("failed to parse file: %s", errors.SanitizeError(err))) + } + candidates := parser.FindSymbolCandidates(result.Tree, content, file, symbolName, symbolKind) + switch len(candidates) { + case 0: + return 0, 0, mcp.NewToolResultError(fmt.Sprintf("symbol %q not found in %s%s", symbolName, file, s.nearestSymbolSuggestion(ctx, file, content, symbolName))) + case 1: + return candidates[0].NameLine, candidates[0].NameColumn, nil + default: + return 0, 0, mcp.NewToolResultError(fmt.Sprintf("symbol %q is ambiguous in %s (%d matches): %s — disambiguate with symbol_kind, or pass line+column", + symbolName, file, len(candidates), formatSymbolCandidates(candidates))) + } +} + // lspHover performs hover (symbol info) for the given position. func (s *Server) lspHover(ctx context.Context, file string, line, col int, verbose bool) (*mcp.CallToolResult, error) { hover, err := s.lspManager.Hover(ctx, file, line, col) diff --git a/internal/server/handlers_lsp_test.go b/internal/server/handlers_lsp_test.go index 57ff408..05adaae 100644 --- a/internal/server/handlers_lsp_test.go +++ b/internal/server/handlers_lsp_test.go @@ -1,12 +1,25 @@ package server import ( + "context" "strings" "testing" "github.com/lukaszraczylo/mcp-filepuff/internal/lsp" + "github.com/mark3labs/mcp-go/mcp" ) +// errorText extracts the text payload from an error CallToolResult. +func errorText(res *mcp.CallToolResult) string { + if res == nil || len(res.Content) == 0 { + return "" + } + if tc, ok := res.Content[0].(mcp.TextContent); ok { + return tc.Text + } + return "" +} + // makeLocation builds an lsp.Location with a file:// URI. func makeLocation(file string, line, col int) lsp.Location { return lsp.Location{ @@ -179,3 +192,53 @@ func TestFormatReferencesVerbosePreservesLPrefix(t *testing.T) { t.Errorf("verbose output must contain L3:4, got:\n%s", out) } } + +// resolveLSPSymbolPosition underpins lsp_query's symbol_name affordance: it maps +// a symbol name to the line:column of its name identifier, or returns an error +// result for not-found / ambiguous names. It does not require a running LSP. +func TestResolveLSPSymbolPosition(t *testing.T) { + tmpDir := t.TempDir() + srv := newTestServer(t, tmpDir) + + goSrc := "package main\n" + // 1 + "\n" + // 2 + "type A struct{}\n" + // 3 + "func (a A) Run() {}\n" + // 4 + "\n" + // 5 + "type B struct{}\n" + // 6 + "func (b B) Run() {}\n" + // 7 + "\n" + // 8 + "func Unique() {}\n" // 9 + f := writeFile(t, tmpDir, "main.go", goSrc) + ctx := context.Background() + + t.Run("unique symbol resolves to its name position", func(t *testing.T) { + line, col, errRes := srv.resolveLSPSymbolPosition(ctx, f, "Unique", "") + if errRes != nil { + t.Fatalf("unexpected error result for Unique: %s", errorText(errRes)) + } + if line != 9 || col != 6 { + t.Errorf("Unique position = %d:%d, want 9:6", line, col) + } + }) + + t.Run("ambiguous symbol errors", func(t *testing.T) { + _, _, errRes := srv.resolveLSPSymbolPosition(ctx, f, "Run", "") + if errRes == nil { + t.Fatal("expected ambiguity error for Run") + } + if !strings.Contains(errorText(errRes), "ambiguous") { + t.Errorf("expected 'ambiguous' in error, got: %s", errorText(errRes)) + } + }) + + t.Run("missing symbol errors", func(t *testing.T) { + _, _, errRes := srv.resolveLSPSymbolPosition(ctx, f, "Nope", "") + if errRes == nil { + t.Fatal("expected not-found error for Nope") + } + if !strings.Contains(errorText(errRes), "not found") { + t.Errorf("expected 'not found' in error, got: %s", errorText(errRes)) + } + }) +} diff --git a/internal/server/help_content.go b/internal/server/help_content.go index 2c51175..5d30dbe 100644 --- a/internal/server/help_content.go +++ b/internal/server/help_content.go @@ -88,9 +88,17 @@ const helpASTQuery = "# ast_query — flags and examples\n\n" + "| `$NAME` | Matches a single node, captures as `$NAME` |\n" + "| `$$$ARGS` | Matches zero or more nodes (variadic capture) |\n" + "| `$_` | Wildcard — matches any single node, no capture |\n\n" + + "## Matching semantics (important)\n\n" + + "Matching is keyword- and kind-based, not full structural matching. The leading keyword " + + "(`func`/`function`/`class`/`struct`/`interface`) plus placeholders select the node KIND; " + + "argument shapes, return types, and the literal bound to `$NAME` are NOT enforced as filters. " + + "So `func $NAME($$$ARGS) error` matches every function/method, not only those returning `error`. " + + "Captured `$VAR` values appear in output but do not constrain which nodes match.\n\n" + + "For precise selection, combine the pattern with `name_exact`, `name_matches`, and/or `kind_in` — " + + "these ARE applied as filters.\n\n" + "## Examples\n\n" + "```json\n" + - "// All Go functions returning error\n" + + "// All Go functions/methods (the 'error' return is NOT enforced — add name_/kind_in to narrow)\n" + `{"pattern": "func $NAME($$$ARGS) error", "language": "go"}` + "\n\n" + "// Python classes\n" + `{"pattern": "class $NAME: $$$BODY", "language": "python"}` + "\n\n" + @@ -114,10 +122,16 @@ const helpLSPQuery = "# lsp_query — flags and examples\n\n" + "- `compact=true` — collapse to one line per file\n" + "- `verbose=true` — add `Found N reference(s):` header\n\n" + "Note: `include_declaration` and `compact` are errors when used with actions other than `references`.\n\n" + + "## Position: line+column or symbol_name\n\n" + + "Give the position directly with `line`+`column`, or pass `symbol_name` (optionally `symbol_kind`) to " + + "resolve it from the AST — handy when you know the name but not the coordinates. Ambiguous or unknown " + + "names return an error (with candidates / a 'did you mean' hint).\n\n" + "## Examples\n\n" + "```json\n" + "// Hover — type/doc at position\n" + `{"action": "hover", "file": "server.go", "line": 45, "column": 6}` + "\n\n" + + "// Definition by symbol name — no line/column needed\n" + + `{"action": "definition", "file": "server.go", "symbol_name": "NewServer"}` + "\n\n" + "// Definition — where is this symbol defined?\n" + `{"action": "definition", "file": "handler.go", "line": 23, "column": 10}` + "\n\n" + "// References — all usages\n" + diff --git a/internal/server/server.go b/internal/server/server.go index 9c68624..8afaf9a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -222,6 +222,7 @@ func (s *Server) registerTools() { s.mcp.AddTool( mcp.NewTool("ast_query", mcp.WithDescription("Search for AST patterns in code files. Use code patterns with $VAR placeholders to match and capture code structures like functions, classes, and types. "+ + "Matching is keyword/kind-based, not fully structural: argument and return-type constraints in the pattern are not enforced — use name_exact/name_matches/kind_in to filter precisely. "+ "See resource filepuff://help/ast_query for flags and examples."), mcp.WithReadOnlyHintAnnotation(true), mcp.WithString("pattern", @@ -266,7 +267,8 @@ func (s *Server) registerTools() { if s.lspManager != nil { s.mcp.AddTool( mcp.NewTool("lsp_query", - mcp.WithDescription("Query LSP for symbol info, definition, or references at a specific file position. "+ + mcp.WithDescription("Query LSP for symbol info, definition, or references at a file position. "+ + "Give the position as line+column, or pass symbol_name to resolve it via AST (no line/column needed). "+ "See resource filepuff://help/lsp_query for flags and examples."), mcp.WithReadOnlyHintAnnotation(true), mcp.WithString("action", @@ -277,13 +279,17 @@ func (s *Server) registerTools() { mcp.Required(), mcp.Description("Path to the file"), ), + mcp.WithString("symbol_name", + mcp.Description("Resolve the position from a named symbol (function, struct, class, etc.) instead of line+column. Errors if the name is ambiguous or not found."), + ), + mcp.WithString("symbol_kind", + mcp.Description("Disambiguate symbol_name by kind when multiple symbols share the name. Accepted: function, method, struct, class, interface, type, enum, trait, constant, module."), + ), mcp.WithNumber("line", - mcp.Required(), - mcp.Description("Line number (1-indexed)"), + mcp.Description("Line number (1-indexed). Required unless symbol_name is given."), ), mcp.WithNumber("column", - mcp.Required(), - mcp.Description("Column number (1-indexed)"), + mcp.Description("Column number (1-indexed). Required unless symbol_name is given."), ), mcp.WithBoolean("include_declaration", mcp.Description("Include the declaration in results. Only valid for action=references (default: true)."),