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 <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

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{}

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{
+			spn := span.New(span.FileURI(f.Name()), span.NewPoint(0, 0, f.Offset(src)), span.Point{})
-				URI: span.FileURI(f.Name()),
-				Start: span.Point{
-					Offset: f.Offset(src),
-				},
-			}
-			spn.End = spn.Start
 			args = append(args, fmt.Sprint(spn))
 			app := &cmd.Application{}
 			app.Config = *exported.Config

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,
 		})

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) {

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)
 }

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 {
+	if _, ok := d[spn.URI()]; !ok {
-		d[spn.URI] = []protocol.Diagnostic{}
+		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)
+		got, err := mapper.Point(test.pos)
-		if got.Offset != test.want {
+		if err != nil && test.want != -1 {
-			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)
+			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)
+		spn, err := m.RangeSpan(edit.Range)
-		if spn.Start.Offset > prev {
+		if err != nil {
-			result = append(result, content[prev:spn.Start.Offset]...)
+			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:]...)

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 {
+func (m *ColumnMapper) Location(s span.Span) (Location, error) {
-	return Location{
+	rng, err := m.Range(s)
-		URI:   NewURI(s.URI),
+	if err != nil {
-		Range: m.Range(s),
+		return Location{}, err
 	}
+	return Location{URI: NewURI(s.URI()), Range: rng}, nil
 }
 
-func (m *ColumnMapper) Range(s span.Span) Range {
+func (m *ColumnMapper) Range(s span.Span) (Range, error) {
-	return Range{
+	if m.URI != s.URI() {
-		Start: m.Position(s.Start),
+		return Range{}, fmt.Errorf("column mapper is for file %q instead of %q", m.URI, s.URI())
-		End:   m.Position(s.End),
 	}
+	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 {
+func (m *ColumnMapper) Position(p span.Point) (Position, error) {
-	chr := span.ToUTF16Column(m.Converter, p, m.Content)
+	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 {
+func (m *ColumnMapper) PointSpan(p Position) (span.Span, error) {
-	return span.Span{
+	start, err := m.Point(p)
-		URI:   m.URI,
+	if err != nil {
-		Start: m.Point(l.Range.Start),
+		return span.Span{}, err
-		End:   m.Point(l.Range.End),
+	}
-	}.Clean(m.Converter)
+	return span.New(m.URI, start, start).WithAll(m.Converter)
 }
 
-func (m *ColumnMapper) RangeSpan(r Range) span.Span {
+func (m *ColumnMapper) Point(p Position) (span.Point, error) {
-	return span.Span{
+	line := int(p.Line) + 1
-		URI:   m.URI,
+	offset, err := m.Converter.ToOffset(line, 1)
-		Start: m.Point(r.Start),
+	if err != nil {
-		End:   m.Point(r.End),
+		return span.Point{}, err
-	}.Clean(m.Converter)
+	}
-}
+	lineStart := span.NewPoint(line, 1, offset)
-
+	return span.FromUTF16Column(lineStart, int(p.Character)+1, m.Content)
-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)
 }

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)
+		spn, err := m.RangeSpan(*change.Range)
-		if spn.Start.Offset <= 0 {
+		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)
+	spn, err := m.PointSpan(params.Position)
-	items, prefix, err := source.Completion(ctx, f, spn.Range(m.Converter).Start)
+	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)
+	spn, err := m.PointSpan(params.Position)
-	ident, err := source.Identifier(ctx, s.view, f, spn.Range(m.Converter).Start)
+	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)
+	spn, err := m.PointSpan(params.Position)
-	info, err := source.SignatureHelp(ctx, f, spn.Range(m.Converter).Start)
+	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)
+	spn, err := m.PointSpan(params.Position)
-	ident, err := source.Identifier(ctx, s.view, f, spn.Range(m.Converter).Start)
 	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)
+	spn, err := m.PointSpan(params.Position)
-	ident, err := source.Identifier(ctx, s.view, f, spn.Range(m.Converter).Start)
 	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)
 }
 

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)
+				s, err := spn.WithOffset(c)
-				if end := bytes.IndexAny(content[s.Start.Offset:], " \n,():;[]"); end > 0 {
+				//we just don't bother producing an error if this failed
-					spn.End = s.Start
+				if err == nil {
-					spn.End.Column += end
+					start := s.Start()
-					spn.End.Offset += end
+					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 {
+		if _, ok := reports[spn.URI()]; ok {
-			reports[spn.URI] = append(reports[spn.URI], diagnostic)
+			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),

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{
+		s := span.New(file.URI(), span.NewPoint(op.I1+1, 1, 0), span.NewPoint(op.I2+1, 1, 0))
-			Start: span.Point{Line: op.I1 + 1},
-			End:   span.Point{Line: op.I2 + 1},
-		}
 		switch op.Kind {
 		case diff.Delete:
 			// Delete: unformatted[i1:i2] is deleted.

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 New(NewURI(suf.remains), NewPoint(suf.num, hold, offset), Point{})
-		return Span{
-			URI:   NewURI(suf.remains),
-			Start: p,
-			End:   p,
-		}
 	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 New(NewURI(valid), NewPoint(hold, 0, offset), Point{})
-		return Span{
-			URI:   NewURI(valid),
-			Start: p,
-			End:   p,
-		}
 	}
 	// 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{
+		return New(NewURI(valid), end, Point{})
-			URI:   NewURI(valid),
-			Start: end,
-			End:   end,
-		}
 	}
 	valid = suf.remains
 	hold = suf.num
 	suf = rstripSuffix(suf.remains)
 	if suf.sep != ":" {
 		// line#offset only
-		return Span{
+		return New(NewURI(valid), NewPoint(hold, 0, offset), end)
-			URI:   NewURI(valid),
-			Start: Point{Line: clamp(hold), Column: 0, Offset: clamp(offset)},
-			End:   end,
-		}
 	}
 	// we have a column, so if end only had one number, it is also the column
 	if !hadCol {
-		end.Column = end.Line
+		end = NewPoint(suf.num, end.v.Line, end.v.Offset)
-		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,
 	}
+	return New(NewURI(suf.remains), NewPoint(suf.num, hold, offset), end)
 }
 
 type suffix struct {

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"`
+	v span
-	Start Point `json:"start"`
-	End   Point `json:"end"`
 }
 
 // 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
+var invalidPoint = Point{v: point{Line: 0, Column: 0, Offset: -1}}
-// 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)
-}
 
 // Converter is the interface to an object that can convert between line:column
 // and offset forms for a single file.
 type Converter interface {
-	Offsets
+	//ToPosition converts from an offset to a line:column pair.
-	Coords
+	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
+	if !s.IsValid() || (!fullForm && s.v.Start.isZero() && s.v.End.isZero()) {
-	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 {
 		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)
+	if s.IsPoint() {
-	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 {
 		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.
+func (s Span) WithPosition(c Converter) (Span, error) {
-// If the field is missing and Offsets is supplied it will be used to
+	if err := s.update(c, true, false); err != nil {
-// calculate it from the line and column.
+		return Span{}, err
-// The value will then be adjusted to the canonical form.
+	}
-func (s Span) CleanOffset(offsets Offsets) Span {
+	return s, nil
-	if offsets != nil {
+}
-		if (s.Start.Line > 1 || s.Start.Column > 1) && s.Start.Offset == 0 {
+
-			s.Start.updateOffset(offsets)
+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 {
+		if s.v.End.Offset == s.v.Start.Offset {
-			s.End.updateOffset(offsets)
+			s.v.End = s.v.Start
+		} else if err := s.v.End.updatePosition(c); err != nil {
+			return err
 		}
 	}
-	if s.Start.Offset < 0 {
+	if withOffset && !s.HasOffset() {
-		s.Start.Offset = 0
+		if err := s.v.Start.updateOffset(c); err != nil {
-	}
+			return err
-	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 s.End.Line == 0 && s.End.Offset > 0 {
+		if s.v.End.Line == s.v.Start.Line && s.v.End.Column == s.v.Start.Column {
-			s.End.Line, s.End.Column = coords.ToCoord(s.End.Offset)
+			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 {
+	return nil
-		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
 }
 
-// Clean returns a copy of the Span fully normalized.
+func (p *point) updatePosition(c Converter) error {
-// If passed a converter, it will use it to fill in any missing fields by
+	line, col, err := c.ToPosition(p.Offset)
-// converting between offset and line column fields.
+	if err != nil {
-// It does not attempt to validate that already filled fields have consistent
+		return err
-// 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
 	}
-	return s
+	p.Line = line
+	p.Column = col
+	return nil
 }
 
-// IsPoint returns true if the span represents a single point.
+func (p *point) updateOffset(c Converter) error {
-// It is only valid on spans that are "clean".
+	offset, err := c.ToOffset(p.Line, p.Column)
-func (s Span) IsPoint() bool {
+	if err != nil {
-	return s.Start == s.End
+		return err
-}
+	}
-
+	p.Offset = offset
-func (p *Point) updateOffset(offsets Offsets) {
+	return nil
-	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
 }

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_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-1:2#1"},
+		{"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:0#9990-1000:0#9990"},
+		{"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-14:9#138"},
+		{"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) {
+func (l lines) ToPosition(offset int) (int, int, error) {
-	return (offset / int(l)) + 1, (offset % int(l)) + 1
+	return (offset / int(l)) + 1, (offset % int(l)) + 1, nil
 }
 
-func (l lines) ToOffset(line, col int) int {
+func (l lines) ToOffset(line, col int) (int, error) {
-	return (int(l) * (line - 1)) + (col - 1)
+	return (int(l) * (line - 1)) + (col - 1), nil
 }

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())}
+	if f == nil {
-	s.Start.Offset = f.Offset(r.Start)
+		return Span{}, fmt.Errorf("file not found in FileSet")
-	if r.End.IsValid() {
-		s.End.Offset = f.Offset(r.End)
 	}
+	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 {
+func (s Span) Range(converter *TokenConverter) (Range, error) {
-	s = s.CleanOffset(converter)
+	s, err := s.WithOffset(converter)
+	if err != nil {
+		return Range{}, err
+	}
 	return Range{
 		FileSet: converter.fset,
-		Start:   converter.file.Pos(s.Start.Offset),
+		Start:   converter.file.Pos(s.Start().Offset()),
-		End:     converter.file.Pos(s.End.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, fmt.Errorf("line is not valid")
-		return -1
 	}
 	lineMax := l.file.LineCount() + 1
 	if line > lineMax {
-		// after the end of the file
+		return -1, fmt.Errorf("line is beyond end of file")
-		return -1
 	} else if line == lineMax {
 		if col > 1 {
-			// after the end of the file
+			return -1, fmt.Errorf("column is beyond end of file")
-			return -1
 		}
 		// 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
 }

internal/span/token_test.go

@@ -29,9 +29,9 @@ package test
 }
 
 var tokenTests = []span.Span{
-	{span.FileURI("/a.go"), span.Point{}, span.Point{}},
+	span.New(span.FileURI("/a.go"), span.NewPoint(1, 1, 0), span.Point{}),
-	{span.FileURI("/a.go"), span.Point{3, 7, 20}, span.Point{3, 7, 20}},
+	span.New(span.FileURI("/a.go"), span.NewPoint(3, 7, 20), span.NewPoint(3, 7, 20)),
-	{span.FileURI("/b.go"), span.Point{4, 9, 15}, span.Point{4, 13, 19}},
+	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{
+		checkToken(t, c, span.New(
-			URI:   test.URI,
+			test.URI(),
-			Start: span.Point{Line: test.Start.Line, Column: test.Start.Column},
+			span.NewPoint(test.Start().Line(), test.Start().Column(), 0),
-			End:   span.Point{Line: test.End.Line, Column: test.End.Column},
+			span.NewPoint(test.End().Line(), test.End().Column(), 0),
-		}, test)
+		), test)
-		checkToken(t, c, span.Span{
+		checkToken(t, c, span.New(
-			URI:   test.URI,
+			test.URI(),
-			Start: span.Point{Offset: test.Start.Offset},
+			span.NewPoint(0, 0, test.Start().Offset()),
-			End:   span.Point{Offset: test.End.Offset},
+			span.NewPoint(0, 0, test.End().Offset()),
-		}, test)
+		), test)
 	}
 }
 
 func checkToken(t *testing.T, c *span.TokenConverter, in, expect span.Span) {
-	rng := in.Range(c)
+	rng, err := in.Range(c)
-	gotLoc := rng.Span()
+	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 {

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 {
+func ToUTF16Column(p Point, content []byte) (int, error) {
-	if content == nil || p.Column < 0 {
+	if content == nil {
-		return -1
+		return -1, fmt.Errorf("ToUTF16Column: missing content")
 	}
-	if p.Column == 0 {
+	if !p.HasPosition() {
-		return 1
+		return -1, fmt.Errorf("ToUTF16Column: point is missing position")
 	}
-	// make sure we have a valid offset
+	if !p.HasOffset() {
-	p.updateOffset(offsets)
+		return -1, fmt.Errorf("ToUTF16Column: point is missing offset")
-	lineOffset := p.Offset - (p.Column - 1)
+	}
-	if lineOffset < 0 || p.Offset > len(content) {
+	offset := p.Offset()
-		return -1
+	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
+// FromUTF16Column advances the point by the utf16 character offset given the
-// offset given the supplied file contents.
+// 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 {
+func FromUTF16Column(p Point, chr int, content []byte) (Point, error) {
-	// first build a point for the start of the line the normal way
+	if !p.HasOffset() {
-	p := Point{Line: line, Column: 1, Offset: 0}
+		return Point{}, fmt.Errorf("FromUTF16Column: point is missing offset")
-	// now use that to work out the byte offset of the start of the line
-	p.updateOffset(offsets)
-	if chr <= 1 {
-		return p
 	}
-	// use that to pick the line out of the file content
+	// if chr is 1 then no adjustment needed
-	remains := content[p.Offset:]
+	if chr <= 1 {
-	// and now scan forward the specified number of characters
+		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.v.Column += w
-		p.Offset += w
+		p.v.Offset += w
 	}
-	return p
+	return p, nil
 }

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
+			offset, err := c.ToOffset(p.Line(), p.Column())
-			p.Offset = (line-1)*13 + (p.Column - 1)
+			if err != nil {
-			offset := c.ToOffset(p.Line, p.Column)
+				t.Error(err)
-			if p.Offset != offset {
+			}
-				t.Errorf("ToOffset(%v,%v): expected %v, got %v", p.Line, p.Column, p.Offset, offset)
+			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)
 			}
 		}
 	}