internal/span: support line directives

When //line directives are in play, the ast.File's Offset function will
return offsets in the generated file. We want offsets in the authored
file, so we need to pass a Converter for the authored file, in addition
to the ast.File for the generated file. For the same reason, we have to
start (Range).Span() by translating into positions in the authored file,
then calculate offsets from that.

A lot of call sites outside of the LSP don't pass the Converter, but
they probably don't matter much. I think everything inside does because
it ends up using mappedRange.

Updates golang/go#35720.

Change-Id: I7be09b3a50720b078e862d48cfdb02208f8187ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208501
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/source/completion.go

@@ -18,7 +18,6 @@ import (
 	"golang.org/x/tools/internal/lsp/fuzzy"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/snippet"
-	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/trace"
 	errors "golang.org/x/xerrors"
 )
@@ -254,11 +253,8 @@ func (c *completer) setSurrounding(ident *ast.Ident) {
 	c.surrounding = &Selection{
 		content: ident.Name,
 		cursor:  c.pos,
-		mappedRange: mappedRange{
 		// Overwrite the prefix only.
-			spanRange: span.NewRange(c.snapshot.View().Session().Cache().FileSet(), ident.Pos(), c.pos),
-			m:         c.mapper,
-		},
+		mappedRange: newMappedRange(c.snapshot.View().Session().Cache().FileSet(), c.mapper, ident.Pos(), ident.End()),
 	}
 
 	if c.opts.FuzzyMatching {
@@ -275,10 +271,7 @@ func (c *completer) getSurrounding() *Selection {
 		c.surrounding = &Selection{
 			content:     "",
 			cursor:      c.pos,
-			mappedRange: mappedRange{
-				spanRange: span.NewRange(c.snapshot.View().Session().Cache().FileSet(), c.pos, c.pos),
-				m:         c.mapper,
-			},
+			mappedRange: newMappedRange(c.snapshot.View().Session().Cache().FileSet(), c.mapper, c.pos, c.pos),
 		}
 	}
 	return c.surrounding

internal/lsp/source/completion_literal.go

@@ -14,7 +14,6 @@ import (
 	"golang.org/x/tools/internal/lsp/diff"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/snippet"
-	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/log"
 )
 
@@ -159,11 +158,7 @@ func (c *completer) literal(literalType types.Type, imp *importInfo) {
 // referenceEdit produces text edits that prepend a "&" operator to the
 // specified node.
 func referenceEdit(fset *token.FileSet, m *protocol.ColumnMapper, node ast.Node) ([]protocol.TextEdit, error) {
-	rng := span.Range{
-		FileSet: fset,
-		Start:   node.Pos(),
-		End:     node.Pos(),
-	}
+	rng := newMappedRange(fset, m, node.Pos(), node.Pos())
 	spn, err := rng.Span()
 	if err != nil {
 		return nil, err

internal/lsp/source/folding_range.go

@@ -7,7 +7,6 @@ import (
 	"sort"
 
 	"golang.org/x/tools/internal/lsp/protocol"
-	"golang.org/x/tools/internal/span"
 )
 
 type FoldingRangeInfo struct {
@@ -83,10 +82,7 @@ func foldingRange(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node) *Fo
 		return nil
 	}
 	return &FoldingRangeInfo{
-		mappedRange: mappedRange{
-			m:         m,
-			spanRange: span.NewRange(fset, start, end),
-		},
+		mappedRange: newMappedRange(fset, m, start, end),
 		Kind:        kind,
 	}
 }
@@ -171,10 +167,7 @@ func lineFoldingRange(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node)
 		return nil
 	}
 	return &FoldingRangeInfo{
-		mappedRange: mappedRange{
-			m:         m,
-			spanRange: span.NewRange(fset, start, end),
-		},
+		mappedRange: newMappedRange(fset, m, start, end),
 		Kind:        kind,
 	}
 }
@@ -189,11 +182,8 @@ func commentsFoldingRange(fset *token.FileSet, m *protocol.ColumnMapper, file *a
 			continue
 		}
 		comments = append(comments, &FoldingRangeInfo{
-			mappedRange: mappedRange{
-				m: m,
 			// Fold from the end of the first line comment to the end of the comment block.
-				spanRange: span.NewRange(fset, commentGrp.List[0].End(), commentGrp.End()),
-			},
+			mappedRange: newMappedRange(fset, m, commentGrp.List[0].End(), commentGrp.End()),
 			Kind:        protocol.Comment,
 		})
 	}

internal/lsp/source/util.go

@@ -30,6 +30,18 @@ type mappedRange struct {
 	protocolRange *protocol.Range
 }
 
+func newMappedRange(fset *token.FileSet, m *protocol.ColumnMapper, start, end token.Pos) mappedRange {
+	return mappedRange{
+		spanRange: span.Range{
+			FileSet:   fset,
+			Start:     start,
+			End:       end,
+			Converter: m.Converter,
+		},
+		m: m,
+	}
+}
+
 func (s mappedRange) Range() (protocol.Range, error) {
 	if s.protocolRange == nil {
 		spn, err := s.spanRange.Span()
@@ -171,10 +183,7 @@ func posToRange(view View, m *protocol.ColumnMapper, pos, end token.Pos) (mapped
 	if !end.IsValid() {
 		return mappedRange{}, errors.Errorf("invalid position for %v", end)
 	}
-	return mappedRange{
-		m:         m,
-		spanRange: span.NewRange(view.Session().Cache().FileSet(), pos, end),
-	}, nil
+	return newMappedRange(view.Session().Cache().FileSet(), m, pos, end), nil
 }
 
 // Matches cgo generated comment as well as the proposed standard:

internal/span/token.go

@@ -16,6 +16,7 @@ type Range struct {
 	FileSet   *token.FileSet
 	Start     token.Pos
 	End       token.Pos
+	Converter Converter
 }
 
 // TokenConverter is a Converter backed by a token file set and file.
@@ -64,33 +65,56 @@ func (r Range) Span() (Span, error) {
 	if f == nil {
 		return Span{}, fmt.Errorf("file not found in FileSet")
 	}
-	s := Span{}
+	var s Span
 	var err error
-	s.v.Start.Offset, err = offset(f, r.Start)
+	var startFilename string
+	startFilename, s.v.Start.Line, s.v.Start.Column, err = position(f, r.Start)
 	if err != nil {
 		return Span{}, err
 	}
+	s.v.URI = FileURI(startFilename)
 	if r.End.IsValid() {
-		s.v.End.Offset, err = offset(f, r.End)
+		var endFilename string
+		endFilename, s.v.End.Line, s.v.End.Column, err = position(f, r.End)
 		if err != nil {
 			return Span{}, err
 		}
-	}
 		// In the presence of line directives, a single File can have sections from
 		// multiple file names.
-	filename := f.Position(r.Start).Filename
-	if r.End.IsValid() {
-		if endFilename := f.Position(r.End).Filename; filename != endFilename {
-			return Span{}, fmt.Errorf("span begins in file %q but ends in %q", filename, endFilename)
+		if endFilename != startFilename {
+			return Span{}, fmt.Errorf("span begins in file %q but ends in %q", startFilename, endFilename)
 		}
 	}
-	s.v.URI = FileURI(filename)
-
 	s.v.Start.clean()
 	s.v.End.clean()
 	s.v.clean()
-	converter := NewTokenConverter(r.FileSet, f)
-	return s.WithPosition(converter)
+	if r.Converter != nil {
+		return s.WithOffset(r.Converter)
+	}
+	if startFilename != f.Name() {
+		return Span{}, fmt.Errorf("must supply Converter for file %q containing lines from %q", f.Name(), startFilename)
+	}
+	return s.WithOffset(NewTokenConverter(r.FileSet, f))
+}
+
+func position(f *token.File, pos token.Pos) (string, int, int, error) {
+	off, err := offset(f, pos)
+	if err != nil {
+		return "", 0, 0, err
+	}
+	return positionFromOffset(f, off)
+}
+
+func positionFromOffset(f *token.File, offset int) (string, int, int, error) {
+	if offset > f.Size() {
+		return "", 0, 0, fmt.Errorf("offset %v is past the end of the file %v", offset, f.Size())
+	}
+	pos := f.Pos(offset)
+	p := f.Position(pos)
+	if offset == f.Size() {
+		return p.Filename, p.Line + 1, 1, nil
+	}
+	return p.Filename, p.Line, p.Column, nil
 }
 
 // offset is a copy of the Offset function in go/token, but with the adjustment
@@ -121,19 +145,13 @@ func (s Span) Range(converter *TokenConverter) (Range, error) {
 		FileSet:   converter.fset,
 		Start:     converter.file.Pos(s.Start().Offset()),
 		End:       converter.file.Pos(s.End().Offset()),
+		Converter: converter,
 	}, nil
 }
 
 func (l *TokenConverter) ToPosition(offset int) (int, int, error) {
-	if offset > l.file.Size() {
-		return 0, 0, fmt.Errorf("offset %v is past the end of the file %v", offset, l.file.Size())
-	}
-	pos := l.file.Pos(offset)
-	p := l.fset.Position(pos)
-	if offset == l.file.Size() {
-		return p.Line + 1, 1, nil
-	}
-	return p.Line, p.Column, nil
+	_, line, col, err := positionFromOffset(l.file, offset)
+	return line, col, err
 }
 
 func (l *TokenConverter) ToOffset(line, col int) (int, error) {

internal/span/token_test.go

@@ -7,6 +7,7 @@ package span_test
 import (
 	"fmt"
 	"go/token"
+	"path"
 	"testing"
 
 	"golang.org/x/tools/internal/span"
@@ -48,6 +49,7 @@ func TestToken(t *testing.T) {
 	for _, test := range tokenTests {
 		f := files[test.URI()]
 		c := span.NewTokenConverter(fset, f)
+		t.Run(path.Base(f.Name()), func(t *testing.T) {
 			checkToken(t, c, span.New(
 				test.URI(),
 				span.NewPoint(test.Start().Line(), test.Start().Column(), 0),
@@ -58,6 +60,7 @@ func TestToken(t *testing.T) {
 				span.NewPoint(0, 0, test.Start().Offset()),
 				span.NewPoint(0, 0, test.End().Offset()),
 			), test)
+		})
 	}
 }