From 35df7431c3c931d36c2c7229a7ebb36ca5a7d017 Mon Sep 17 00:00:00 2001 From: karitham Date: Fri, 7 Aug 2026 22:14:10 +0200 Subject: [PATCH] lsp: block-diff range formatting --- lsp/format.go | 61 +++++--- lsp/format_range_fuzz_test.go | 18 ++- lsp/format_range_server_test.go | 134 ++++++++++++++---- lsp/format_range_test.go | 16 +-- .../fuzz/FuzzFormatRangeText/7f0ba3fa9729404d | 4 - .../fuzz/FuzzFormatRangeText/aec81daebe605589 | 2 + 6 files changed, 172 insertions(+), 63 deletions(-) delete mode 100644 lsp/testdata/fuzz/FuzzFormatRangeText/7f0ba3fa9729404d create mode 100644 lsp/testdata/fuzz/FuzzFormatRangeText/aec81daebe605589 diff --git a/lsp/format.go b/lsp/format.go index 287bc3b..075b24f 100644 --- a/lsp/format.go +++ b/lsp/format.go @@ -188,41 +188,66 @@ type blockEdit struct { // of non-blank lines, plus the leading and trailing blank regions when // they differ. Blank lines are preserved exactly by the formatter, so old // and new split into the same number of aligned blocks. +// blockDiff returns the edits turning old into new, one per changed +// segment: the blank-line runs and the blocks of non-blank lines between +// them. Blank lines are preserved structurally by the formatter (their +// whitespace may be trimmed), so old and new split into the same number of +// aligned blocks; every edit is bounded by blank lines or file edges, so +// any subset splices safely. func blockDiff(old, new []byte) []blockEdit { + // CRLF input normalizes to LF everywhere, blank lines included: the + // block alignment no longer holds, so a single whole-document edit is + // the only safe splice. + if bytes.Contains(old, []byte("\r\n")) { + if string(old) == string(new) { + return nil + } + + return []blockEdit{{0, len(old), string(new)}} + } + oldBlocks := blocks(old) newBlocks := blocks(new) - if len(oldBlocks) != len(newBlocks) { - // Should not happen: the formatter preserves the blank-line - // structure. Fall back to a single whole-document edit. + // No non-blank lines at all, or an unaligned block structure: fall + // back to a single whole-document edit. + if len(oldBlocks) == 0 || len(oldBlocks) != len(newBlocks) { + if string(old) == string(new) { + return nil + } + return []blockEdit{{0, len(old), string(new)}} } var edits []blockEdit - // Leading blank region. - oldLead := old[:oldBlocks[0].start] - newLead := new[:newBlocks[0].start] - if string(oldLead) != string(newLead) { - edits = append(edits, blockEdit{0, oldBlocks[0].start, string(newLead)}) - } - + prevOld, prevNew := 0, 0 for i := range oldBlocks { - if oldBlocks[i].text != newBlocks[i].text { + // The segment before the block: leading blanks, or the blank run + // between two blocks. + if !bytes.Equal(old[prevOld:oldBlocks[i].start], new[prevNew:newBlocks[i].start]) { + edits = append(edits, blockEdit{ + start: prevOld, + end: oldBlocks[i].start, + text: string(new[prevNew:newBlocks[i].start]), + }) + } + + // The block itself. + if !bytes.Equal(old[oldBlocks[i].start:oldBlocks[i].end], new[newBlocks[i].start:newBlocks[i].end]) { edits = append(edits, blockEdit{ start: oldBlocks[i].start, end: oldBlocks[i].end, - text: newBlocks[i].text, + text: string(new[newBlocks[i].start:newBlocks[i].end]), }) } + + prevOld, prevNew = oldBlocks[i].end, newBlocks[i].end } - // Trailing blank region. - oldTail := old[oldBlocks[len(oldBlocks)-1].end:] - newTail := new[newBlocks[len(newBlocks)-1].end:] - if string(oldTail) != string(newTail) { - last := oldBlocks[len(oldBlocks)-1] - edits = append(edits, blockEdit{last.end, len(old), string(newTail)}) + // The trailing segment. + if !bytes.Equal(old[prevOld:], new[prevNew:]) { + edits = append(edits, blockEdit{prevOld, len(old), string(new[prevNew:])}) } return edits diff --git a/lsp/format_range_fuzz_test.go b/lsp/format_range_fuzz_test.go index a874ec0..085e985 100644 --- a/lsp/format_range_fuzz_test.go +++ b/lsp/format_range_fuzz_test.go @@ -62,18 +62,19 @@ func FuzzFormatRangeText(f *testing.F) { } // Edits are ordered, non-overlapping, line-aligned, and in bounds. + // A zero-length edit is a valid insertion. prev := 0 for _, e := range edits { - if e.start < prev || e.end > len(content) || e.start >= e.end { + if e.start < prev || e.end > len(content) || e.start > e.end { t.Fatalf("invalid edit [%d, %d) in %q", e.start, e.end, content) } - if e.start != 0 && content[e.start-1] != '\n' { + if !atLineBoundary(content, e.start) { t.Fatalf("edit starts mid-line at %d in %q", e.start, content) } - if e.end != len(content) && content[e.end] != '\n' { + if !atLineBoundary(content, e.end) { t.Fatalf("edit ends mid-line at %d in %q", e.end, content) } @@ -97,3 +98,14 @@ func FuzzFormatRangeText(f *testing.F) { } }) } + +// atLineBoundary reports whether offset is at a line boundary: the file +// edges, right after a newline (line start), or right before one (line +// end). +func atLineBoundary(content []byte, offset int) bool { + if offset == 0 || offset == len(content) { + return true + } + + return content[offset-1] == '\n' || content[offset] == '\n' +} diff --git a/lsp/format_range_server_test.go b/lsp/format_range_server_test.go index 5c7c368..0d74e1d 100644 --- a/lsp/format_range_server_test.go +++ b/lsp/format_range_server_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.lsp.dev/protocol" "go.lsp.dev/uri" @@ -14,8 +15,7 @@ import ( func TestServerRangeFormatting(t *testing.T) { ctx := t.Context() - fileURI, err := uri.Parse("file:///tmp/range.thrift") - assert.NoError(t, err) + fileURI := uri.URI("file:///tmp/range.thrift") // Only struct B needs reformatting; A and C are already formatted. content := `struct A { 1: string a } @@ -29,52 +29,75 @@ struct C { 3: i64 c } srv := NewServer(cache.New(nil), nil, formatter.Options{}) - err = srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ + require.NoError(t, srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: protocol.TextDocumentItem{ URI: fileURI, LanguageID: "thrift", Version: 0, Text: content, }, - }) - assert.NoError(t, err) + })) + + // applyEdits applies the edits to the given text. + applyEdits := func(text string, edits []protocol.TextEdit) string { + out := text + for _, e := range edits { + out = applyEdit(out, e) + } + + return out + } formatting := func() string { edits, err := srv.Formatting(ctx, &protocol.DocumentFormattingParams{ TextDocument: protocol.TextDocumentIdentifier{URI: fileURI}, }) - assert.NoError(t, err) - assert.Len(t, edits, 1) + require.NoError(t, err) + require.Len(t, edits, 1) return edits[0].NewText } - t.Run("formats struct bounded by blank lines", func(t *testing.T) { - // Select struct B entirely (lines 2-4). + t.Run("formats the enclosing block of the selection", func(t *testing.T) { + // Select the two lines of struct B's body (lines 3-4). edits, err := srv.RangeFormatting(ctx, &protocol.DocumentRangeFormattingParams{ TextDocument: protocol.TextDocumentIdentifier{URI: fileURI}, Range: protocol.Range{ - Start: protocol.Position{Line: 2, Character: 0}, - End: protocol.Position{Line: 4, Character: 1}, + Start: protocol.Position{Line: 3, Character: 0}, + End: protocol.Position{Line: 4, Character: 0}, }, }) - assert.NoError(t, err) - assert.Len(t, edits, 1) + require.NoError(t, err) + require.Len(t, edits, 1) - // The edit covers exactly struct B's lines and collapses it. + // The edit covers exactly struct B's block and collapses it. assert.Equal(t, protocol.Range{ Start: protocol.Position{Line: 2, Character: 0}, - End: protocol.Position{Line: 4, Character: 1}, + End: protocol.Position{Line: 5, Character: 0}, }, edits[0].Range) - assert.Equal(t, "struct B { 2: i32 b }", edits[0].NewText) + assert.Equal(t, "struct B { 2: i32 b }\n", edits[0].NewText) // Applying the edit reproduces the whole-document formatting. want := formatting() - spliced := content[:strings.Index(content, "struct B")] + edits[0].NewText + content[strings.Index(content, "}\n\nstruct C")+1:] - assert.Equal(t, want, spliced) + assert.Equal(t, want, applyEdits(content, edits)) }) - t.Run("whole document range delegates to full formatting", func(t *testing.T) { + t.Run("mid-declaration selection still formats the enclosing block", func(t *testing.T) { + // Select a slice that cuts struct B in half (inside the opening + // line to inside the field line). + edits, err := srv.RangeFormatting(ctx, &protocol.DocumentRangeFormattingParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: fileURI}, + Range: protocol.Range{ + Start: protocol.Position{Line: 2, Character: 5}, + End: protocol.Position{Line: 3, Character: 4}, + }, + }) + require.NoError(t, err) + require.Len(t, edits, 1) + assert.Equal(t, "struct B { 2: i32 b }\n", edits[0].NewText) + }) + + t.Run("whole document range returns the full formatting edits", func(t *testing.T) { edits, err := srv.RangeFormatting(ctx, &protocol.DocumentRangeFormattingParams{ TextDocument: protocol.TextDocumentIdentifier{URI: fileURI}, Range: protocol.Range{ @@ -82,22 +105,77 @@ struct C { 3: i64 c } End: protocol.Position{Line: 7, Character: 0}, }, }) - assert.NoError(t, err) - assert.Len(t, edits, 1) - assert.Equal(t, formatting(), edits[0].NewText) + require.NoError(t, err) + + assert.Equal(t, formatting(), applyEdits(content, edits)) }) - t.Run("no edits for a selection that cuts a declaration", func(t *testing.T) { - // Start inside struct B's opening line, end inside its field line: - // the expanded range is not bounded by blank lines. + t.Run("selection outside any changed block yields no edits", func(t *testing.T) { + // Struct C is already formatted: selecting it changes nothing. edits, err := srv.RangeFormatting(ctx, &protocol.DocumentRangeFormattingParams{ TextDocument: protocol.TextDocumentIdentifier{URI: fileURI}, Range: protocol.Range{ - Start: protocol.Position{Line: 2, Character: 5}, - End: protocol.Position{Line: 3, Character: 4}, + Start: protocol.Position{Line: 6, Character: 0}, + End: protocol.Position{Line: 6, Character: 20}, }, }) - assert.NoError(t, err) + require.NoError(t, err) assert.Empty(t, edits) }) + + t.Run("unsaved overlay content is formatted", func(t *testing.T) { + // Change the buffer without saving: range formatting must read + // the overlay, not the disk. + unsaved := `struct A { 1: string a } + +struct D { +4: i64 d +} +` + require.NoError(t, srv.DidChange(ctx, &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{URI: fileURI}, + Version: 1, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + &protocol.TextDocumentContentChangeWholeDocument{ + Text: unsaved, + }, + }, + })) + + edits, err := srv.RangeFormatting(ctx, &protocol.DocumentRangeFormattingParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: fileURI}, + Range: protocol.Range{ + Start: protocol.Position{Line: 3, Character: 0}, + End: protocol.Position{Line: 3, Character: 6}, + }, + }) + require.NoError(t, err) + require.Len(t, edits, 1) + assert.Equal(t, "struct D { 4: i64 d }\n", edits[0].NewText) + assert.Equal(t, "struct A { 1: string a }\n\nstruct D { 4: i64 d }\n", applyEdits(unsaved, edits)) + }) +} + +// applyEdit applies a single text edit to text, resolving the range's +// line/character positions to byte offsets. +func applyEdit(text string, edit protocol.TextEdit) string { + start := offsetAt(text, edit.Range.Start) + end := offsetAt(text, edit.Range.End) + + return text[:start] + edit.NewText + text[end:] +} + +// offsetAt resolves a position to a byte offset within text. +func offsetAt(text string, pos protocol.Position) int { + offset := 0 + + for range pos.Line { + if i := strings.IndexByte(text[offset:], '\n'); i >= 0 { + offset += i + 1 + } + } + + return offset + int(pos.Character) } diff --git a/lsp/format_range_test.go b/lsp/format_range_test.go index a18db69..1264675 100644 --- a/lsp/format_range_test.go +++ b/lsp/format_range_test.go @@ -55,26 +55,22 @@ func TestBlockDiff(t *testing.T) { old: "struct A {}\n\n\n", new: "struct A {}\n", want: []blockEdit{{ - start: len("struct A {}"), + start: len("struct A {}\n"), end: len("struct A {}\n\n\n"), text: "", }}, }, { - name: "crlf line endings are preserved", + name: "crlf input falls back to a whole-document edit", old: "struct A {}\r\n\r\nstruct B {\r\n2: i32 b\r\n}\r\n", new: "struct A {}\r\n\r\nstruct B { 2: i32 b }\r\n", - want: []blockEdit{{ - start: strings.Index("struct A {}\r\n\r\nstruct B {\r\n2: i32 b\r\n}\r\n", "struct B"), - end: len("struct A {}\r\n\r\nstruct B {\r\n2: i32 b\r\n}\r\n"), - text: "struct B { 2: i32 b }\r\n", - }}, + want: []blockEdit{{0, len("struct A {}\r\n\r\nstruct B {\r\n2: i32 b\r\n}\r\n"), "struct A {}\r\n\r\nstruct B { 2: i32 b }\r\n"}}, }, { - name: "block structure change falls back to one edit", + name: "unaligned block structure falls back to one edit", old: "struct A {}\n\n\nstruct B {}\n", - new: "struct A {}\n\nstruct B {}\n", - want: []blockEdit{{0, len("struct A {}\n\n\nstruct B {}\n"), "struct A {}\n\nstruct B {}\n"}}, + new: "struct A {}\n\nstruct B {}\n\nstruct C {}\n", + want: []blockEdit{{0, len("struct A {}\n\n\nstruct B {}\n"), "struct A {}\n\nstruct B {}\n\nstruct C {}\n"}}, }, } diff --git a/lsp/testdata/fuzz/FuzzFormatRangeText/7f0ba3fa9729404d b/lsp/testdata/fuzz/FuzzFormatRangeText/7f0ba3fa9729404d deleted file mode 100644 index 5add488..0000000 --- a/lsp/testdata/fuzz/FuzzFormatRangeText/7f0ba3fa9729404d +++ /dev/null @@ -1,4 +0,0 @@ -go test fuzz v1 -[]byte(" #0000000000\n\n00") -int(0) -int(-49) diff --git a/lsp/testdata/fuzz/FuzzFormatRangeText/aec81daebe605589 b/lsp/testdata/fuzz/FuzzFormatRangeText/aec81daebe605589 new file mode 100644 index 0000000..264f3ab --- /dev/null +++ b/lsp/testdata/fuzz/FuzzFormatRangeText/aec81daebe605589 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("#\n \n#") -- 2.51.2