From 2f43c6d1a261a733f4aeb551a5d5b410f3c18233 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Fri, 15 Mar 2019 13:19:43 -0400 Subject: [PATCH] internal/span: change to private fields Change span to hide its fields and have validating accessors This catches the cases where either the offset or the position is being used when it was not set. It also normalizes the forms as the API now controls them, and allows us to simplify some of the logic. The converters are now allowed to return an error, which lets us cleanly propagate bad cases. The lsp was then converted to the new format, and also had some error checking of its own added on the top. All this allowed me to find and fix a few issues, most notably a case where the wrong column mapper was being used during the conversion of definition results. Change-Id: Iebdf8901e8269b28aaef60caf76574baa25c46d4 Reviewed-on: https://go-review.googlesource.com/c/tools/+/167858 Run-TryBot: Ian Cottrell TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cmd/definition.go | 21 +- internal/lsp/cmd/definition_test.go | 8 +- internal/lsp/diagnostics.go | 8 +- internal/lsp/format.go | 21 +- internal/lsp/imports.go | 9 +- internal/lsp/lsp_test.go | 52 +++-- internal/lsp/protocol/span.go | 93 +++++---- internal/lsp/server.go | 118 ++++++++++-- internal/lsp/source/diagnostics.go | 29 ++- internal/lsp/source/format.go | 5 +- internal/span/parse.go | 39 +--- internal/span/span.go | 288 ++++++++++++++++------------ internal/span/span_test.go | 23 ++- internal/span/token.go | 58 +++--- internal/span/token_test.go | 38 ++-- internal/span/utf16.go | 66 ++++--- internal/span/utf16_test.go | 26 ++- 17 files changed, 549 insertions(+), 353 deletions(-) diff --git a/internal/lsp/cmd/definition.go b/internal/lsp/cmd/definition.go index ea7bab88..046dbc3f 100644 --- a/internal/lsp/cmd/definition.go +++ b/internal/lsp/cmd/definition.go @@ -59,14 +59,14 @@ func (d *definition) Run(ctx context.Context, args ...string) error { } view := cache.NewView(&d.query.app.Config) from := span.Parse(args[0]) - f, err := view.GetFile(ctx, from.URI) + f, err := view.GetFile(ctx, from.URI()) if err != nil { return err } tok := f.GetToken(ctx) - pos := tok.Pos(from.Start.Offset) + pos := tok.Pos(from.Start().Offset()) if !pos.IsValid() { - return fmt.Errorf("invalid position %v", from.Start.Offset) + return fmt.Errorf("invalid position %v", from) } ident, err := source.Identifier(ctx, view, f, pos) if err != nil { @@ -108,17 +108,26 @@ func buildDefinition(ctx context.Context, view source.View, ident *source.Identi if err != nil { return nil, err } + spn, err := ident.Declaration.Range.Span() + if err != nil { + return nil, err + } return &Definition{ - Span: ident.Declaration.Range.Span(), + Span: spn, Description: content, }, nil } func buildGuruDefinition(ctx context.Context, view source.View, ident *source.IdentifierInfo) (*guru.Definition, error) { - spn := ident.Declaration.Range.Span() + spn, err := ident.Declaration.Range.Span() + if err != nil { + return nil, err + } pkg := ident.File.GetPackage(ctx) // guru does not support ranges - spn.End = span.Point{} + if !spn.IsPoint() { + spn = span.New(spn.URI(), spn.Start(), spn.Start()) + } // Behavior that attempts to match the expected output for guru. For an example // of the format, see the associated definition tests. buf := &bytes.Buffer{} diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go index c6c7467d..8c2f530a 100644 --- a/internal/lsp/cmd/definition_test.go +++ b/internal/lsp/cmd/definition_test.go @@ -59,13 +59,7 @@ func TestDefinition(t *testing.T) { } args = append(args, "definition") f := fset.File(src) - spn := span.Span{ - URI: span.FileURI(f.Name()), - Start: span.Point{ - Offset: f.Offset(src), - }, - } - spn.End = spn.Start + spn := span.New(span.FileURI(f.Name()), span.NewPoint(0, 0, f.Offset(src)), span.Point{}) args = append(args, fmt.Sprint(spn)) app := &cmd.Application{} app.Config = *exported.Config diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 7393be21..9aa3c7aa 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -47,7 +47,7 @@ func (s *server) setContent(ctx context.Context, uri span.URI, content []byte) e func toProtocolDiagnostics(ctx context.Context, v source.View, diagnostics []source.Diagnostic) ([]protocol.Diagnostic, error) { reports := []protocol.Diagnostic{} for _, diag := range diagnostics { - _, m, err := newColumnMap(ctx, v, diag.Span.URI) + _, m, err := newColumnMap(ctx, v, diag.Span.URI()) if err != nil { return nil, err } @@ -62,9 +62,13 @@ func toProtocolDiagnostics(ctx context.Context, v source.View, diagnostics []sou case source.SeverityWarning: severity = protocol.SeverityWarning } + rng, err := m.Range(diag.Span) + if err != nil { + return nil, err + } reports = append(reports, protocol.Diagnostic{ Message: diag.Message, - Range: m.Range(diag.Span), + Range: rng, Severity: severity, Source: src, }) diff --git a/internal/lsp/format.go b/internal/lsp/format.go index 5ce6a346..5145e2a2 100644 --- a/internal/lsp/format.go +++ b/internal/lsp/format.go @@ -11,11 +11,14 @@ import ( // formatRange formats a document with a given range. func formatRange(ctx context.Context, v source.View, s span.Span) ([]protocol.TextEdit, error) { - f, m, err := newColumnMap(ctx, v, s.URI) + f, m, err := newColumnMap(ctx, v, s.URI()) + if err != nil { + return nil, err + } + rng, err := s.Range(m.Converter) if err != nil { return nil, err } - rng := s.Range(m.Converter) if rng.Start == rng.End { // If we have a single point, assume we want the whole file. tok := f.GetToken(ctx) @@ -28,21 +31,25 @@ func formatRange(ctx context.Context, v source.View, s span.Span) ([]protocol.Te if err != nil { return nil, err } - return toProtocolEdits(m, edits), nil + return toProtocolEdits(m, edits) } -func toProtocolEdits(m *protocol.ColumnMapper, edits []source.TextEdit) []protocol.TextEdit { +func toProtocolEdits(m *protocol.ColumnMapper, edits []source.TextEdit) ([]protocol.TextEdit, error) { if edits == nil { - return nil + return nil, nil } result := make([]protocol.TextEdit, len(edits)) for i, edit := range edits { + rng, err := m.Range(edit.Span) + if err != nil { + return nil, err + } result[i] = protocol.TextEdit{ - Range: m.Range(edit.Span), + Range: rng, NewText: edit.NewText, } } - return result + return result, nil } func newColumnMap(ctx context.Context, v source.View, uri span.URI) (source.File, *protocol.ColumnMapper, error) { diff --git a/internal/lsp/imports.go b/internal/lsp/imports.go index c20f1a61..3d634b2b 100644 --- a/internal/lsp/imports.go +++ b/internal/lsp/imports.go @@ -14,11 +14,14 @@ import ( ) func organizeImports(ctx context.Context, v source.View, s span.Span) ([]protocol.TextEdit, error) { - f, m, err := newColumnMap(ctx, v, s.URI) + f, m, err := newColumnMap(ctx, v, s.URI()) + if err != nil { + return nil, err + } + rng, err := s.Range(m.Converter) if err != nil { return nil, err } - rng := s.Range(m.Converter) if rng.Start == rng.End { // If we have a single point, assume we want the whole file. tok := f.GetToken(ctx) @@ -31,5 +34,5 @@ func organizeImports(ctx context.Context, v source.View, s span.Span) ([]protoco if err != nil { return nil, err } - return toProtocolEdits(m, edits), nil + return toProtocolEdits(m, edits) } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index eec3cb86..f1df8222 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -177,8 +177,8 @@ func (d diagnostics) test(t *testing.T, v source.View) int { func (d diagnostics) collect(e *packagestest.Exported, fset *token.FileSet, rng packagestest.Range, msgSource, msg string) { spn, m := testLocation(e, fset, rng) - if _, ok := d[spn.URI]; !ok { - d[spn.URI] = []protocol.Diagnostic{} + if _, ok := d[spn.URI()]; !ok { + d[spn.URI()] = []protocol.Diagnostic{} } // If a file has an empty diagnostic message, return. This allows us to // avoid testing diagnostics in files that may have a lot of them. @@ -186,16 +186,20 @@ func (d diagnostics) collect(e *packagestest.Exported, fset *token.FileSet, rng return } severity := protocol.SeverityError - if strings.Contains(string(spn.URI), "analyzer") { + if strings.Contains(string(spn.URI()), "analyzer") { severity = protocol.SeverityWarning } + dRng, err := m.Range(spn) + if err != nil { + return + } want := protocol.Diagnostic{ - Range: m.Range(spn), + Range: dRng, Severity: severity, Source: msgSource, Message: msg, } - d[spn.URI] = append(d[spn.URI], want) + d[spn.URI()] = append(d[spn.URI()], want) } // diffDiagnostics prints the diff between expected and actual diagnostics test @@ -431,18 +435,29 @@ func (d definitions) test(t *testing.T, s *server, typ bool) { func (d definitions) collect(e *packagestest.Exported, fset *token.FileSet, src, target packagestest.Range) { sSrc, mSrc := testLocation(e, fset, src) + lSrc, err := mSrc.Location(sSrc) + if err != nil { + return + } sTarget, mTarget := testLocation(e, fset, target) - d[mSrc.Location(sSrc)] = mTarget.Location(sTarget) + lTarget, err := mTarget.Location(sTarget) + if err != nil { + return + } + d[lSrc] = lTarget } func testLocation(e *packagestest.Exported, fset *token.FileSet, rng packagestest.Range) (span.Span, *protocol.ColumnMapper) { - spn := span.NewRange(fset, rng.Start, rng.End).Span() + spn, err := span.NewRange(fset, rng.Start, rng.End).Span() + if err != nil { + return spn, nil + } f := fset.File(rng.Start) content, err := e.FileContents(f.Name()) if err != nil { return spn, nil } - m := protocol.NewColumnMapper(spn.URI, fset, f, content) + m := protocol.NewColumnMapper(spn.URI(), fset, f, content) return spn, m } @@ -474,9 +489,12 @@ func TestBytesOffset(t *testing.T) { f := fset.AddFile(fname, -1, len(test.text)) f.SetLinesForContent([]byte(test.text)) mapper := protocol.NewColumnMapper(span.FileURI(fname), fset, f, []byte(test.text)) - got := mapper.Point(test.pos) - if got.Offset != test.want { - t.Errorf("want %d for %q(Line:%d,Character:%d), but got %d", test.want, test.text, int(test.pos.Line), int(test.pos.Character), got.Offset) + got, err := mapper.Point(test.pos) + if err != nil && test.want != -1 { + t.Errorf("unexpected error: %v", err) + } + if err == nil && got.Offset() != test.want { + t.Errorf("want %d for %q(Line:%d,Character:%d), but got %d", test.want, test.text, int(test.pos.Line), int(test.pos.Character), got.Offset()) } } } @@ -485,14 +503,18 @@ func applyEdits(m *protocol.ColumnMapper, content []byte, edits []protocol.TextE prev := 0 result := make([]byte, 0, len(content)) for _, edit := range edits { - spn := m.RangeSpan(edit.Range).Clean(nil) - if spn.Start.Offset > prev { - result = append(result, content[prev:spn.Start.Offset]...) + spn, err := m.RangeSpan(edit.Range) + if err != nil { + return nil, err + } + offset := spn.Start().Offset() + if offset > prev { + result = append(result, content[prev:offset]...) } if len(edit.NewText) > 0 { result = append(result, []byte(edit.NewText)...) } - prev = spn.End.Offset + prev = spn.End().Offset() } if prev < len(content) { result = append(result, content[prev:]...) diff --git a/internal/lsp/protocol/span.go b/internal/lsp/protocol/span.go index 52a95409..998856fa 100644 --- a/internal/lsp/protocol/span.go +++ b/internal/lsp/protocol/span.go @@ -7,7 +7,9 @@ package protocol import ( + "fmt" "go/token" + "golang.org/x/tools/internal/span" ) @@ -29,51 +31,74 @@ func NewColumnMapper(uri span.URI, fset *token.FileSet, f *token.File, content [ } } -func (m *ColumnMapper) Location(s span.Span) Location { - return Location{ - URI: NewURI(s.URI), - Range: m.Range(s), +func (m *ColumnMapper) Location(s span.Span) (Location, error) { + rng, err := m.Range(s) + if err != nil { + return Location{}, err } + return Location{URI: NewURI(s.URI()), Range: rng}, nil } -func (m *ColumnMapper) Range(s span.Span) Range { - return Range{ - Start: m.Position(s.Start), - End: m.Position(s.End), +func (m *ColumnMapper) Range(s span.Span) (Range, error) { + if m.URI != s.URI() { + return Range{}, fmt.Errorf("column mapper is for file %q instead of %q", m.URI, s.URI()) } + s, err := s.WithOffset(m.Converter) + if err != nil { + return Range{}, err + } + start, err := m.Position(s.Start()) + if err != nil { + return Range{}, err + } + end, err := m.Position(s.End()) + if err != nil { + return Range{}, err + } + return Range{Start: start, End: end}, nil } -func (m *ColumnMapper) Position(p span.Point) Position { - chr := span.ToUTF16Column(m.Converter, p, m.Content) +func (m *ColumnMapper) Position(p span.Point) (Position, error) { + chr, err := span.ToUTF16Column(p, m.Content) + if err != nil { + return Position{}, err + } return Position{ - Line: float64(p.Line - 1), + Line: float64(p.Line() - 1), Character: float64(chr - 1), + }, nil +} + +func (m *ColumnMapper) Span(l Location) (span.Span, error) { + return m.RangeSpan(l.Range) +} + +func (m *ColumnMapper) RangeSpan(r Range) (span.Span, error) { + start, err := m.Point(r.Start) + if err != nil { + return span.Span{}, err } + end, err := m.Point(r.End) + if err != nil { + return span.Span{}, err + } + return span.New(m.URI, start, end).WithAll(m.Converter) } -func (m *ColumnMapper) Span(l Location) span.Span { - return span.Span{ - URI: m.URI, - Start: m.Point(l.Range.Start), - End: m.Point(l.Range.End), - }.Clean(m.Converter) +func (m *ColumnMapper) PointSpan(p Position) (span.Span, error) { + start, err := m.Point(p) + if err != nil { + return span.Span{}, err + } + return span.New(m.URI, start, start).WithAll(m.Converter) } -func (m *ColumnMapper) RangeSpan(r Range) span.Span { - return span.Span{ - URI: m.URI, - Start: m.Point(r.Start), - End: m.Point(r.End), - }.Clean(m.Converter) -} - -func (m *ColumnMapper) PointSpan(p Position) span.Span { - return span.Span{ - URI: m.URI, - Start: m.Point(p), - }.Clean(m.Converter) -} - -func (m *ColumnMapper) Point(p Position) span.Point { - return span.FromUTF16Column(m.Converter, int(p.Line)+1, int(p.Character)+1, m.Content) +func (m *ColumnMapper) Point(p Position) (span.Point, error) { + line := int(p.Line) + 1 + offset, err := m.Converter.ToOffset(line, 1) + if err != nil { + return span.Point{}, err + } + lineStart := span.NewPoint(line, 1, offset) + return span.FromUTF16Column(lineStart, int(p.Character)+1, m.Content) } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index ea753b2c..3c3e6e2a 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -203,17 +203,21 @@ func (s *server) applyChanges(ctx context.Context, params *protocol.DidChangeTex } content := file.GetContent(ctx) for _, change := range params.ContentChanges { - spn := m.RangeSpan(*change.Range).Clean(nil) - if spn.Start.Offset <= 0 { + spn, err := m.RangeSpan(*change.Range) + if err != nil { + return "", err + } + if !spn.HasOffset() { return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "invalid range for content change") } - if spn.End.Offset <= spn.Start.Offset { + start, end := spn.Start().Offset(), spn.End().Offset() + if end <= start { return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "invalid range for content change") } var buf bytes.Buffer - buf.Write(content[:spn.Start.Offset]) + buf.Write(content[:start]) buf.WriteString(change.Text) - buf.Write(content[spn.End.Offset:]) + buf.Write(content[end:]) content = buf.Bytes() } return string(content), nil @@ -265,8 +269,15 @@ func (s *server) Completion(ctx context.Context, params *protocol.CompletionPara if err != nil { return nil, err } - spn := m.PointSpan(params.Position) - items, prefix, err := source.Completion(ctx, f, spn.Range(m.Converter).Start) + spn, err := m.PointSpan(params.Position) + if err != nil { + return nil, err + } + rng, err := spn.Range(m.Converter) + if err != nil { + return nil, err + } + items, prefix, err := source.Completion(ctx, f, rng.Start) if err != nil { return nil, err } @@ -285,8 +296,15 @@ func (s *server) Hover(ctx context.Context, params *protocol.TextDocumentPositio if err != nil { return nil, err } - spn := m.PointSpan(params.Position) - ident, err := source.Identifier(ctx, s.view, f, spn.Range(m.Converter).Start) + spn, err := m.PointSpan(params.Position) + if err != nil { + return nil, err + } + identRange, err := spn.Range(m.Converter) + if err != nil { + return nil, err + } + ident, err := source.Identifier(ctx, s.view, f, identRange.Start) if err != nil { return nil, err } @@ -295,7 +313,14 @@ func (s *server) Hover(ctx context.Context, params *protocol.TextDocumentPositio return nil, err } markdown := "```go\n" + content + "\n```" - rng := m.Range(ident.Range.Span()) + identSpan, err := ident.Range.Span() + if err != nil { + return nil, err + } + rng, err := m.Range(identSpan) + if err != nil { + return nil, err + } return &protocol.Hover{ Contents: protocol.MarkupContent{ Kind: protocol.Markdown, @@ -310,8 +335,15 @@ func (s *server) SignatureHelp(ctx context.Context, params *protocol.TextDocumen if err != nil { return nil, err } - spn := m.PointSpan(params.Position) - info, err := source.SignatureHelp(ctx, f, spn.Range(m.Converter).Start) + spn, err := m.PointSpan(params.Position) + if err != nil { + return nil, err + } + rng, err := spn.Range(m.Converter) + if err != nil { + return nil, err + } + info, err := source.SignatureHelp(ctx, f, rng.Start) if err != nil { return nil, err } @@ -323,12 +355,31 @@ func (s *server) Definition(ctx context.Context, params *protocol.TextDocumentPo if err != nil { return nil, err } - spn := m.PointSpan(params.Position) - ident, err := source.Identifier(ctx, s.view, f, spn.Range(m.Converter).Start) + spn, err := m.PointSpan(params.Position) if err != nil { return nil, err } - return []protocol.Location{m.Location(ident.Declaration.Range.Span())}, nil + rng, err := spn.Range(m.Converter) + if err != nil { + return nil, err + } + ident, err := source.Identifier(ctx, s.view, f, rng.Start) + if err != nil { + return nil, err + } + decSpan, err := ident.Declaration.Range.Span() + if err != nil { + return nil, err + } + _, decM, err := newColumnMap(ctx, s.view, decSpan.URI()) + if err != nil { + return nil, err + } + loc, err := decM.Location(decSpan) + if err != nil { + return nil, err + } + return []protocol.Location{loc}, nil } func (s *server) TypeDefinition(ctx context.Context, params *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { @@ -336,12 +387,31 @@ func (s *server) TypeDefinition(ctx context.Context, params *protocol.TextDocume if err != nil { return nil, err } - spn := m.PointSpan(params.Position) - ident, err := source.Identifier(ctx, s.view, f, spn.Range(m.Converter).Start) + spn, err := m.PointSpan(params.Position) if err != nil { return nil, err } - return []protocol.Location{m.Location(ident.Type.Range.Span())}, nil + rng, err := spn.Range(m.Converter) + if err != nil { + return nil, err + } + ident, err := source.Identifier(ctx, s.view, f, rng.Start) + if err != nil { + return nil, err + } + identSpan, err := ident.Type.Range.Span() + if err != nil { + return nil, err + } + _, identM, err := newColumnMap(ctx, s.view, identSpan.URI()) + if err != nil { + return nil, err + } + loc, err := identM.Location(identSpan) + if err != nil { + return nil, err + } + return []protocol.Location{loc}, nil } func (s *server) Implementation(context.Context, *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { @@ -365,7 +435,10 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara if err != nil { return nil, err } - spn := m.RangeSpan(params.Range) + spn, err := m.RangeSpan(params.Range) + if err != nil { + return nil, err + } edits, err := organizeImports(ctx, s.view, spn) if err != nil { return nil, err @@ -408,7 +481,7 @@ func (s *server) ColorPresentation(context.Context, *protocol.ColorPresentationP } func (s *server) Formatting(ctx context.Context, params *protocol.DocumentFormattingParams) ([]protocol.TextEdit, error) { - spn := span.Span{URI: span.URI(params.TextDocument.URI)} + spn := span.New(span.URI(params.TextDocument.URI), span.Point{}, span.Point{}) return formatRange(ctx, s.view, spn) } @@ -417,7 +490,10 @@ func (s *server) RangeFormatting(ctx context.Context, params *protocol.DocumentR if err != nil { return nil, err } - spn := m.RangeSpan(params.Range) + spn, err := m.RangeSpan(params.Range) + if err != nil { + return nil, err + } return formatRange(ctx, s.view, spn) } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index a0408138..2fede2bf 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "fmt" + "log" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/asmdecl" @@ -86,18 +87,21 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag spn := span.Parse(diag.Pos) if spn.IsPoint() && diag.Kind == packages.TypeError { // Don't set a range if it's anything other than a type error. - if diagFile, err := v.GetFile(ctx, spn.URI); err == nil { + if diagFile, err := v.GetFile(ctx, spn.URI()); err == nil { tok := diagFile.GetToken(ctx) if tok == nil { continue // ignore errors } content := diagFile.GetContent(ctx) c := span.NewTokenConverter(diagFile.GetFileSet(ctx), tok) - s := spn.CleanOffset(c) - if end := bytes.IndexAny(content[s.Start.Offset:], " \n,():;[]"); end > 0 { - spn.End = s.Start - spn.End.Column += end - spn.End.Offset += end + s, err := spn.WithOffset(c) + //we just don't bother producing an error if this failed + if err == nil { + start := s.Start() + offset := start.Offset() + if l := bytes.IndexAny(content[offset:], " \n,():;[]"); l > 0 { + spn = span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+l, offset+l)) + } } } } @@ -106,8 +110,8 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag Message: diag.Msg, Severity: SeverityError, } - if _, ok := reports[spn.URI]; ok { - reports[spn.URI] = append(reports[spn.URI], diagnostic) + if _, ok := reports[spn.URI()]; ok { + reports[spn.URI()] = append(reports[spn.URI()], diagnostic) } } if len(diags) > 0 { @@ -116,13 +120,18 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag // Type checking and parsing succeeded. Run analyses. runAnalyses(ctx, v, pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) { r := span.NewRange(v.FileSet(), diag.Pos, 0) - s := r.Span() + s, err := r.Span() + if err != nil { + //TODO: we could not process the diag.Pos, and thus have no valid span + //we don't have anywhere to put this error though + log.Print(err) + } category := a.Name if diag.Category != "" { category += "." + category } - reports[s.URI] = append(reports[s.URI], Diagnostic{ + reports[s.URI()] = append(reports[s.URI()], Diagnostic{ Source: category, Span: s, Message: fmt.Sprintf(diag.Message), diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 785ab9d9..cda09168 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -68,10 +68,7 @@ func computeTextEdits(ctx context.Context, file File, formatted string) (edits [ u := strings.SplitAfter(string(file.GetContent(ctx)), "\n") f := strings.SplitAfter(formatted, "\n") for _, op := range diff.Operations(u, f) { - s := span.Span{ - Start: span.Point{Line: op.I1 + 1}, - End: span.Point{Line: op.I2 + 1}, - } + s := span.New(file.URI(), span.NewPoint(op.I1+1, 1, 0), span.NewPoint(op.I2+1, 1, 0)) switch op.Kind { case diff.Delete: // Delete: unformatted[i1:i2] is deleted. diff --git a/internal/span/parse.go b/internal/span/parse.go index 9bef33da..b3f268a3 100644 --- a/internal/span/parse.go +++ b/internal/span/parse.go @@ -12,6 +12,8 @@ import ( // Parse returns the location represented by the input. // All inputs are valid locations, as they can always be a pure filename. +// The returned span will be normalized, and thus if printed may produce a +// different string. func Parse(input string) Span { // :0:0#0-0:0#0 valid := input @@ -30,29 +32,19 @@ func Parse(input string) Span { } switch { case suf.sep == ":": - p := Point{Line: clamp(suf.num), Column: clamp(hold), Offset: clamp(offset)} - return Span{ - URI: NewURI(suf.remains), - Start: p, - End: p, - } + return New(NewURI(suf.remains), NewPoint(suf.num, hold, offset), Point{}) case suf.sep == "-": // we have a span, fall out of the case to continue default: // separator not valid, rewind to either the : or the start - p := Point{Line: clamp(hold), Column: 0, Offset: clamp(offset)} - return Span{ - URI: NewURI(valid), - Start: p, - End: p, - } + return New(NewURI(valid), NewPoint(hold, 0, offset), Point{}) } // only the span form can get here // at this point we still don't know what the numbers we have mean // if have not yet seen a : then we might have either a line or a column depending // on whether start has a column or not // we build an end point and will fix it later if needed - end := Point{Line: clamp(suf.num), Column: clamp(hold), Offset: clamp(offset)} + end := NewPoint(suf.num, hold, offset) hold, offset = 0, 0 suf = rstripSuffix(suf.remains) if suf.sep == "#" { @@ -61,33 +53,20 @@ func Parse(input string) Span { } if suf.sep != ":" { // turns out we don't have a span after all, rewind - return Span{ - URI: NewURI(valid), - Start: end, - End: end, - } + return New(NewURI(valid), end, Point{}) } valid = suf.remains hold = suf.num suf = rstripSuffix(suf.remains) if suf.sep != ":" { // line#offset only - return Span{ - URI: NewURI(valid), - Start: Point{Line: clamp(hold), Column: 0, Offset: clamp(offset)}, - End: end, - } + return New(NewURI(valid), NewPoint(hold, 0, offset), end) } // we have a column, so if end only had one number, it is also the column if !hadCol { - end.Column = end.Line - end.Line = clamp(suf.num) - } - return Span{ - URI: NewURI(suf.remains), - Start: Point{Line: clamp(suf.num), Column: clamp(hold), Offset: clamp(offset)}, - End: end, + end = NewPoint(suf.num, end.v.Line, end.v.Offset) } + return New(NewURI(suf.remains), NewPoint(suf.num, hold, offset), end) } type suffix struct { diff --git a/internal/span/span.go b/internal/span/span.go index bdcee58b..b4b6f443 100644 --- a/internal/span/span.go +++ b/internal/span/span.go @@ -5,42 +5,119 @@ package span import ( + "encoding/json" "fmt" ) // Span represents a source code range in standardized form. type Span struct { - URI URI `json:"uri"` - Start Point `json:"start"` - End Point `json:"end"` + v span } // Point represents a single point within a file. // In general this should only be used as part of a Span, as on its own it // does not carry enough information. type Point struct { + v point +} + +type span struct { + URI URI `json:"uri"` + Start point `json:"start"` + End point `json:"end"` +} + +type point struct { Line int `json:"line"` Column int `json:"column"` Offset int `json:"offset"` } -// Offsets is the interface to an object that can convert to offset -// from line:column forms for a single file. -type Offsets interface { - ToOffset(line, col int) int -} - -// Coords is the interface to an object that can convert to line:column -// from offset forms for a single file. -type Coords interface { - ToCoord(offset int) (int, int) -} +var invalidPoint = Point{v: point{Line: 0, Column: 0, Offset: -1}} // Converter is the interface to an object that can convert between line:column // and offset forms for a single file. type Converter interface { - Offsets - Coords + //ToPosition converts from an offset to a line:column pair. + ToPosition(offset int) (int, int, error) + //ToOffset converts from a line:column pair to an offset. + ToOffset(line, col int) (int, error) +} + +func New(uri URI, start Point, end Point) Span { + s := Span{v: span{URI: uri, Start: start.v, End: end.v}} + s.v.clean() + return s +} + +func NewPoint(line, col, offset int) Point { + p := Point{v: point{Line: line, Column: col, Offset: offset}} + p.v.clean() + return p +} + +func (s Span) HasPosition() bool { return s.v.Start.hasPosition() } +func (s Span) HasOffset() bool { return s.v.Start.hasOffset() } +func (s Span) IsValid() bool { return s.v.Start.isValid() } +func (s Span) IsPoint() bool { return s.v.Start == s.v.End } +func (s Span) URI() URI { return s.v.URI } +func (s Span) Start() Point { return Point{s.v.Start} } +func (s Span) End() Point { return Point{s.v.End} } +func (s *Span) MarshalJSON() ([]byte, error) { return json.Marshal(&s.v) } +func (s *Span) UnmarshalJSON(b []byte) error { return json.Unmarshal(b, &s.v) } + +func (p Point) HasPosition() bool { return p.v.hasPosition() } +func (p Point) HasOffset() bool { return p.v.hasOffset() } +func (p Point) IsValid() bool { return p.v.isValid() } +func (p *Point) MarshalJSON() ([]byte, error) { return json.Marshal(&p.v) } +func (p *Point) UnmarshalJSON(b []byte) error { return json.Unmarshal(b, &p.v) } +func (p Point) Line() int { + if !p.v.hasPosition() { + panic(fmt.Errorf("position not set in %v", p.v)) + } + return p.v.Line +} +func (p Point) Column() int { + if !p.v.hasPosition() { + panic(fmt.Errorf("position not set in %v", p.v)) + } + return p.v.Column +} +func (p Point) Offset() int { + if !p.v.hasOffset() { + panic(fmt.Errorf("offset not set in %v", p.v)) + } + return p.v.Offset +} + +func (p point) hasPosition() bool { return p.Line > 0 } +func (p point) hasOffset() bool { return p.Offset >= 0 } +func (p point) isValid() bool { return p.hasPosition() || p.hasOffset() } +func (p point) isZero() bool { + return (p.Line == 1 && p.Column == 1) || (!p.hasPosition() && p.Offset == 0) +} + +func (s *span) clean() { + //this presumes the points are already clean + if !s.End.isValid() || (s.End == point{}) { + s.End = s.Start + } +} + +func (p *point) clean() { + if p.Line < 0 { + p.Line = 0 + } + if p.Column <= 0 { + if p.Line > 0 { + p.Column = 1 + } else { + p.Column = 0 + } + } + if p.Offset == 0 && (p.Line > 1 || p.Column > 1) { + p.Offset = -1 + } } // Format implements fmt.Formatter to print the Location in a standard form. @@ -50,153 +127,114 @@ func (s Span) Format(f fmt.State, c rune) { preferOffset := f.Flag('#') // we should always have a uri, simplify if it is file format //TODO: make sure the end of the uri is unambiguous - uri := string(s.URI) + uri := string(s.v.URI) if !fullForm { - if filename, err := s.URI.Filename(); err == nil { + if filename, err := s.v.URI.Filename(); err == nil { uri = filename } } fmt.Fprint(f, uri) - // see which bits of start to write - printOffset := fullForm || (s.Start.Offset > 0 && (preferOffset || s.Start.Line <= 0)) - printLine := fullForm || (s.Start.Line > 0 && !printOffset) - printColumn := fullForm || (printLine && (s.Start.Column > 1 || s.End.Column > 1)) - if !printLine && !printColumn && !printOffset { + if !s.IsValid() || (!fullForm && s.v.Start.isZero() && s.v.End.isZero()) { return } + // see which bits of start to write + printOffset := s.HasOffset() && (fullForm || preferOffset || !s.HasPosition()) + printLine := s.HasPosition() && (fullForm || !printOffset) + printColumn := printLine && (fullForm || (s.v.Start.Column > 1 || s.v.End.Column > 1)) fmt.Fprint(f, ":") if printLine { - fmt.Fprintf(f, "%d", clamp(s.Start.Line)) + fmt.Fprintf(f, "%d", s.v.Start.Line) } if printColumn { - fmt.Fprintf(f, ":%d", clamp(s.Start.Column)) + fmt.Fprintf(f, ":%d", s.v.Start.Column) } if printOffset { - fmt.Fprintf(f, "#%d", clamp(s.Start.Offset)) + fmt.Fprintf(f, "#%d", s.v.Start.Offset) } // start is written, do we need end? - printLine = fullForm || (printLine && s.End.Line > s.Start.Line) - isPoint := s.End.Line == s.Start.Line && s.End.Column == s.Start.Column - printColumn = fullForm || (printColumn && s.End.Column > 0 && !isPoint) - printOffset = fullForm || (printOffset && s.End.Offset > s.Start.Offset) - if !printLine && !printColumn && !printOffset { + if s.IsPoint() { return } + // we don't print the line if it did not change + printLine = fullForm || (printLine && s.v.End.Line > s.v.Start.Line) fmt.Fprint(f, "-") if printLine { - fmt.Fprintf(f, "%d", clamp(s.End.Line)) + fmt.Fprintf(f, "%d", s.v.End.Line) } if printColumn { if printLine { fmt.Fprint(f, ":") } - fmt.Fprintf(f, "%d", clamp(s.End.Column)) + fmt.Fprintf(f, "%d", s.v.End.Column) } if printOffset { - fmt.Fprintf(f, "#%d", clamp(s.End.Offset)) + fmt.Fprintf(f, "#%d", s.v.End.Offset) } } -// CleanOffset returns a copy of the Span with the Offset field updated. -// If the field is missing and Offsets is supplied it will be used to -// calculate it from the line and column. -// The value will then be adjusted to the canonical form. -func (s Span) CleanOffset(offsets Offsets) Span { - if offsets != nil { - if (s.Start.Line > 1 || s.Start.Column > 1) && s.Start.Offset == 0 { - s.Start.updateOffset(offsets) +func (s Span) WithPosition(c Converter) (Span, error) { + if err := s.update(c, true, false); err != nil { + return Span{}, err + } + return s, nil +} + +func (s Span) WithOffset(c Converter) (Span, error) { + if err := s.update(c, false, true); err != nil { + return Span{}, err + } + return s, nil +} + +func (s Span) WithAll(c Converter) (Span, error) { + if err := s.update(c, true, true); err != nil { + return Span{}, err + } + return s, nil +} + +func (s *Span) update(c Converter, withPos, withOffset bool) error { + if !s.IsValid() { + return fmt.Errorf("cannot add information to an invalid span") + } + if withPos && !s.HasPosition() { + if err := s.v.Start.updatePosition(c); err != nil { + return err } - if (s.End.Line > 1 || s.End.Column > 1) && s.End.Offset == 0 { - s.End.updateOffset(offsets) + if s.v.End.Offset == s.v.Start.Offset { + s.v.End = s.v.Start + } else if err := s.v.End.updatePosition(c); err != nil { + return err } } - if s.Start.Offset < 0 { - s.Start.Offset = 0 - } - if s.End.Offset <= s.Start.Offset { - s.End.Offset = s.Start.Offset - } - return s -} - -// CleanCoords returns a copy of the Span with the Line and Column fields -// cleaned. -// If the fields are missing and Coords is supplied it will be used to -// calculate them from the offset. -// The values will then be adjusted to the canonical form. -func (s Span) CleanCoords(coords Coords) Span { - if coords != nil { - if s.Start.Line == 0 && s.Start.Offset > 0 { - s.Start.Line, s.Start.Column = coords.ToCoord(s.Start.Offset) + if withOffset && !s.HasOffset() { + if err := s.v.Start.updateOffset(c); err != nil { + return err } - if s.End.Line == 0 && s.End.Offset > 0 { - s.End.Line, s.End.Column = coords.ToCoord(s.End.Offset) + if s.v.End.Line == s.v.Start.Line && s.v.End.Column == s.v.Start.Column { + s.v.End.Offset = s.v.Start.Offset + } else if err := s.v.End.updateOffset(c); err != nil { + return err } } - if s.Start.Line <= 0 { - s.Start.Line = 0 - } - if s.Start.Line == 0 { - s.Start.Column = 0 - } else if s.Start.Column <= 0 { - s.Start.Column = 0 - } - if s.End.Line < s.Start.Line { - s.End.Line = s.Start.Line - } - if s.End.Column < s.Start.Column { - s.End.Column = s.Start.Column - } - if s.Start.Column <= 1 && s.End.Column <= 1 { - s.Start.Column = 0 - s.End.Column = 0 - } - if s.Start.Line <= 1 && s.End.Line <= 1 && s.Start.Column <= 1 && s.End.Column <= 1 { - s.Start.Line = 0 - s.End.Line = 0 - } - return s + return nil } -// Clean returns a copy of the Span fully normalized. -// If passed a converter, it will use it to fill in any missing fields by -// converting between offset and line column fields. -// It does not attempt to validate that already filled fields have consistent -// values. -func (s Span) Clean(converter Converter) Span { - s = s.CleanOffset(converter) - s = s.CleanCoords(converter) - if s.End.Offset == 0 { - // in case CleanCoords adjusted the end position - s.End.Offset = s.Start.Offset +func (p *point) updatePosition(c Converter) error { + line, col, err := c.ToPosition(p.Offset) + if err != nil { + return err } - return s + p.Line = line + p.Column = col + return nil } -// IsPoint returns true if the span represents a single point. -// It is only valid on spans that are "clean". -func (s Span) IsPoint() bool { - return s.Start == s.End -} - -func (p *Point) updateOffset(offsets Offsets) { - p.Offset = 0 - if p.Line <= 0 { - return - } - c := p.Column - if c < 1 { - c = 1 - } - if p.Line == 1 && c == 1 { - return - } - p.Offset = offsets.ToOffset(p.Line, c) -} - -func clamp(v int) int { - if v < 0 { - return 0 - } - return v +func (p *point) updateOffset(c Converter) error { + offset, err := c.ToOffset(p.Line, p.Column) + if err != nil { + return err + } + p.Offset = offset + return nil } diff --git a/internal/span/span_test.go b/internal/span/span_test.go index 846e3fcc..2ce698d1 100644 --- a/internal/span/span_test.go +++ b/internal/span/span_test.go @@ -16,12 +16,12 @@ import ( var ( formats = []string{"%v", "%#v", "%+v"} tests = [][]string{ - {"C:/file_a", "C:/file_a", "file:///C:/file_a:0:0#0-0:0#0"}, - {"C:/file_b:1:2", "C:/file_b:#1", "file:///C:/file_b:1:2#1-1:2#1"}, - {"C:/file_c:1000", "C:/file_c:#9990", "file:///C:/file_c:1000:0#9990-1000:0#9990"}, - {"C:/file_d:14:9", "C:/file_d:#138", "file:///C:/file_d:14:9#138-14:9#138"}, + {"C:/file_a", "C:/file_a", "file:///C:/file_a:1:1#0"}, + {"C:/file_b:1:2", "C:/file_b:#1", "file:///C:/file_b:1:2#1"}, + {"C:/file_c:1000", "C:/file_c:#9990", "file:///C:/file_c:1000:1#9990"}, + {"C:/file_d:14:9", "C:/file_d:#138", "file:///C:/file_d:14:9#138"}, {"C:/file_e:1:2-7", "C:/file_e:#1-#6", "file:///C:/file_e:1:2#1-1:7#6"}, - {"C:/file_f:500-502", "C:/file_f:#4990-#5010", "file:///C:/file_f:500:0#4990-502:0#5010"}, + {"C:/file_f:500-502", "C:/file_f:#4990-#5010", "file:///C:/file_f:500:1#4990-502:1#5010"}, {"C:/file_g:3:7-8", "C:/file_g:#26-#27", "file:///C:/file_g:3:7#26-3:8#27"}, {"C:/file_h:3:7-4:8", "C:/file_h:#26-#37", "file:///C:/file_h:3:7#26-4:8#37"}, } @@ -39,7 +39,10 @@ func TestFormat(t *testing.T) { t.Errorf("printing %q got %q expected %q", text, got, expect) } } - complete := spn.Clean(converter) + complete, err := spn.WithAll(converter) + if err != nil { + t.Error(err) + } for fi, format := range []string{"%v", "%#v", "%+v"} { expect := toPath(test[fi]) if got := fmt.Sprintf(format, complete); got != expect { @@ -59,10 +62,10 @@ func toPath(value string) string { type lines int -func (l lines) ToCoord(offset int) (int, int) { - return (offset / int(l)) + 1, (offset % int(l)) + 1 +func (l lines) ToPosition(offset int) (int, int, error) { + return (offset / int(l)) + 1, (offset % int(l)) + 1, nil } -func (l lines) ToOffset(line, col int) int { - return (int(l) * (line - 1)) + (col - 1) +func (l lines) ToOffset(line, col int) (int, error) { + return (int(l) * (line - 1)) + (col - 1), nil } diff --git a/internal/span/token.go b/internal/span/token.go index fb428094..324c2587 100644 --- a/internal/span/token.go +++ b/internal/span/token.go @@ -5,6 +5,7 @@ package span import ( + "fmt" "go/token" ) @@ -35,13 +36,13 @@ func NewRange(fset *token.FileSet, start, end token.Pos) Range { } } -// NewTokenConverter returns an implementation of Coords and Offsets backed by a +// NewTokenConverter returns an implementation of Converter backed by a // token.File. func NewTokenConverter(fset *token.FileSet, f *token.File) *TokenConverter { return &TokenConverter{fset: fset, file: f} } -// NewContentConverter returns an implementation of Coords and Offsets for the +// NewContentConverter returns an implementation of Converter for the // given file content. func NewContentConverter(filename string, content []byte) *TokenConverter { fset := token.NewFileSet() @@ -58,57 +59,64 @@ func (r Range) IsPoint() bool { // Span converts a Range to a Span that represents the Range. // It will fill in all the members of the Span, calculating the line and column // information. -func (r Range) Span() Span { +func (r Range) Span() (Span, error) { f := r.FileSet.File(r.Start) - s := Span{URI: FileURI(f.Name())} - s.Start.Offset = f.Offset(r.Start) - if r.End.IsValid() { - s.End.Offset = f.Offset(r.End) + if f == nil { + return Span{}, fmt.Errorf("file not found in FileSet") } + s := Span{v: span{URI: FileURI(f.Name())}} + s.v.Start.Offset = f.Offset(r.Start) + if r.End.IsValid() { + s.v.End.Offset = f.Offset(r.End) + } + s.v.Start.clean() + s.v.End.clean() + s.v.clean() converter := NewTokenConverter(r.FileSet, f) - return s.CleanCoords(converter) + return s.WithPosition(converter) } // Range converts a Span to a Range that represents the Span for the supplied // File. -func (s Span) Range(converter *TokenConverter) Range { - s = s.CleanOffset(converter) +func (s Span) Range(converter *TokenConverter) (Range, error) { + s, err := s.WithOffset(converter) + if err != nil { + return Range{}, err + } return Range{ FileSet: converter.fset, - Start: converter.file.Pos(s.Start.Offset), - End: converter.file.Pos(s.End.Offset), - } + Start: converter.file.Pos(s.Start().Offset()), + End: converter.file.Pos(s.End().Offset()), + }, nil } -func (l *TokenConverter) ToCoord(offset int) (int, int) { +func (l *TokenConverter) ToPosition(offset int) (int, int, error) { + //TODO: check offset fits in file pos := l.file.Pos(offset) p := l.fset.Position(pos) - return p.Line, p.Column + return p.Line, p.Column, nil } -func (l *TokenConverter) ToOffset(line, col int) int { +func (l *TokenConverter) ToOffset(line, col int) (int, error) { if line < 0 { - // before the start of the file - return -1 + return -1, fmt.Errorf("line is not valid") } lineMax := l.file.LineCount() + 1 if line > lineMax { - // after the end of the file - return -1 + return -1, fmt.Errorf("line is beyond end of file") } else if line == lineMax { if col > 1 { - // after the end of the file - return -1 + return -1, fmt.Errorf("column is beyond end of file") } // at the end of the file, allowing for a trailing eol - return l.file.Size() + return l.file.Size(), nil } pos := lineStart(l.file, line) if !pos.IsValid() { - return -1 + return -1, fmt.Errorf("line is not in file") } // we assume that column is in bytes here, and that the first byte of a // line is at column 1 pos += token.Pos(col - 1) - return l.file.Offset(pos) + return l.file.Offset(pos), nil } diff --git a/internal/span/token_test.go b/internal/span/token_test.go index fa8f22e1..7e5b4674 100644 --- a/internal/span/token_test.go +++ b/internal/span/token_test.go @@ -29,9 +29,9 @@ package test } var tokenTests = []span.Span{ - {span.FileURI("/a.go"), span.Point{}, span.Point{}}, - {span.FileURI("/a.go"), span.Point{3, 7, 20}, span.Point{3, 7, 20}}, - {span.FileURI("/b.go"), span.Point{4, 9, 15}, span.Point{4, 13, 19}}, + span.New(span.FileURI("/a.go"), span.NewPoint(1, 1, 0), span.Point{}), + span.New(span.FileURI("/a.go"), span.NewPoint(3, 7, 20), span.NewPoint(3, 7, 20)), + span.New(span.FileURI("/b.go"), span.NewPoint(4, 9, 15), span.NewPoint(4, 13, 19)), } func TestToken(t *testing.T) { @@ -43,24 +43,30 @@ func TestToken(t *testing.T) { files[span.FileURI(f.uri)] = file } for _, test := range tokenTests { - f := files[test.URI] + f := files[test.URI()] c := span.NewTokenConverter(fset, f) - checkToken(t, c, span.Span{ - URI: test.URI, - Start: span.Point{Line: test.Start.Line, Column: test.Start.Column}, - End: span.Point{Line: test.End.Line, Column: test.End.Column}, - }, test) - checkToken(t, c, span.Span{ - URI: test.URI, - Start: span.Point{Offset: test.Start.Offset}, - End: span.Point{Offset: test.End.Offset}, - }, test) + checkToken(t, c, span.New( + test.URI(), + span.NewPoint(test.Start().Line(), test.Start().Column(), 0), + span.NewPoint(test.End().Line(), test.End().Column(), 0), + ), test) + checkToken(t, c, span.New( + test.URI(), + span.NewPoint(0, 0, test.Start().Offset()), + span.NewPoint(0, 0, test.End().Offset()), + ), test) } } func checkToken(t *testing.T, c *span.TokenConverter, in, expect span.Span) { - rng := in.Range(c) - gotLoc := rng.Span() + rng, err := in.Range(c) + if err != nil { + t.Error(err) + } + gotLoc, err := rng.Span() + if err != nil { + t.Error(err) + } expected := fmt.Sprintf("%+v", expect) got := fmt.Sprintf("%+v", gotLoc) if expected != got { diff --git a/internal/span/utf16.go b/internal/span/utf16.go index 79f35f2d..e1f5dd80 100644 --- a/internal/span/utf16.go +++ b/internal/span/utf16.go @@ -5,6 +5,7 @@ package span import ( + "fmt" "unicode/utf16" "unicode/utf8" ) @@ -13,50 +14,57 @@ import ( // supplied file contents. // This is used to convert from the native (always in bytes) column // representation and the utf16 counts used by some editors. -func ToUTF16Column(offsets Offsets, p Point, content []byte) int { - if content == nil || p.Column < 0 { - return -1 +func ToUTF16Column(p Point, content []byte) (int, error) { + if content == nil { + return -1, fmt.Errorf("ToUTF16Column: missing content") } - if p.Column == 0 { - return 1 + if !p.HasPosition() { + return -1, fmt.Errorf("ToUTF16Column: point is missing position") } - // make sure we have a valid offset - p.updateOffset(offsets) - lineOffset := p.Offset - (p.Column - 1) - if lineOffset < 0 || p.Offset > len(content) { - return -1 + if !p.HasOffset() { + return -1, fmt.Errorf("ToUTF16Column: point is missing offset") + } + offset := p.Offset() + col := p.Column() + if col == 1 { + // column 1, so it must be chr 1 + return 1, nil + } + // work out the offset at the start of the line using the column + lineOffset := offset - (col - 1) + if lineOffset < 0 || offset > len(content) { + return -1, fmt.Errorf("ToUTF16Column: offsets %v-%v outside file contents (%v)", lineOffset, offset, len(content)) } // use the offset to pick out the line start start := content[lineOffset:] // now truncate down to the supplied column - start = start[:p.Column] + start = start[:col] // and count the number of utf16 characters // in theory we could do this by hand more efficiently... - return len(utf16.Encode([]rune(string(start)))) + return len(utf16.Encode([]rune(string(start)))), nil } -// FromUTF16Column calculates the byte column expressed by the utf16 character -// offset given the supplied file contents. +// FromUTF16Column advances the point by the utf16 character offset given the +// supplied line contents. // This is used to convert from the utf16 counts used by some editors to the // native (always in bytes) column representation. -func FromUTF16Column(offsets Offsets, line, chr int, content []byte) Point { - // first build a point for the start of the line the normal way - p := Point{Line: line, Column: 1, Offset: 0} - // now use that to work out the byte offset of the start of the line - p.updateOffset(offsets) - if chr <= 1 { - return p +func FromUTF16Column(p Point, chr int, content []byte) (Point, error) { + if !p.HasOffset() { + return Point{}, fmt.Errorf("FromUTF16Column: point is missing offset") } - // use that to pick the line out of the file content - remains := content[p.Offset:] - // and now scan forward the specified number of characters + // if chr is 1 then no adjustment needed + if chr <= 1 { + return p, nil + } + remains := content[p.Offset():] + // scan forward the specified number of characters for count := 1; count < chr; count++ { if len(remains) <= 0 { - return Point{Offset: -1} + return Point{}, fmt.Errorf("FromUTF16Column: chr goes beyond the content") } r, w := utf8.DecodeRune(remains) if r == '\n' { - return Point{Offset: -1} + return Point{}, fmt.Errorf("FromUTF16Column: chr goes beyond the line") } remains = remains[w:] if r >= 0x10000 { @@ -67,8 +75,8 @@ func FromUTF16Column(offsets Offsets, line, chr int, content []byte) Point { break } } - p.Column += w - p.Offset += w + p.v.Column += w + p.v.Offset += w } - return p + return p, nil } diff --git a/internal/span/utf16_test.go b/internal/span/utf16_test.go index 007bab77..3a3b659d 100644 --- a/internal/span/utf16_test.go +++ b/internal/span/utf16_test.go @@ -39,22 +39,30 @@ func TestUTF16(t *testing.T) { runeChr = chr runeColumn = chr + 2 } - p := span.Point{Line: line, Column: runeColumn} + p := span.NewPoint(line, runeColumn, (line-1)*13+(runeColumn-1)) // check conversion to utf16 format - gotChr := span.ToUTF16Column(c, p, input) + gotChr, err := span.ToUTF16Column(p, input) + if err != nil { + t.Error(err) + } if runeChr != gotChr { t.Errorf("ToUTF16Column(%v): expected %v, got %v", p, runeChr, gotChr) } - // we deliberately delay setting the point's offset - p.Offset = (line-1)*13 + (p.Column - 1) - offset := c.ToOffset(p.Line, p.Column) - if p.Offset != offset { - t.Errorf("ToOffset(%v,%v): expected %v, got %v", p.Line, p.Column, p.Offset, offset) + offset, err := c.ToOffset(p.Line(), p.Column()) + if err != nil { + t.Error(err) + } + if p.Offset() != offset { + t.Errorf("ToOffset(%v,%v): expected %v, got %v", p.Line(), p.Column(), p.Offset(), offset) } // and check the conversion back - gotPoint := span.FromUTF16Column(c, p.Line, chr, input) + lineStart := span.NewPoint(p.Line(), 1, p.Offset()-(p.Column()-1)) + gotPoint, err := span.FromUTF16Column(lineStart, chr, input) + if err != nil { + t.Error(err) + } if p != gotPoint { - t.Errorf("FromUTF16Column(%v,%v): expected %v, got %v", p.Line, chr, p, gotPoint) + t.Errorf("FromUTF16Column(%v,%v): expected %v, got %v", p.Line(), chr, p, gotPoint) } } }