From 3b9babbc87819a6ec047c6f9ab11d92d45a6ad5d Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Wed, 27 Aug 2025 14:46:00 -0300 Subject: [PATCH] fix(lsp): simplify init/ping, store capabilities (#713) * fix(lsp): simplify init/ping Signed-off-by: Carlos Alexandro Becker * feat(lsp): store server capabilities Signed-off-by: Carlos Alexandro Becker * fix(lsp): improve init Signed-off-by: Carlos Alexandro Becker * fix(lsp): cancel request id Signed-off-by: Carlos Alexandro Becker --------- Signed-off-by: Carlos Alexandro Becker --- internal/lsp/caps.go | 112 +++++++++++++++++++++++ internal/lsp/client.go | 156 ++++++-------------------------- internal/lsp/transport.go | 11 ++- internal/lsp/watcher/watcher.go | 9 +- 4 files changed, 153 insertions(+), 135 deletions(-) create mode 100644 internal/lsp/caps.go diff --git a/internal/lsp/caps.go b/internal/lsp/caps.go new file mode 100644 index 0000000000000000000000000000000000000000..7edc0886f72a92183a8570e45db74218e3aead47 --- /dev/null +++ b/internal/lsp/caps.go @@ -0,0 +1,112 @@ +package lsp + +import "github.com/charmbracelet/crush/internal/lsp/protocol" + +func (c *Client) setCapabilities(caps protocol.ServerCapabilities) { + c.capsMu.Lock() + defer c.capsMu.Unlock() + c.caps = caps + c.capsSet.Store(true) +} + +func (c *Client) getCapabilities() (protocol.ServerCapabilities, bool) { + c.capsMu.RLock() + defer c.capsMu.RUnlock() + return c.caps, c.capsSet.Load() +} + +func (c *Client) IsMethodSupported(method string) bool { + // Always allow core lifecycle and generic methods + switch method { + case "initialize", "shutdown", "exit", "$/cancelRequest": + return true + } + + caps, ok := c.getCapabilities() + if !ok { + // caps not set yet, be permissive + return true + } + + switch method { + case "textDocument/hover": + return caps.HoverProvider != nil + case "textDocument/definition": + return caps.DefinitionProvider != nil + case "textDocument/references": + return caps.ReferencesProvider != nil + case "textDocument/implementation": + return caps.ImplementationProvider != nil + case "textDocument/typeDefinition": + return caps.TypeDefinitionProvider != nil + case "textDocument/documentColor", "textDocument/colorPresentation": + return caps.ColorProvider != nil + case "textDocument/foldingRange": + return caps.FoldingRangeProvider != nil + case "textDocument/declaration": + return caps.DeclarationProvider != nil + case "textDocument/selectionRange": + return caps.SelectionRangeProvider != nil + case "textDocument/prepareCallHierarchy", "callHierarchy/incomingCalls", "callHierarchy/outgoingCalls": + return caps.CallHierarchyProvider != nil + case "textDocument/semanticTokens/full", "textDocument/semanticTokens/full/delta", "textDocument/semanticTokens/range": + return caps.SemanticTokensProvider != nil + case "textDocument/linkedEditingRange": + return caps.LinkedEditingRangeProvider != nil + case "workspace/willCreateFiles": + return caps.Workspace != nil && caps.Workspace.FileOperations != nil && caps.Workspace.FileOperations.WillCreate != nil + case "workspace/willRenameFiles": + return caps.Workspace != nil && caps.Workspace.FileOperations != nil && caps.Workspace.FileOperations.WillRename != nil + case "workspace/willDeleteFiles": + return caps.Workspace != nil && caps.Workspace.FileOperations != nil && caps.Workspace.FileOperations.WillDelete != nil + case "textDocument/moniker": + return caps.MonikerProvider != nil + case "textDocument/prepareTypeHierarchy", "typeHierarchy/supertypes", "typeHierarchy/subtypes": + return caps.TypeHierarchyProvider != nil + case "textDocument/inlineValue": + return caps.InlineValueProvider != nil + case "textDocument/inlayHint", "inlayHint/resolve": + return caps.InlayHintProvider != nil + case "textDocument/diagnostic", "workspace/diagnostic": + return caps.DiagnosticProvider != nil + case "textDocument/inlineCompletion": + return caps.InlineCompletionProvider != nil + case "workspace/textDocumentContent": + return caps.Workspace != nil && caps.Workspace.TextDocumentContent != nil + case "textDocument/willSaveWaitUntil": + if caps.TextDocumentSync == nil { + return false + } + return true + case "textDocument/completion", "completionItem/resolve": + return caps.CompletionProvider != nil + case "textDocument/signatureHelp": + return caps.SignatureHelpProvider != nil + case "textDocument/documentHighlight": + return caps.DocumentHighlightProvider != nil + case "textDocument/documentSymbol": + return caps.DocumentSymbolProvider != nil + case "textDocument/codeAction", "codeAction/resolve": + return caps.CodeActionProvider != nil + case "workspace/symbol", "workspaceSymbol/resolve": + return caps.WorkspaceSymbolProvider != nil + case "textDocument/codeLens", "codeLens/resolve": + return caps.CodeLensProvider != nil + case "textDocument/documentLink", "documentLink/resolve": + return caps.DocumentLinkProvider != nil + case "textDocument/formatting": + return caps.DocumentFormattingProvider != nil + case "textDocument/rangeFormatting": + return caps.DocumentRangeFormattingProvider != nil + case "textDocument/rangesFormatting": + return caps.DocumentRangeFormattingProvider != nil + case "textDocument/onTypeFormatting": + return caps.DocumentOnTypeFormattingProvider != nil + case "textDocument/rename", "textDocument/prepareRename": + return caps.RenameProvider != nil + case "workspace/executeCommand": + return caps.ExecuteCommandProvider != nil + default: + return true + } +} diff --git a/internal/lsp/client.go b/internal/lsp/client.go index a6b9fcbb4caea4992fb2dbce6ddc6e75066c9da7..e09a6a446db2f62476e072c79daadd2d832f895b 100644 --- a/internal/lsp/client.go +++ b/internal/lsp/client.go @@ -62,6 +62,11 @@ type Client struct { // Server state serverState atomic.Value + + // Server capabilities as returned by initialize + caps protocol.ServerCapabilities + capsMu sync.RWMutex + capsSet atomic.Bool } // NewClient creates a new LSP client. @@ -143,7 +148,7 @@ func (c *Client) RegisterServerRequestHandler(method string, handler ServerReque } func (c *Client) InitializeLSPClient(ctx context.Context, workspaceDir string) (*protocol.InitializeResult, error) { - initParams := &protocol.InitializeParams{ + initParams := protocol.ParamInitialize{ WorkspaceFoldersInitializeParams: protocol.WorkspaceFoldersInitializeParams{ WorkspaceFolders: []protocol.WorkspaceFolder{ { @@ -220,12 +225,14 @@ func (c *Client) InitializeLSPClient(ctx context.Context, workspaceDir string) ( }, } - var result protocol.InitializeResult - if err := c.Call(ctx, "initialize", initParams, &result); err != nil { + result, err := c.Initialize(ctx, initParams) + if err != nil { return nil, fmt.Errorf("initialize failed: %w", err) } - if err := c.Notify(ctx, "initialized", struct{}{}); err != nil { + c.setCapabilities(result.Capabilities) + + if err := c.Initialized(ctx, protocol.InitializedParams{}); err != nil { return nil, fmt.Errorf("initialized notification failed: %w", err) } @@ -234,14 +241,9 @@ func (c *Client) InitializeLSPClient(ctx context.Context, workspaceDir string) ( c.RegisterServerRequestHandler("workspace/configuration", HandleWorkspaceConfiguration) c.RegisterServerRequestHandler("client/registerCapability", HandleRegisterCapability) c.RegisterNotificationHandler("window/showMessage", HandleServerMessage) - c.RegisterNotificationHandler("textDocument/publishDiagnostics", - func(params json.RawMessage) { HandleDiagnostics(c, params) }) - - // Notify the LSP server - err := c.Initialized(ctx, protocol.InitializedParams{}) - if err != nil { - return nil, fmt.Errorf("initialization failed: %w", err) - } + c.RegisterNotificationHandler("textDocument/publishDiagnostics", func(params json.RawMessage) { + HandleDiagnostics(c, params) + }) return &result, nil } @@ -329,16 +331,7 @@ func (c *Client) WaitForServerReady(ctx context.Context) error { slog.Debug("Waiting for LSP server to be ready...") } - // Determine server type for specialized initialization - serverType := c.detectServerType() - - // For TypeScript-like servers, we need to open some key files first - if serverType == ServerTypeTypeScript { - if cfg.Options.DebugLSP { - slog.Debug("TypeScript-like server detected, opening key configuration files") - } - c.openKeyConfigFiles(ctx) - } + c.openKeyConfigFiles(ctx) for { select { @@ -347,21 +340,19 @@ func (c *Client) WaitForServerReady(ctx context.Context) error { return fmt.Errorf("timeout waiting for LSP server to be ready") case <-ticker.C: // Try a ping method appropriate for this server type - err := c.pingServerByType(ctx, serverType) - if err == nil { - // Server responded successfully - c.SetServerState(StateReady) + if err := c.ping(ctx); err != nil { if cfg.Options.DebugLSP { - slog.Debug("LSP server is ready") + slog.Debug("LSP server not ready yet", "error", err, "server", c.name) } - return nil - } else { - slog.Debug("LSP server not ready yet", "error", err, "serverType", serverType) + continue } + // Server responded successfully + c.SetServerState(StateReady) if cfg.Options.DebugLSP { - slog.Debug("LSP server not ready yet", "error", err, "serverType", serverType) + slog.Debug("LSP server is ready") } + return nil } } } @@ -443,86 +434,13 @@ func (c *Client) openKeyConfigFiles(ctx context.Context) { } } -// pingServerByType sends a ping request appropriate for the server type -func (c *Client) pingServerByType(ctx context.Context, serverType ServerType) error { - switch serverType { - case ServerTypeTypeScript: - // For TypeScript, try a document symbol request on an open file - return c.pingTypeScriptServer(ctx) - case ServerTypeGo: - // For Go, workspace/symbol works well - return c.pingWithWorkspaceSymbol(ctx) - case ServerTypeRust: - // For Rust, workspace/symbol works well - return c.pingWithWorkspaceSymbol(ctx) - default: - // Default ping method - return c.pingWithWorkspaceSymbol(ctx) - } -} - -// pingTypeScriptServer tries to ping a TypeScript server with appropriate methods -func (c *Client) pingTypeScriptServer(ctx context.Context) error { - // First try workspace/symbol which works for many servers - if err := c.pingWithWorkspaceSymbol(ctx); err == nil { - return nil - } - - // If that fails, try to find an open file and request document symbols - c.openFilesMu.RLock() - defer c.openFilesMu.RUnlock() - - // If we have any open files, try to get document symbols for one - for uri := range c.openFiles { - filePath, err := protocol.DocumentURI(uri).Path() - if err != nil { - slog.Error("Failed to convert URI to path for TypeScript symbol collection", "uri", uri, "error", err) - continue - } - - if strings.HasSuffix(filePath, ".ts") || strings.HasSuffix(filePath, ".js") || - strings.HasSuffix(filePath, ".tsx") || strings.HasSuffix(filePath, ".jsx") { - var symbols []protocol.DocumentSymbol - err := c.Call(ctx, "textDocument/documentSymbol", protocol.DocumentSymbolParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: protocol.DocumentURI(uri), - }, - }, &symbols) - if err == nil { - return nil - } - } - } - - // If we have no open TypeScript files, try to find and open one - workDir := config.Get().WorkingDir() - err := filepath.WalkDir(workDir, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - - // Skip directories and non-TypeScript files - if d.IsDir() { - return nil - } - - ext := filepath.Ext(path) - if ext == ".ts" || ext == ".js" || ext == ".tsx" || ext == ".jsx" { - // Found a TypeScript file, try to open it - if err := c.OpenFile(ctx, path); err == nil { - // Successfully opened, stop walking - return filepath.SkipAll - } - } - +// ping sends a ping request... +func (c *Client) ping(ctx context.Context) error { + if _, err := c.Symbol(ctx, protocol.WorkspaceSymbolParams{}); err == nil { return nil - }) - if err != nil { - slog.Debug("Error walking directory for TypeScript files", "error", err) } - - // Final fallback - just try a generic capability - return c.pingWithServerCapabilities(ctx) + // This is a very lightweight request that should work for most servers + return c.Notify(ctx, "$/cancelRequest", protocol.CancelParams{ID: "1"}) } // openTypeScriptFiles finds and opens TypeScript files to help initialize the server @@ -597,20 +515,6 @@ func shouldSkipDir(path string) bool { return skipDirs[dirName] } -// pingWithWorkspaceSymbol tries a workspace/symbol request -func (c *Client) pingWithWorkspaceSymbol(ctx context.Context) error { - var result []protocol.SymbolInformation - return c.Call(ctx, "workspace/symbol", protocol.WorkspaceSymbolParams{ - Query: "", - }, &result) -} - -// pingWithServerCapabilities tries to get server capabilities -func (c *Client) pingWithServerCapabilities(ctx context.Context) error { - // This is a very lightweight request that should work for most servers - return c.Notify(ctx, "$/cancelRequest", struct{ ID int }{ID: -1}) -} - type OpenFileInfo struct { Version int32 URI protocol.DocumentURI @@ -668,7 +572,7 @@ func (c *Client) OpenFile(ctx context.Context, filepath string) error { }, } - if err := c.Notify(ctx, "textDocument/didOpen", params); err != nil { + if err := c.DidOpen(ctx, params); err != nil { return err } @@ -718,7 +622,7 @@ func (c *Client) NotifyChange(ctx context.Context, filepath string) error { }, } - return c.Notify(ctx, "textDocument/didChange", params) + return c.DidChange(ctx, params) } func (c *Client) CloseFile(ctx context.Context, filepath string) error { @@ -741,7 +645,7 @@ func (c *Client) CloseFile(ctx context.Context, filepath string) error { if cfg.Options.DebugLSP { slog.Debug("Closing file", "file", filepath) } - if err := c.Notify(ctx, "textDocument/didClose", params); err != nil { + if err := c.DidClose(ctx, params); err != nil { return err } diff --git a/internal/lsp/transport.go b/internal/lsp/transport.go index b468101dbc36537c9f306399b4af6cbbe451d96f..483281d25c51a6bfb71ca3314419b570f9a6bf0d 100644 --- a/internal/lsp/transport.go +++ b/internal/lsp/transport.go @@ -188,9 +188,12 @@ func (c *Client) handleMessages() { // Call makes a request and waits for the response func (c *Client) Call(ctx context.Context, method string, params any, result any) error { - cfg := config.Get() + if !c.IsMethodSupported(method) { + return fmt.Errorf("method not supported by server: %s", method) + } id := c.nextID.Add(1) + cfg := config.Get() if cfg.Options.DebugLSP { slog.Debug("Making call", "method", method, "id", id) } @@ -253,6 +256,12 @@ func (c *Client) Call(ctx context.Context, method string, params any, result any // Notify sends a notification (a request without an ID that doesn't expect a response) func (c *Client) Notify(ctx context.Context, method string, params any) error { cfg := config.Get() + if !c.IsMethodSupported(method) { + if cfg.Options.DebugLSP { + slog.Debug("Skipping notification: method not supported by server", "method", method) + } + return nil + } if cfg.Options.DebugLSP { slog.Debug("Sending notification", "method", method) } diff --git a/internal/lsp/watcher/watcher.go b/internal/lsp/watcher/watcher.go index 476c49361e2ba4e07b6c9b64a8d884e74d3013ed..ad03099ae9a2b1e516fdcab820052c1ca858bd2a 100644 --- a/internal/lsp/watcher/watcher.go +++ b/internal/lsp/watcher/watcher.go @@ -623,18 +623,11 @@ func (w *WorkspaceWatcher) matchesPattern(path string, pattern protocol.GlobPatt if basePath == "" { return false } - // For relative patterns - if basePath, err = protocol.DocumentURI(basePath).Path(); err != nil { - // XXX: Do we want to return here, or send the error up the stack? - slog.Error("Error converting base path to URI", "basePath", basePath, "error", err) - } - - basePath = filepath.ToSlash(basePath) // Make path relative to basePath for matching relPath, err := filepath.Rel(basePath, path) if err != nil { - slog.Error("Error getting relative path", "path", path, "basePath", basePath, "error", err) + slog.Error("Error getting relative path", "path", path, "basePath", basePath, "error", err, "server", w.name) return false } relPath = filepath.ToSlash(relPath)