wip

Author: stefanvanburen

private/buf/buflsp/completion_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"buf.build/go/standard/xslices"
+	"github.com/bufbuild/buf/private/buf/buflsp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.lsp.dev/protocol"
@@ -412,3 +413,102 @@ func TestCompletionOptions(t *testing.T) {
 		})
 	}
 }
+func TestCompletionAdditionalTextEdits(t *testing.T) {
+	t.Parallel()
+
+	ctx := t.Context()
+
+	testProtoPath, err := filepath.Abs("testdata/completion/additional_edits_test.proto")
+	require.NoError(t, err)
+
+	clientJSONConn, testURI := setupLSPServer(t, testProtoPath)
+
+	tests := []struct {
+		name                 string
+		line                 uint32
+		character            uint32
+		triggerCompletion    string // Text to search for in completions
+		expectAdditionalEdit bool
+		validateEdit         func(t *testing.T, edit protocol.TextEdit)
+	}{
+		{
+			name:                 "syntax_completion_adds_semicolon",
+			line:                 0,
+			character:            10, // Inside the quotes: syntax = "|"
+			triggerCompletion:    "proto3",
+			expectAdditionalEdit: true,
+			validateEdit: func(t *testing.T, edit protocol.TextEdit) {
+				// Should add ";" at end of line since the file is missing it
+				assert.Equal(t, ";", edit.NewText, "should add semicolon")
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var completionList protocol.CompletionList
+			_, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentCompletion, protocol.CompletionParams{
+				TextDocumentPositionParams: protocol.TextDocumentPositionParams{
+					TextDocument: protocol.TextDocumentIdentifier{
+						URI: testURI,
+					},
+					Position: protocol.Position{
+						Line:      tt.line,
+						Character: tt.character,
+					},
+				},
+			}, &completionList)
+			require.NoError(t, err)
+
+			// Find the completion item we're looking for
+			var foundItem *protocol.CompletionItem
+			for _, item := range completionList.Items {
+				if item.Label == tt.triggerCompletion ||
+					(item.TextEdit != nil && item.TextEdit.NewText == tt.triggerCompletion) {
+					foundItem = &item
+					break
+				}
+			}
+
+			require.NotNil(t, foundItem, "should find completion item %q", tt.triggerCompletion)
+
+			if tt.expectAdditionalEdit {
+				require.NotEmpty(t, foundItem.AdditionalTextEdits, "should have additional text edits")
+
+				// Validate edits are minimal
+				for i, edit := range foundItem.AdditionalTextEdits {
+					// Additional edits should be small and targeted
+					assert.LessOrEqual(t, len(edit.NewText), 100,
+						"additional edit %d should be small (got %d chars)", i, len(edit.NewText))
+
+					// Validate specific edit properties if provided
+					if tt.validateEdit != nil {
+						tt.validateEdit(t, edit)
+					}
+
+					// Additional edits should not overlap with the main edit
+					if foundItem.TextEdit != nil {
+						assert.False(t, buflsp.RangesOverlap(foundItem.TextEdit.Range, edit.Range),
+							"additional edit %d should not overlap with main edit", i)
+					}
+				}
+
+				// Verify edits don't overlap with each other
+				for i := 0; i < len(foundItem.AdditionalTextEdits); i++ {
+					for j := i + 1; j < len(foundItem.AdditionalTextEdits); j++ {
+						assert.False(t, buflsp.RangesOverlap(
+							foundItem.AdditionalTextEdits[i].Range,
+							foundItem.AdditionalTextEdits[j].Range),
+							"additional edits %d and %d should not overlap", i, j)
+					}
+				}
+			} else {
+				assert.Empty(t, foundItem.AdditionalTextEdits, "should not have additional text edits")
+			}
+		})
+	}
+}
+
+// RangesOverlap returns true if two LSP ranges overlap.

private/buf/buflsp/document_link_test.go

@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/bufbuild/buf/private/buf/buflsp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.lsp.dev/protocol"
@@ -151,7 +152,7 @@ func assertNoOverlappingRanges(t *testing.T, links []protocol.DocumentLink) {
 			// Check if ranges overlap
 			assert.False(
 				t,
-				rangesOverlap(range1, range2),
+				buflsp.RangesOverlap(range1, range2),
 				"Document link ranges overlap:\nLink %d (target=%s): Line %d:%d to %d:%d\nLink %d (target=%s): Line %d:%d to %d:%d",
 				i, links[i].Target,
 				range1.Start.Line, range1.Start.Character,
@@ -165,14 +166,6 @@ func assertNoOverlappingRanges(t *testing.T, links []protocol.DocumentLink) {
 }
 
 // rangesOverlap returns true if two ranges overlap.
-func rangesOverlap(r1, r2 protocol.Range) bool {
-	// A range ends before another starts if r1.End <= r2.Start
-	// Ranges don't overlap if one ends before the other starts
-	if positionLessOrEqual(r1.End, r2.Start) || positionLessOrEqual(r2.End, r1.Start) {
-		return false
-	}
-	return true
-}
 
 // positionLessOrEqual returns true if pos1 <= pos2.
 func positionLessOrEqual(pos1, pos2 protocol.Position) bool {

private/buf/buflsp/format_test.go

@@ -16,12 +16,12 @@ package buflsp_test
 
 import (
 	"context"
-	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/bufbuild/buf/private/buf/buflsp"
 	"github.com/bufbuild/buf/private/buf/bufformat"
 	"github.com/bufbuild/buf/private/pkg/storage"
 	"github.com/bufbuild/buf/private/pkg/storage/storageos"
@@ -156,7 +156,7 @@ func TestFormatting(t *testing.T) {
 			}
 
 			originalContent := mustReadFile(t, testProtoPath)
-			result := applyTextEdits(originalContent, textEdits)
+			result := buflsp.ApplyTextEdits(originalContent, textEdits)
 			expectedFormatted := getExpectedFormattedContent(t, ctx, testProtoPath)
 
 			assert.Equal(t, expectedFormatted, result)
@@ -179,7 +179,7 @@ func TestFormattingIdempotency(t *testing.T) {
 			// First format: get the formatted content
 			firstEdits := callFormatting(t, ctx, testProtoPath)
 			originalContent := mustReadFile(t, testProtoPath)
-			formattedContent := applyTextEdits(originalContent, firstEdits)
+			formattedContent := buflsp.ApplyTextEdits(originalContent, firstEdits)
 
 			// Write formatted content to a temp directory with necessary dependencies
 			tmpDir := setupTempFormatDir(t, testProtoPath, protoFile, formattedContent)
@@ -244,7 +244,7 @@ func assertEditsAreMinimal(t *testing.T, original, expected string, edits []prot
 		totalEditSize, fullReplaceSize)
 
 	// Verify edits actually transform original to expected
-	result := applyTextEdits(original, edits)
+	result := buflsp.ApplyTextEdits(original, edits)
 	assert.Equal(t, expected, result, "applying edits should produce expected formatted text")
 
 	// Verify no edit is entirely unchanged
@@ -257,11 +257,11 @@ func assertEditsDoNotOverlap(t *testing.T, edits []protocol.TextEdit) {
 
 	for i := 0; i < len(edits); i++ {
 		for j := i + 1; j < len(edits); j++ {
-			assert.Falsef(t, rangesOverlapFormat(edits[i].Range, edits[j].Range),
+			assert.Falsef(t, buflsp.RangesOverlap(edits[i].Range, edits[j].Range),
 				"edits %d and %d overlap: %s and %s",
 				i, j,
-				formatRange(edits[i].Range),
-				formatRange(edits[j].Range))
+				buflsp.FormatRange(edits[i].Range),
+				buflsp.FormatRange(edits[j].Range))
 		}
 	}
 }
@@ -288,112 +288,10 @@ func assertNoRedundantEdits(t *testing.T, original string, edits []protocol.Text
 
 		assert.NotEqualf(t, edit.NewText, replacedText,
 			"edit %d replaces text with identical text (not minimal): range %s",
-			i, formatRange(edit.Range))
+			i, buflsp.FormatRange(edit.Range))
 	}
 }
 
-func applyTextEdits(text string, edits []protocol.TextEdit) string {
-	lines := strings.Split(text, "\n")
-	// Apply edits in reverse order to maintain line/character positions
-	for i := len(edits) - 1; i >= 0; i-- {
-		edit := edits[i]
-		startLine := int(edit.Range.Start.Line)
-		startChar := int(edit.Range.Start.Character)
-		endLine := int(edit.Range.End.Line)
-		endChar := int(edit.Range.End.Character)
-
-		if startLine < 0 || startLine >= len(lines) {
-			continue
-		}
-		if endLine < 0 || endLine > len(lines) {
-			endLine = len(lines)
-		}
-
-		// LSP TextEdit semantics: Range.End is exclusive at character level
-		// Get the prefix (part of start line before the edit)
-		prefix := ""
-		if startLine < len(lines) && startChar <= len(lines[startLine]) {
-			prefix = lines[startLine][:startChar]
-		}
-
-		// Get the suffix (part of end line after the edit)
-		// If endChar is 0, we're at the start of endLine, so no suffix from that line
-		suffix := ""
-		if endChar > 0 && endLine < len(lines) {
-			if endChar <= len(lines[endLine]) {
-				suffix = lines[endLine][endChar:]
-			}
-		} else if startLine == endLine && endChar == 0 {
-			// Pure insertion at the beginning of a line - need the whole line as suffix
-			if endLine < len(lines) {
-				suffix = lines[endLine]
-			}
-		}
-
-		// Determine which lines to delete
-		// For a pure insertion (start == end), the line gets incorporated into replacement via suffix
-		// So we need to delete it from the original lines array
-		// Otherwise, we delete from startLine to endLine (inclusive or exclusive depending on endChar)
-		deleteEndLine := endLine
-		if startLine == endLine && startChar == endChar {
-			// Pure insertion - the line is preserved in suffix, so delete it from lines array
-			deleteEndLine = startLine
-		} else if endChar == 0 && endLine > startLine {
-			// Range ends at the beginning of endLine, so don't include endLine in deletion
-			deleteEndLine = endLine - 1
-		}
-
-		// Split the new text into lines
-		newText := edit.NewText
-		var newLines []string
-		endsWithNewline := false
-		if newText != "" {
-			newLines = strings.Split(newText, "\n")
-			// If newText ends with \n, split gives us a trailing empty string.
-			// This indicates that subsequent content should be on a new line.
-			endsWithNewline = strings.HasSuffix(newText, "\n")
-			if endsWithNewline && len(newLines) > 0 && newLines[len(newLines)-1] == "" {
-				newLines = newLines[:len(newLines)-1]
-			}
-		}
-
-		// Build the replacement
-		var replacement []string
-		if len(newLines) == 0 {
-			// No new content
-			combined := prefix + suffix
-			// Only create a line if there's actually content or we're doing a mid-line edit
-			if combined != "" || (startChar > 0 || endChar > 0) {
-				replacement = []string{combined}
-			} else {
-				// Deleting full lines with no replacement
-				replacement = []string{}
-			}
-		} else {
-			// Add prefix to first line
-			replacement = make([]string, len(newLines))
-			copy(replacement, newLines)
-			replacement[0] = prefix + replacement[0]
-			// Add suffix: if newText ended with \n, suffix goes on a new line
-			// Otherwise, append it to the last line
-			if suffix != "" {
-				if endsWithNewline {
-					replacement = append(replacement, suffix)
-				} else {
-					replacement[len(replacement)-1] = replacement[len(replacement)-1] + suffix
-				}
-			} else if startLine == endLine && startChar == endChar && endsWithNewline {
-				// Pure insertion at start of line where newText ends with newline
-				// Even if the original line is blank (suffix == ""), we should preserve it
-				replacement = append(replacement, "")
-			}
-		}
-
-		// Replace lines[startLine:deleteEndLine+1] with replacement
-		lines = append(lines[:startLine], append(replacement, lines[deleteEndLine+1:]...)...)
-	}
-	return strings.Join(lines, "\n")
-}
 
 func TestApplyTextEdits(t *testing.T) {
 	t.Parallel()
@@ -460,7 +358,7 @@ func TestApplyTextEdits(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := applyTextEdits(tt.input, tt.edits)
+			result := buflsp.ApplyTextEdits(tt.input, tt.edits)
 			assert.Equal(t, tt.expected, result)
 		})
 	}
@@ -475,7 +373,7 @@ func TestApplyTextEditsDebug(t *testing.T) {
 
 	edits := callFormatting(t, ctx, testProtoPath)
 	originalContent := mustReadFile(t, testProtoPath)
-	result := applyTextEdits(originalContent, edits)
+	result := buflsp.ApplyTextEdits(originalContent, edits)
 	expected := getExpectedFormattedContent(t, ctx, testProtoPath)
 
 	assert.Equal(t, expected, result, "Result doesn't match expected")
@@ -554,24 +452,3 @@ func setupTempFormatDir(t *testing.T, originalPath, protoFile, formattedContent
 	return tmpDir
 }
 
-// rangesOverlap returns true if two LSP ranges overlap.
-// Two ranges overlap if one starts before the other ends.
-func rangesOverlapFormat(r1, r2 protocol.Range) bool {
-	// Compare positions: r1.Start < r2.End && r2.Start < r1.End
-	return positionLess(r1.Start, r2.End) && positionLess(r2.Start, r1.End)
-}
-
-// positionLess returns true if p1 is before p2 in the document.
-func positionLess(p1, p2 protocol.Position) bool {
-	if p1.Line != p2.Line {
-		return p1.Line < p2.Line
-	}
-	return p1.Character < p2.Character
-}
-
-// formatRange returns a human-readable string representation of an LSP range.
-func formatRange(r protocol.Range) string {
-	return fmt.Sprintf("[%d:%d-%d:%d]",
-		r.Start.Line, r.Start.Character,
-		r.End.Line, r.End.Character)
-}