LSP fix positional encoding for diagnostics (#4066)

Author: emcfarlane

private/buf/buflsp/file.go

@@ -44,6 +44,7 @@ import (
 	"github.com/bufbuild/protocompile/experimental/incremental"
 	"github.com/bufbuild/protocompile/experimental/incremental/queries"
 	"github.com/bufbuild/protocompile/experimental/ir"
+	"github.com/bufbuild/protocompile/experimental/report"
 	"github.com/bufbuild/protocompile/experimental/seq"
 	"github.com/bufbuild/protocompile/experimental/source"
 	"go.lsp.dev/protocol"
@@ -63,7 +64,7 @@ type file struct {
 	uri       protocol.URI
 	checkWork chan<- struct{}
 
-	text string
+	file *report.File
 	// Version is an opaque version identifier given to us by the LSP client. This
 	// is used in the protocol to disambiguate which version of a file e.g. publishing
 	// diagnostics or symbols an operating refers to.
@@ -153,13 +154,19 @@ func (f *file) ReadFromDisk(ctx context.Context) (err error) {
 		return nil
 	}
 
-	data, err := os.ReadFile(f.uri.Filename())
+	reader, err := os.Open(f.uri.Filename())
+	if err != nil {
+		return fmt.Errorf("could not open file %q from disk: %w", f.uri, err)
+	}
+	defer reader.Close()
+	text, err := readAllAsString(reader)
 	if err != nil {
 		return fmt.Errorf("could not read file %q from disk: %w", f.uri, err)
 	}
 
 	f.version = -1
-	f.text = string(data)
+	f.file = report.NewFile(f.objectInfo.Path(), text)
+	f.hasText = true
 	return nil
 }
 
@@ -170,7 +177,7 @@ func (f *file) Update(ctx context.Context, version int32, text string) {
 
 	f.lsp.logger.Info(fmt.Sprintf("new file version: %v, %v -> %v", f.uri, f.version, version))
 	f.version = version
-	f.text = text
+	f.file = report.NewFile(f.objectInfo.Path(), text)
 	f.hasText = true
 }
 
@@ -435,11 +442,11 @@ func (f *file) RefreshIR(ctx context.Context) {
 	)
 
 	openerMap := map[string]string{
-		f.objectInfo.Path(): f.text,
+		f.objectInfo.Path(): f.file.Text(),
 	}
 	files := []*file{f}
 	for path, file := range f.importToFile {
-		openerMap[path] = file.text
+		openerMap[path] = file.file.Text()
 		files = append(files, file)
 	}
 	opener := source.NewMap(openerMap)
@@ -939,7 +946,7 @@ func (f *file) newFileOpener() fileOpener {
 		if file == nil {
 			return nil, fmt.Errorf("%s: %w", path, fs.ErrNotExist)
 		}
-		return xio.CompositeReadCloser(strings.NewReader(file.text), xio.NopCloser), nil
+		return xio.CompositeReadCloser(strings.NewReader(file.file.Text()), xio.NopCloser), nil
 	}
 }
 
@@ -1095,24 +1102,18 @@ func (f *file) appendLintErrors(source string, err error) bool {
 	}
 
 	for _, annotation := range annotations.FileAnnotations() {
+		// Convert 1-indexed byte-based line/column to byte offset.
+		startLocation := f.file.InverseLocation(annotation.StartLine(), annotation.StartColumn(), positionalEncoding)
+		endLocation := f.file.InverseLocation(annotation.EndLine(), annotation.EndColumn(), positionalEncoding)
+		protocolRange := reportLocationsToProtocolRange(startLocation, endLocation)
 		f.diagnostics = append(f.diagnostics, protocol.Diagnostic{
-			Range: protocol.Range{
-				Start: protocol.Position{
-					Line:      uint32(annotation.StartLine()) - 1,
-					Character: uint32(annotation.StartColumn()) - 1,
-				},
-				End: protocol.Position{
-					Line:      uint32(annotation.EndLine()) - 1,
-					Character: uint32(annotation.EndColumn()) - 1,
-				},
-			},
+			Range:    protocolRange,
 			Code:     annotation.Type(),
 			Severity: protocol.DiagnosticSeverityWarning,
 			Source:   source,
 			Message:  annotation.Message(),
 		})
 	}
-
 	return true
 }
 
@@ -1149,3 +1150,11 @@ func (f *file) PublishDiagnostics(ctx context.Context) {
 type wktObjectInfo struct {
 	storage.ObjectInfo
 }
+
+func readAllAsString(reader io.Reader) (string, error) {
+	var builder strings.Builder
+	if _, err := io.Copy(&builder, reader); err != nil {
+		return "", err
+	}
+	return builder.String(), nil
+}

private/buf/buflsp/image.go

@@ -24,6 +24,7 @@ import (
 	"buf.build/go/standard/xslices"
 	"github.com/bufbuild/buf/private/bufpkg/bufimage"
 	"github.com/bufbuild/protocompile"
+	"github.com/bufbuild/protocompile/experimental/report"
 	"github.com/bufbuild/protocompile/linker"
 	"github.com/bufbuild/protocompile/parser"
 	"github.com/bufbuild/protocompile/protoutil"
@@ -75,7 +76,7 @@ func buildImage(
 		logger.Warn("error building image", slog.String("path", path), xslog.ErrorAttr(err))
 		if len(errorsWithPos) > 0 {
 			diagnostics = xslices.Map(errorsWithPos, func(errorWithPos reporter.ErrorWithPos) protocol.Diagnostic {
-				return newDiagnostic(errorWithPos, false)
+				return newDiagnostic(errorWithPos, false, opener, logger)
 			})
 		}
 	}
@@ -178,10 +179,40 @@ func buildImage(
 //
 // Unfortunately, protocompile's errors are currently too meagre to provide full code
 // spans; that will require a fix in the compiler.
-func newDiagnostic(err reporter.ErrorWithPos, isWarning bool) protocol.Diagnostic {
+func newDiagnostic(err reporter.ErrorWithPos, isWarning bool, opener fileOpener, logger *slog.Logger) protocol.Diagnostic {
+	// Read the file text for the error's filename to convert byte offset to UTF-16 column.
+	position := err.GetPosition()
+	filename := position.Filename
+	// Fallback to byte-based column (will be wrong for non-ASCII).
+	utf16Col := err.GetPosition().Col - 1
+
+	// TODO: this is a temporary workaround for old diagnostic errors.
+	// When using the new compiler these conversions will be already handled.
+	if rc, openErr := opener(filename); openErr == nil {
+		defer rc.Close()
+		text, readErr := readAllAsString(rc)
+		if readErr == nil {
+			file := report.NewFile(filename, text)
+			loc := file.Location(position.Offset, positionalEncoding)
+			utf16Col = loc.Column - 1
+		} else {
+			logger.Warn(
+				"failed to read file for diagnostic position encoding",
+				slog.String("filename", filename),
+				xslog.ErrorAttr(readErr),
+			)
+		}
+	} else {
+		logger.Warn(
+			"failed to open file for diagnostic position encoding",
+			slog.String("filename", filename),
+			xslog.ErrorAttr(openErr),
+		)
+	}
+
 	pos := protocol.Position{
 		Line:      uint32(err.GetPosition().Line - 1),
-		Character: uint32(err.GetPosition().Col - 1),
+		Character: uint32(utf16Col),
 	}
 
 	severity := protocol.DiagnosticSeverityError

private/buf/buflsp/server.go

@@ -289,7 +289,7 @@ func (s *server) Formatting(
 			warningErrorsWithPos = append(warningErrorsWithPos, errorWithPos)
 		},
 	))
-	parsed, err := parser.Parse(file.uri.Filename(), strings.NewReader(file.text), handler)
+	parsed, err := parser.Parse(file.uri.Filename(), strings.NewReader(file.file.Text()), handler)
 	if err == nil {
 		_, _ = parser.ResultFromAST(parsed, true, handler)
 	}
@@ -306,14 +306,14 @@ func (s *server) Formatting(
 		return nil, err
 	}
 	newText := out.String()
-	if newText == file.text {
+	if newText == file.file.Text() {
 		return nil, nil
 	}
 
 	// Calculate the end location for the file range.
-	endLine := strings.Count(file.text, "\n")
+	endLine := strings.Count(file.file.Text(), "\n")
 	endCharacter := 0
-	for _, char := range file.text[strings.LastIndexByte(file.text, '\n')+1:] {
+	for _, char := range file.file.Text()[strings.LastIndexByte(file.file.Text(), '\n')+1:] {
 		endCharacter += utf16.RuneLen(char)
 	}
 	return []protocol.TextEdit{

private/buf/buflsp/symbol.go

@@ -175,7 +175,7 @@ func (s *symbol) FormatDocs(ctx context.Context) string {
 	case *imported:
 		imported, _ := s.kind.(*imported)
 		// Provide a preview of the imported file.
-		return fmt.Sprintf("```proto\n%s\n```", imported.file.text)
+		return fmt.Sprintf("```proto\n%s\n```", imported.file.file.Text())
 	case *tag:
 		plural := func(i int) string {
 			if i == 1 {
@@ -398,6 +398,10 @@ func positionInRange(position protocol.Position, within protocol.Range) int {
 func reportSpanToProtocolRange(span report.Span) protocol.Range {
 	startLocation := span.File.Location(span.Start, positionalEncoding)
 	endLocation := span.File.Location(span.End, positionalEncoding)
+	return reportLocationsToProtocolRange(startLocation, endLocation)
+}
+
+func reportLocationsToProtocolRange(startLocation, endLocation report.Location) protocol.Range {
 	return protocol.Range{
 		Start: protocol.Position{
 			Line:      uint32(startLocation.Line - 1),