fix(lsp): recover from handler panics and decode client uris

2026-09-14 18:22:00 +02:00
parent e307bf830f
commit 909f874797
4 changed files with 190 additions and 23 deletions
@@ -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),
@@ -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 ----------------------------------------------
@@ -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
@@ -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)
+	}
+}