Skip to content

Commit cd70d46

Browse files
committed
feat(radls): immutable DocumentVersion snapshots per document
Replace the in-place-mutated DocState with an immutable DocumentVersion built fresh on every didOpen/didChange. A Document owns the current snapshot via sync/atomic.Pointer; readers grab the snapshot lock-free, writers serialize through the Document's mutex. The load-bearing guarantee: once an LSP request handler has a *DocumentVersion, the world it observes is frozen for the duration of the request. Today the mux dispatches handlers serially so no concurrency exists, but the next phase needs to spawn goroutines per request to plumb context cancellation - and you absolutely do not want hover racing against the next keystroke. State.docs is now URI -> *Document. Per-document writes coordinate via Document.mu; parser access goes through State.parserMu since tree-sitter parsers are not goroutine-safe. The docs map is itself guarded by an RWMutex so concurrent reads from different documents don't block each other. Diagnostics conversion moves into document.go's runChecker, which takes (checker, LineIndex, encoding) - the canonical boundary between check.Diagnostic (utf-8 byte cols) and lsp.Diagnostic (client's negotiated encoding). The previous resolveDiagnostics helper in diagnostics.go is gone; snapshot construction now owns the translation end-to-end. Tests: snapshot stability (pointer doesn't observe later writes), concurrent readers + writers (race-clean under -race), guard against UpdateDoc before AddDoc. Run with -race; passes.
1 parent 65a230e commit cd70d46

7 files changed

Lines changed: 472 additions & 186 deletions

File tree

radls/analysis/diagnostics.go

Lines changed: 0 additions & 48 deletions
This file was deleted.

radls/analysis/diagnostics_test.go

Lines changed: 57 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -3,74 +3,66 @@ package analysis
33
import (
44
"testing"
55

6+
"github.com/amterp/rad/radls/lsp"
7+
"github.com/amterp/rad/rts"
68
"github.com/amterp/rad/rts/check"
9+
"github.com/amterp/rad/rts/rl"
710
)
811

9-
// TestToLspRangeUTF16 verifies the byte->utf-16 boundary conversion.
10-
// Documents the contract: check.Range carries utf-8 byte columns (the
11-
// tree-sitter native); resolveDiagnostics translates to the client's
12-
// negotiated encoding before publishing.
13-
func TestToLspRangeUTF16(t *testing.T) {
12+
// TestRunCheckerConvertsUTF16 verifies the byte->utf-16 boundary
13+
// conversion that runChecker applies when translating check
14+
// diagnostics into the wire format. Documents the contract:
15+
// check.Range carries utf-8 byte columns (tree-sitter native);
16+
// buildVersion translates to the client's negotiated encoding.
17+
func TestRunCheckerConvertsUTF16(t *testing.T) {
1418
// Line 0: `x = "中"` (9 bytes; 中 = 3 bytes, 1 utf-16 unit)
1519
// Line 1: `y = "🎉"` (10 bytes; 🎉 = 4 bytes, 2 utf-16 units)
1620
text := "x = \"\"\ny = \"🎉\""
17-
18-
s := NewState()
19-
s.SetEncoding(EncodingUTF16)
20-
doc := &DocState{lineIndex: NewLineIndex(text)}
21+
idx := NewLineIndex(text)
2122

2223
cases := []struct {
23-
name string
24-
in check.Range
25-
wantSL int
26-
wantSC int
27-
wantEL int
28-
wantEC int
24+
name string
25+
in check.Range
26+
enc PositionEncoding
27+
wantSC int
28+
wantEC int
2929
}{
30-
{
31-
name: "ascii range untouched",
32-
in: mkRange(0, 0, 0, 3),
33-
wantSL: 0, wantSC: 0, wantEL: 0, wantEC: 3,
34-
},
35-
{
36-
name: "byte col after CJK char compresses to utf-16",
37-
in: mkRange(0, 4, 0, 9), // covers `"中"` (5 bytes -> 3 utf-16 units)
38-
wantSL: 0, wantSC: 4, wantEL: 0, wantEC: 7,
39-
},
40-
{
41-
name: "byte col after astral char compresses",
42-
in: mkRange(1, 4, 1, 10), // covers `"🎉"` (6 bytes -> 4 utf-16 units)
43-
wantSL: 1, wantSC: 4, wantEL: 1, wantEC: 8,
44-
},
30+
{"ascii range untouched utf-16", mkRange(0, 0, 0, 3), EncodingUTF16, 0, 3},
31+
{"byte col after CJK -> utf-16", mkRange(0, 4, 0, 9), EncodingUTF16, 4, 7},
32+
{"byte col after astral -> utf-16", mkRange(1, 4, 1, 10), EncodingUTF16, 4, 8},
33+
{"utf-8 passthrough", mkRange(0, 4, 0, 9), EncodingUTF8, 4, 9},
4534
}
4635

4736
for _, tc := range cases {
4837
t.Run(tc.name, func(t *testing.T) {
49-
got := s.toLspRange(tc.in, doc)
50-
if got.Start.Line != tc.wantSL || got.Start.Character != tc.wantSC ||
51-
got.End.Line != tc.wantEL || got.End.Character != tc.wantEC {
52-
t.Errorf("got %d:%d-%d:%d, want %d:%d-%d:%d",
53-
got.Start.Line, got.Start.Character, got.End.Line, got.End.Character,
54-
tc.wantSL, tc.wantSC, tc.wantEL, tc.wantEC)
38+
ck := &stubChecker{result: check.Result{Diagnostics: []check.Diagnostic{
39+
{Range: tc.in, Severity: check.Error, Message: "x"},
40+
}}}
41+
diags := runChecker(ck, idx, tc.enc)
42+
if len(diags) != 1 {
43+
t.Fatalf("expected 1 diagnostic, got %d", len(diags))
44+
}
45+
got := diags[0].Range
46+
if got.Start.Character != tc.wantSC || got.End.Character != tc.wantEC {
47+
t.Errorf("got %d-%d, want %d-%d",
48+
got.Start.Character, got.End.Character,
49+
tc.wantSC, tc.wantEC)
5550
}
5651
})
5752
}
5853
}
5954

60-
// TestToLspRangeUTF8 verifies utf-8 is a passthrough (with clamping).
61-
// A client that negotiates utf-8 should get byte columns unchanged.
62-
func TestToLspRangeUTF8(t *testing.T) {
63-
text := "x = \"\""
64-
65-
s := NewState()
66-
s.SetEncoding(EncodingUTF8)
67-
doc := &DocState{lineIndex: NewLineIndex(text)}
68-
69-
in := mkRange(0, 4, 0, 9)
70-
got := s.toLspRange(in, doc)
71-
if got.Start.Character != 4 || got.End.Character != 9 {
72-
t.Errorf("utf-8 passthrough: got %d-%d, want 4-9",
73-
got.Start.Character, got.End.Character)
55+
// TestRunCheckerEmpty verifies the no-diagnostics path still returns
56+
// a non-nil (zero-length) slice, since the LSP wire format wants an
57+
// explicit [] for "no diagnostics."
58+
func TestRunCheckerEmpty(t *testing.T) {
59+
ck := &stubChecker{result: check.Result{Diagnostics: nil}}
60+
got := runChecker(ck, NewLineIndex(""), EncodingUTF16)
61+
if got == nil {
62+
t.Errorf("expected non-nil empty slice, got nil")
63+
}
64+
if len(got) != 0 {
65+
t.Errorf("expected zero diagnostics, got %d", len(got))
7466
}
7567
}
7668

@@ -80,3 +72,18 @@ func mkRange(sl, sc, el, ec int) check.Range {
8072
End: check.Pos{Line: el, Character: ec},
8173
}
8274
}
75+
76+
// stubChecker is a minimal RadChecker for testing the conversion path
77+
// without needing a real parser/tree.
78+
type stubChecker struct {
79+
result check.Result
80+
}
81+
82+
func (s *stubChecker) UpdateSrc(string) {}
83+
func (s *stubChecker) Update(*rts.RadTree, string, *rl.SourceFile) {}
84+
func (s *stubChecker) Check() (check.Result, error) { return s.result, nil }
85+
86+
// Compile-time interface check. lsp import keeps the file honest if
87+
// the test ever drops its only lsp reference.
88+
var _ check.RadChecker = (*stubChecker)(nil)
89+
var _ = lsp.Diagnostic{}

radls/analysis/document.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package analysis
2+
3+
import (
4+
"sync"
5+
"sync/atomic"
6+
7+
"github.com/amterp/rad/radls/lsp"
8+
9+
"github.com/amterp/rad/rts"
10+
"github.com/amterp/rad/rts/check"
11+
"github.com/amterp/rad/rts/rl"
12+
)
13+
14+
// DocumentVersion is an immutable snapshot of a single parse of a
15+
// document. Multiple readers can hold the same *DocumentVersion
16+
// concurrently without coordination: the data inside is guaranteed
17+
// not to mutate. The next didChange produces a NEW DocumentVersion
18+
// (built off the old) and atomically swaps the owning Document's
19+
// current pointer; old versions remain valid for any reader still
20+
// holding them, and are GC'd once unreferenced.
21+
//
22+
// This is the load-bearing piece of Phase 8: it lets LSP request
23+
// handlers (hover, goto-def, completion, etc.) grab a snapshot once
24+
// and reason about a frozen world for the duration of the request,
25+
// rather than racing against the next keystroke.
26+
type DocumentVersion struct {
27+
uri string
28+
version int64
29+
text string
30+
tree *rts.RadTree
31+
ast *rl.SourceFile
32+
lineIndex *LineIndex
33+
diagnostics []lsp.Diagnostic
34+
}
35+
36+
func (v *DocumentVersion) URI() string { return v.uri }
37+
func (v *DocumentVersion) Version() int64 { return v.version }
38+
func (v *DocumentVersion) Text() string { return v.text }
39+
func (v *DocumentVersion) Tree() *rts.RadTree { return v.tree }
40+
func (v *DocumentVersion) AST() *rl.SourceFile { return v.ast }
41+
func (v *DocumentVersion) LineIndex() *LineIndex { return v.lineIndex }
42+
func (v *DocumentVersion) Diagnostics() []lsp.Diagnostic { return v.diagnostics }
43+
44+
// GetLine returns the source of the line at the given index, or "" if
45+
// out of range. Kept on DocumentVersion (not LineIndex) because callers
46+
// usually want both the text and the index together.
47+
func (v *DocumentVersion) GetLine(line int) string {
48+
idx := v.lineIndex
49+
if line < 0 || line >= idx.LineCount() {
50+
return ""
51+
}
52+
return idx.lineSlice(line)
53+
}
54+
55+
// Document owns the current snapshot of one LSP document. The snapshot
56+
// pointer is read lock-free; writers serialize through `mu` to ensure
57+
// a coherent prev->next chain (tree-sitter parsing today is wholesale,
58+
// but a future incremental-parse path would still want this invariant).
59+
type Document struct {
60+
snapshot atomic.Pointer[DocumentVersion]
61+
mu sync.Mutex
62+
}
63+
64+
// Snapshot returns the current immutable version of this document.
65+
// Lock-free; safe to call from any goroutine.
66+
func (d *Document) Snapshot() *DocumentVersion {
67+
return d.snapshot.Load()
68+
}
69+
70+
// Update runs `produce` under the writer lock to compute the next
71+
// version from the previous (nil on first open), then atomically swaps
72+
// it into place. Returns the new version.
73+
func (d *Document) Update(produce func(prev *DocumentVersion) *DocumentVersion) *DocumentVersion {
74+
d.mu.Lock()
75+
defer d.mu.Unlock()
76+
prev := d.snapshot.Load()
77+
next := produce(prev)
78+
d.snapshot.Store(next)
79+
return next
80+
}
81+
82+
// buildVersion is the canonical way to construct a DocumentVersion: it
83+
// parses the source, builds the AST, indexes lines, runs the static
84+
// checker, and translates diagnostics into the negotiated encoding.
85+
// Caller (typically State) owns the parser and encoding it passes in.
86+
func buildVersion(
87+
parser *rts.RadParser,
88+
encoding PositionEncoding,
89+
uri string,
90+
version int64,
91+
text string,
92+
) *DocumentVersion {
93+
tree := parser.Parse(text)
94+
ast := safeConvertCST(tree, text, uri)
95+
lineIndex := NewLineIndex(text)
96+
97+
checker := check.NewCheckerWithTree(tree, parser, text, ast)
98+
diags := runChecker(checker, lineIndex, encoding)
99+
100+
return &DocumentVersion{
101+
uri: uri,
102+
version: version,
103+
text: text,
104+
tree: tree,
105+
ast: ast,
106+
lineIndex: lineIndex,
107+
diagnostics: diags,
108+
}
109+
}
110+
111+
// runChecker is the boundary between check.Diagnostic (utf-8 byte
112+
// columns) and lsp.Diagnostic (negotiated encoding). Lives here so the
113+
// snapshot construction path owns the translation, rather than scatter
114+
// it across the analysis package.
115+
func runChecker(checker check.RadChecker, idx *LineIndex, enc PositionEncoding) []lsp.Diagnostic {
116+
out := make([]lsp.Diagnostic, 0)
117+
result, err := checker.Check()
118+
if err != nil {
119+
return out
120+
}
121+
for _, cd := range result.Diagnostics {
122+
rang := lsp.Range{
123+
Start: lsp.Pos{
124+
Line: cd.Range.Start.Line,
125+
Character: idx.ByteColumnTo(cd.Range.Start.Line, cd.Range.Start.Character, enc),
126+
},
127+
End: lsp.Pos{
128+
Line: cd.Range.End.Line,
129+
Character: idx.ByteColumnTo(cd.Range.End.Line, cd.Range.End.Character, enc),
130+
},
131+
}
132+
out = append(out, lsp.NewDiagnosticFromCheckWithRange(cd, rang))
133+
}
134+
return out
135+
}

0 commit comments

Comments
 (0)