diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go index 3088d438ef0..430258681f5 100644 --- a/go/packages/packagestest/expect.go +++ b/go/packages/packagestest/expect.go @@ -422,7 +422,7 @@ func (e *Exported) rangeConverter(n *expect.Note, args []interface{}) (span.Rang // end of file identifier, look up the current file f := e.ExpectFileSet.File(n.Pos) eof := f.Pos(f.Size()) - return span.Range{FileSet: e.ExpectFileSet, Start: eof, End: token.NoPos}, args, nil + return span.NewRange(e.ExpectFileSet, eof, token.NoPos), args, nil default: // look up an marker by name mark, ok := e.markers[string(arg)] @@ -439,7 +439,7 @@ func (e *Exported) rangeConverter(n *expect.Note, args []interface{}) (span.Rang if start == token.NoPos { return span.Range{}, nil, fmt.Errorf("%v: pattern %s did not match", e.ExpectFileSet.Position(n.Pos), arg) } - return span.Range{FileSet: e.ExpectFileSet, Start: start, End: end}, args, nil + return span.NewRange(e.ExpectFileSet, start, end), args, nil case *regexp.Regexp: start, end, err := expect.MatchBefore(e.ExpectFileSet, e.FileContents, n.Pos, arg) if err != nil { @@ -448,7 +448,7 @@ func (e *Exported) rangeConverter(n *expect.Note, args []interface{}) (span.Rang if start == token.NoPos { return span.Range{}, nil, fmt.Errorf("%v: pattern %s did not match", e.ExpectFileSet.Position(n.Pos), arg) } - return span.Range{FileSet: e.ExpectFileSet, Start: start, End: end}, args, nil + return span.NewRange(e.ExpectFileSet, start, end), args, nil default: return span.Range{}, nil, fmt.Errorf("cannot convert %v to pos", arg) } diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index d6c4bc06ed8..5d751ebe5a7 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -324,7 +324,7 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod Tok: tok, Mapper: &protocol.ColumnMapper{ URI: fh.URI(), - Converter: span.NewTokenConverter(fset, tok), + Converter: span.NewTokenConverter(tok), Content: src, }, ParseErr: parseErr, diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 05fb00500de..d8e66ab5835 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -322,7 +322,7 @@ func computeTextEdits(ctx context.Context, snapshot Snapshot, pgf *ParsedGoFile, // ProtocolEditsFromSource converts text edits to LSP edits using the original // source. -func ProtocolEditsFromSource(src []byte, edits []diff.TextEdit, converter span.Converter) ([]protocol.TextEdit, error) { +func ProtocolEditsFromSource(src []byte, edits []diff.TextEdit, converter *span.TokenConverter) ([]protocol.TextEdit, error) { m := lsppos.NewMapper(src) var result []protocol.TextEdit for _, edit := range edits { diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 768ff8c6367..a76c9162d1b 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -36,13 +36,8 @@ type MappedRange struct { // NewMappedRange returns a MappedRange for the given start and end token.Pos. 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, + spanRange: span.NewRange(fset, start, end), + m: m, } } diff --git a/internal/span/span.go b/internal/span/span.go index 4d2ad098667..fdf0644c9be 100644 --- a/internal/span/span.go +++ b/internal/span/span.go @@ -41,15 +41,6 @@ var Invalid = Span{v: span{Start: invalidPoint.v, End: invalidPoint.v}} 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 { - //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() @@ -217,28 +208,28 @@ func (s Span) Format(f fmt.State, c rune) { } } -func (s Span) WithPosition(c Converter) (Span, error) { +func (s Span) WithPosition(c *TokenConverter) (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) { +func (s Span) WithOffset(c *TokenConverter) (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) { +func (s Span) WithAll(c *TokenConverter) (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 { +func (s *Span) update(c *TokenConverter, withPos, withOffset bool) error { if !s.IsValid() { return fmt.Errorf("cannot add information to an invalid span") } @@ -265,7 +256,7 @@ func (s *Span) update(c Converter, withPos, withOffset bool) error { return nil } -func (p *point) updatePosition(c Converter) error { +func (p *point) updatePosition(c *TokenConverter) error { line, col, err := c.ToPosition(p.Offset) if err != nil { return err @@ -275,7 +266,7 @@ func (p *point) updatePosition(c Converter) error { return nil } -func (p *point) updateOffset(c Converter) error { +func (p *point) updateOffset(c *TokenConverter) error { offset, err := c.ToOffset(p.Line, p.Column) if err != nil { return err diff --git a/internal/span/span_test.go b/internal/span/span_test.go index 150ea3fbac9..3956565966c 100644 --- a/internal/span/span_test.go +++ b/internal/span/span_test.go @@ -6,6 +6,7 @@ package span_test import ( "fmt" + "go/token" "path/filepath" "strings" "testing" @@ -59,12 +60,15 @@ func toPath(value string) string { return filepath.FromSlash(value) } -type lines int - -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, error) { - return (int(l) * (line - 1)) + (col - 1), nil +// lines creates a new tokenConverter for a file with 1000 lines, each width +// bytes wide. +func lines(width int) *span.TokenConverter { + fset := token.NewFileSet() + f := fset.AddFile("", -1, 1000*width) + var lines []int + for i := 0; i < 1000; i++ { + lines = append(lines, i*width) + } + f.SetLines(lines) + return span.NewTokenConverter(f) } diff --git a/internal/span/token.go b/internal/span/token.go index 3fd22199d19..c2b65e7bec5 100644 --- a/internal/span/token.go +++ b/internal/span/token.go @@ -5,6 +5,7 @@ package span import ( + "errors" "fmt" "go/token" ) @@ -13,38 +14,37 @@ import ( // It also carries the FileSet that produced the positions, so that it is // self contained. type Range struct { - FileSet *token.FileSet - Start token.Pos - End token.Pos - Converter Converter + Start token.Pos + End token.Pos + file *token.File // possibly nil, if Start or End is invalid } -type FileConverter struct { - file *token.File -} - -// TokenConverter is a Converter backed by a token file set and file. -// It uses the file set methods to work out the conversions, which -// makes it fast and does not require the file contents. +// TokenConverter converts between offsets and (col, row) using a token.File. type TokenConverter struct { - FileConverter - fset *token.FileSet + file *token.File } // NewRange creates a new Range from a FileSet and two positions. // To represent a point pass a 0 as the end pos. func NewRange(fset *token.FileSet, start, end token.Pos) Range { + file := fset.File(start) + if file == nil { + // TODO: report a bug here via the bug reporting API, once it is available. + } return Range{ - FileSet: fset, - Start: start, - End: end, + Start: start, + End: end, + file: file, } } // NewTokenConverter returns an implementation of Converter backed by a // token.File. -func NewTokenConverter(fset *token.FileSet, f *token.File) *TokenConverter { - return &TokenConverter{fset: fset, FileConverter: FileConverter{file: f}} +func NewTokenConverter(f *token.File) *TokenConverter { + if f == nil { + // TODO: report a bug here using the bug reporting API. + } + return &TokenConverter{file: f} } // NewContentConverter returns an implementation of Converter for the @@ -53,7 +53,7 @@ func NewContentConverter(filename string, content []byte) *TokenConverter { fset := token.NewFileSet() f := fset.AddFile(filename, -1, len(content)) f.SetLinesForContent(content) - return NewTokenConverter(fset, f) + return NewTokenConverter(f) } // IsPoint returns true if the range represents a single point. @@ -68,16 +68,15 @@ func (r Range) Span() (Span, error) { if !r.Start.IsValid() { return Span{}, fmt.Errorf("start pos is not valid") } - f := r.FileSet.File(r.Start) - if f == nil { - return Span{}, fmt.Errorf("file not found in FileSet") + if r.file == nil { + return Span{}, errors.New("range missing file association") } - return FileSpan(f, r.Converter, r.Start, r.End) + return FileSpan(r.file, NewTokenConverter(r.file), r.Start, r.End) } // FileSpan returns a span within tok, using converter to translate between // offsets and positions. -func FileSpan(tok *token.File, converter Converter, start, end token.Pos) (Span, error) { +func FileSpan(tok *token.File, converter *TokenConverter, start, end token.Pos) (Span, error) { var s Span var err error var startFilename string @@ -107,7 +106,7 @@ func FileSpan(tok *token.File, converter Converter, start, end token.Pos) (Span, if startFilename != tok.Name() { return Span{}, fmt.Errorf("must supply Converter for file %q containing lines from %q", tok.Name(), startFilename) } - return s.WithOffset(&FileConverter{tok}) + return s.WithOffset(&TokenConverter{tok}) } func position(f *token.File, pos token.Pos) (string, int, int, error) { @@ -136,7 +135,7 @@ func positionFromOffset(f *token.File, offset int) (string, int, int, error) { // that it does not panic on invalid positions. func offset(f *token.File, pos token.Pos) (int, error) { if int(pos) < f.Base() || int(pos) > f.Base()+f.Size() { - return 0, fmt.Errorf("invalid pos") + return 0, fmt.Errorf("invalid pos: %d not in [%d, %d]", pos, f.Base(), f.Base()+f.Size()) } return int(pos) - f.Base(), nil } @@ -148,28 +147,30 @@ func (s Span) Range(converter *TokenConverter) (Range, error) { if err != nil { return Range{}, err } + file := converter.file // go/token will panic if the offset is larger than the file's size, // so check here to avoid panicking. - if s.Start().Offset() > converter.file.Size() { - return Range{}, fmt.Errorf("start offset %v is past the end of the file %v", s.Start(), converter.file.Size()) + if s.Start().Offset() > file.Size() { + // TODO: report a bug here via the future bug reporting API. + return Range{}, fmt.Errorf("start offset %v is past the end of the file %v", s.Start(), file.Size()) } - if s.End().Offset() > converter.file.Size() { - return Range{}, fmt.Errorf("end offset %v is past the end of the file %v", s.End(), converter.file.Size()) + if s.End().Offset() > file.Size() { + // TODO: report a bug here via the future bug reporting API. + return Range{}, fmt.Errorf("end offset %v is past the end of the file %v", s.End(), file.Size()) } return Range{ - FileSet: converter.fset, - Start: converter.file.Pos(s.Start().Offset()), - End: converter.file.Pos(s.End().Offset()), - Converter: converter, + Start: file.Pos(s.Start().Offset()), + End: file.Pos(s.End().Offset()), + file: file, }, nil } -func (l *FileConverter) ToPosition(offset int) (int, int, error) { +func (l *TokenConverter) ToPosition(offset int) (int, int, error) { _, line, col, err := positionFromOffset(l.file, offset) return line, col, err } -func (l *FileConverter) ToOffset(line, col int) (int, error) { +func (l *TokenConverter) ToOffset(line, col int) (int, error) { if line < 0 { return -1, fmt.Errorf("line is not valid") } diff --git a/internal/span/token_test.go b/internal/span/token_test.go index 81b263180ea..5f48d68b39b 100644 --- a/internal/span/token_test.go +++ b/internal/span/token_test.go @@ -48,7 +48,7 @@ func TestToken(t *testing.T) { } for _, test := range tokenTests { f := files[test.URI()] - c := span.NewTokenConverter(fset, f) + c := span.NewTokenConverter(f) t.Run(path.Base(f.Name()), func(t *testing.T) { checkToken(t, c, span.New( test.URI(),