From 909f8747975a022bb69ebe1ce67700301555cf68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Mon, 14 Sep 2026 18:22:00 +0200 Subject: [PATCH] fix(lsp): recover from handler panics and decode client uris --- lsp/handlers.go | 47 ++++++++++++----- lsp/protocol.go | 3 +- lsp/server.go | 37 ++++++++++--- lsp/server_test.go | 126 +++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 190 insertions(+), 23 deletions(-) diff --git a/lsp/handlers.go b/lsp/handlers.go index bb7ed52..c1f00e9 100644 --- a/lsp/handlers.go +++ b/lsp/handlers.go @@ -22,12 +22,13 @@ import ( "sourcedock.dev/petrbalvin/gasm-devkit/token" ) -// textflagMacros are the flag names defined by textflag.h; they are highlighted -// as macros and offered as completions after a TEXT/GLOBL directive. +// textflagMacros are the flag names defined by runtime/textflag.h; they are +// highlighted as macros and offered as completions after a TEXT/GLOBL +// directive. var textflagMacros = map[string]bool{ - "NOPROFILE": true, "DUPOK": true, "NOSPLIT": true, "RODATA": true, - "NOPTR": true, "WRAPPER": true, "NEEDCTXT": true, "TOPFRAME": true, - "LEAF": true, "ABI0": true, "REFLECTDATA": true, + "NOPROF": true, "DUPOK": true, "NOSPLIT": true, "RODATA": true, + "NOPTR": true, "WRAPPER": true, "NEEDCTXT": true, "TLSBSS": true, + "NOFRAME": true, "REFLECTMETHOD": true, "TOPFRAME": true, "ABIWRAPPER": true, } // completion builds the completion list for a document. @@ -81,13 +82,13 @@ func (s *Server) hover(p hoverParams) *Hover { var md string if in, ok := a.Lookup(word); ok { - md = "**" + in.Name + "** — " + in.Summary + md = "**" + in.Name + "**: " + in.Summary } else if r, ok := a.Register(word); ok { - md = "**" + r.Name + "** — " + r.Class.String() + " register. " + r.Desc + md = "**" + r.Name + "**: " + r.Class.String() + " register. " + r.Desc } else if desc, ok := arch.PseudoRegDesc(word); ok { - md = "**" + strings.ToUpper(word) + "** — pseudo-register. " + desc + md = "**" + strings.ToUpper(word) + "**: pseudo-register. " + desc } else if textflagMacros[strings.ToUpper(word)] { - md = "**" + strings.ToUpper(word) + "** — textflag.h flag" + md = "**" + strings.ToUpper(word) + "**: textflag.h flag" } else { return nil } @@ -271,7 +272,9 @@ func (s *Server) codeActions(p codeActionParams) []CodeAction { for _, diag := range p.Context.Diagnostics { switch diag.Code { case "missing-ret": - // Offer to add RET at the end of the function. + // Offer to add RET at the end of the flagged function only: the + // diagnostic's range covers the TEXT keyword, so a line match + // picks the function the diagnostic belongs to. f, _ := parser.Parse(uriPath(p.TextDocument.URI), text) if f == nil { continue @@ -281,6 +284,9 @@ func (s *Server) codeActions(p codeActionParams) []CodeAction { if !ok { continue } + if t.Keyword.Pos.Line-1 != int(diag.Range.Start.Line) { + continue + } if len(t.Body) == 0 { continue } @@ -298,7 +304,7 @@ func (s *Server) codeActions(p codeActionParams) []CodeAction { endLine := len(lines) - 1 endChar := len([]rune(lines[endLine])) actions = append(actions, CodeAction{ - Title: "Add RET", + Title: "Add RET to " + t.Name.Name, Kind: "quickfix", Edit: &WorkspaceEdit{ Changes: map[string][]TextEdit{p.TextDocument.URI: { @@ -792,13 +798,28 @@ func textRange(t *ast.Text) Range { // diagnosticsFor computes the LSP diagnostics of one document; the push // (publishDiagnostics) and pull (textDocument/diagnostic) paths share it. +// Parse errors surface as error-severity diagnostics so a malformed line is +// visible in the editor instead of only breaking derived features. func (s *Server) diagnosticsFor(uri string) []Diagnostic { text := s.docs[uri] - f, _ := parser.Parse(uriPath(uri), text) + f, errs := parser.Parse(uriPath(uri), text) cfg := lint.Config{Arch: arch.FromFilename(uriPath(uri))} diags := lint.File(f, cfg) - out := make([]Diagnostic, 0, len(diags)) + out := make([]Diagnostic, 0, len(diags)+len(errs)) + for _, e := range errs { + pos := token.Position{Line: 1, Column: 1} + if pe, ok := e.(parser.Error); ok && pe.Pos.IsValid() { + pos = pe.Pos + } + out = append(out, Diagnostic{ + Range: toRange(pos.Line, pos.Column, token.Position{}), + Severity: sevError, + Code: "syntax", + Source: "gasm", + Message: e.Error(), + }) + } for _, d := range diags { out = append(out, Diagnostic{ Range: toRange(d.Pos.Line, d.Pos.Column, d.End), diff --git a/lsp/protocol.go b/lsp/protocol.go index 5fe51f5..b5ab681 100644 --- a/lsp/protocol.go +++ b/lsp/protocol.go @@ -4,7 +4,7 @@ // Package lsp implements a Language Server Protocol server for GAsm. It // speaks JSON-RPC 2.0 over any io.Reader/io.Writer pair (normally standard // input/output) and provides completion, hover documentation, document -// symbols, diagnostics and semantic-token highlighting — all backed by the +// symbols, diagnostics and semantic-token highlighting, all backed by the // pure-Go lexer, parser, arch and lint packages. It is the vendor-neutral // integration point: any LSP-capable editor can use it with no editor-specific // plugin code. @@ -31,6 +31,7 @@ type rpcError struct { const ( errMethodNotFound = -32601 + errInternalError = -32603 ) // --- LSP positions and ranges ---------------------------------------------- diff --git a/lsp/server.go b/lsp/server.go index 612e1de..f980272 100644 --- a/lsp/server.go +++ b/lsp/server.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "io" + "net/url" "strconv" "strings" "sync" @@ -18,10 +19,11 @@ import ( // Server is a GAsm language server bound to a byte stream. type Server struct { - in *bufio.Reader - out io.Writer - mu sync.Mutex // guards writes to out - docs map[string]string + in *bufio.Reader + out io.Writer + mu sync.Mutex // guards writes to out + docs map[string]string + version string // reported in the initialize result ("" omits it) } // New returns a server reading from in and writing to out. @@ -33,6 +35,9 @@ func New(in io.Reader, out io.Writer) *Server { } } +// SetVersion records the server version reported in the initialize result. +func (s *Server) SetVersion(v string) { s.version = v } + // Run serves requests until the input is exhausted or an exit is requested. func (s *Server) Run() error { for { @@ -109,9 +114,24 @@ func (s *Server) notify(method string, params any) { } // dispatch routes one message. It returns true when the server should stop. +// A panic in any handler is recovered and answered as an internal error: +// handlers parse live editor buffers, so malformed input must never take +// the whole server down. func (s *Server) dispatch(msg *rpcMessage) (exit bool) { + defer func() { + if r := recover(); r != nil { + if msg != nil && msg.ID != nil { + s.respondError(msg.ID, errInternalError, fmt.Sprintf("internal error: %v", r)) + } + } + }() + switch msg.Method { case "initialize": + info := map[string]string{"name": "gasm"} + if s.version != "" { + info["version"] = s.version + } s.respond(msg.ID, initializeResult{ Capabilities: ServerCapabilities{ TextDocumentSync: 1, // full sync @@ -138,7 +158,7 @@ func (s *Server) dispatch(msg *rpcMessage) (exit bool) { DocumentLinkProvider: map[string]any{}, FoldingRangeProvider: true, }, - ServerInfo: map[string]string{"name": "gasm", "version": "0.31.1"}, + ServerInfo: info, }) case "initialized", "textDocument/didSave": @@ -292,9 +312,14 @@ func lintSeverity(s lint.Severity) int { } } -// uriPath strips a file:// scheme and returns the path component. +// uriPath strips a file:// scheme and percent-decodes the path component. +// LSP clients percent-encode URIs, so a raw slice would break every on-disk +// lookup for paths containing spaces or non-ASCII characters. func uriPath(uri string) string { if rest, ok := strings.CutPrefix(uri, "file://"); ok { + if decoded, err := url.PathUnescape(rest); err == nil { + return decoded + } return rest } return uri diff --git a/lsp/server_test.go b/lsp/server_test.go index 76a5d6d..a062f05 100644 --- a/lsp/server_test.go +++ b/lsp/server_test.go @@ -381,7 +381,7 @@ func TestCodeActions(t *testing.T) { frame(10, "textDocument/codeAction", map[string]any{ "textDocument": map[string]any{"uri": "file:///f_amd64.s"}, "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 2, "character": 0}}, - "context": map[string]any{"diagnostics": []map[string]any{{"code": "missing-ret", "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 0, "character": 4}}}}}, + "context": map[string]any{"diagnostics": []map[string]any{{"code": "missing-ret", "range": map[string]any{"start": map[string]any{"line": 1, "character": 0}, "end": map[string]any{"line": 1, "character": 4}}}}}, }) + frame(nil, "exit", nil) msgs := run(t, in) resp := findByID(msgs, 10) @@ -395,8 +395,8 @@ func TestCodeActions(t *testing.T) { if len(actions) == 0 { t.Fatal("want at least 1 code action for missing-ret") } - if actions[0].Title != "Add RET" { - t.Errorf("action title = %q, want Add RET", actions[0].Title) + if actions[0].Title != "Add RET to foo" { + t.Errorf("action title = %q, want Add RET to foo", actions[0].Title) } } @@ -571,3 +571,123 @@ func TestFoldingRanges(t *testing.T) { t.Errorf("folding kind = %q, want region", ranges[0].Kind) } } + +// TestInitializeVersion checks the version reported in the initialize +// result when the caller stamps one. +func TestInitializeVersion(t *testing.T) { + var out bytes.Buffer + srv := New(strings.NewReader(frame(1, "initialize", map[string]any{})+frame(nil, "exit", nil)), &out) + srv.SetVersion("9.9.9") + if err := srv.Run(); err != nil { + t.Fatalf("server run: %v", err) + } + msgs := readFrames(t, &out) + resp := findByID(msgs, 1) + if resp == nil { + t.Fatal("no initialize response") + } + var res initializeResult + if err := json.Unmarshal(mustResult(t, resp), &res); err != nil { + t.Fatal(err) + } + if res.ServerInfo["version"] != "9.9.9" { + t.Errorf("serverInfo = %v, want version 9.9.9", res.ServerInfo) + } +} + +// TestSyntaxDiagnosticsPublished checks that parse errors reach the editor +// as error-severity diagnostics with the syntax code. +func TestSyntaxDiagnosticsPublished(t *testing.T) { + msgs := run(t, session("file:///f_amd64.s", "TEXT $\n")+frame(nil, "exit", nil)) + pub := findMethod(msgs, "textDocument/publishDiagnostics") + if pub == nil { + t.Fatal("no publishDiagnostics notification") + } + var p publishDiagnosticsParams + json.Unmarshal(pub.Params, &p) + found := false + for _, d := range p.Diagnostics { + if d.Code == "syntax" && d.Severity == sevError { + found = true + } + } + if !found { + t.Fatalf("expected a syntax error diagnostic, got %+v", p.Diagnostics) + } +} + +// TestDispatchRecoversFromPanic exercises the per-message recover: a panic +// inside a handler is answered as an internal error instead of taking the +// server down. +func TestDispatchRecoversFromPanic(t *testing.T) { + var out bytes.Buffer + srv := New(strings.NewReader(""), &out) + srv.docs = nil // force a nil-map write inside didOpen + raw := json.RawMessage(`{"textDocument":{"uri":"file:///x.s","text":"RET"}}`) + srv.dispatch(&rpcMessage{ID: rawID(t, 7), Method: "textDocument/didOpen", Params: raw}) + msgs := readFrames(t, &out) + resp := findByID(msgs, 7) + if resp == nil { + t.Fatal("no error response after panic") + } + if resp.Error == nil || resp.Error.Code != errInternalError { + t.Fatalf("error = %+v, want internal error", resp.Error) + } +} + +func rawID(t *testing.T, n int) *json.RawMessage { + t.Helper() + b, err := json.Marshal(n) + if err != nil { + t.Fatal(err) + } + raw := json.RawMessage(b) + return &raw +} + +// TestURIDecoding pins the percent-decoding of file URIs: clients encode +// non-ASCII paths, and the decoded form is what resolves on disk. +func TestURIDecoding(t *testing.T) { + got := uriPath("file:///home/petrbalvin/Repozit%C3%A1%C5%99e/k.s") + if got != "/home/petrbalvin/Repozitáře/k.s" { + t.Errorf("uriPath = %q", got) + } + if got := uriPath("/plain/path.s"); got != "/plain/path.s" { + t.Errorf("uriPath plain = %q", got) + } +} + +// TestCodeActionsTargetsFlaggedFunctionOnly checks that a missing-ret +// diagnostic offers an edit for the flagged function only, even when the +// file defines several functions. +func TestCodeActionsTargetsFlaggedFunctionOnly(t *testing.T) { + doc := "TEXT \u00b7first(SB), NOSPLIT, $0\n" + + "\tMOVQ AX, CX\n" + + "\tRET\n" + + "TEXT \u00b7second(SB), NOSPLIT, $0\n" + + "\tMOVQ AX, CX\n" + in := session("file:///f_amd64.s", doc) + + frame(11, "textDocument/codeAction", map[string]any{ + "textDocument": map[string]any{"uri": "file:///f_amd64.s"}, + "range": map[string]any{"start": map[string]any{"line": 0, "character": 0}, "end": map[string]any{"line": 4, "character": 0}}, + "context": map[string]any{"diagnostics": []map[string]any{{"code": "missing-ret", "range": map[string]any{"start": map[string]any{"line": 3, "character": 0}, "end": map[string]any{"line": 3, "character": 4}}}}}, + }) + frame(nil, "exit", nil) + msgs := run(t, in) + resp := findByID(msgs, 11) + if resp == nil { + t.Fatal("no codeAction response") + } + var actions []CodeAction + if err := json.Unmarshal(mustResult(t, resp), &actions); err != nil { + t.Fatal(err) + } + if len(actions) != 1 { + t.Fatalf("actions = %d, want 1", len(actions)) + } + if actions[0].Title != "Add RET to second" { + t.Errorf("action title = %q, want Add RET to second", actions[0].Title) + } + if !strings.Contains(actions[0].Edit.Changes["file:///f_amd64.s"][0].NewText, "\tRET\nTEXT \u00b7second") { + t.Errorf("edit does not insert RET at the end of second:\n%s", actions[0].Edit.Changes["file:///f_amd64.s"][0].NewText) + } +}