From 761dbfd69de990f1b1cba240b6f41b4507896093 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Fri, 22 Nov 2019 15:20:08 -0500 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion.go | 17 ++--- internal/lsp/source/completion_literal.go | 7 +-- internal/lsp/source/folding_range.go | 24 +++---- internal/lsp/source/util.go | 17 +++-- internal/span/token.go | 76 ++++++++++++++--------- internal/span/token_test.go | 23 ++++--- 6 files changed, 86 insertions(+), 78 deletions(-) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 537264a967..88aeb73640 100644 --- a/internal/lsp/source/completion.go +++ b/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, - }, + // Overwrite the prefix only. + mappedRange: newMappedRange(c.snapshot.View().Session().Cache().FileSet(), c.mapper, ident.Pos(), ident.End()), } if c.opts.FuzzyMatching { @@ -273,12 +269,9 @@ func (c *completer) setSurrounding(ident *ast.Ident) { func (c *completer) getSurrounding() *Selection { if c.surrounding == nil { 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, - }, + content: "", + cursor: c.pos, + mappedRange: newMappedRange(c.snapshot.View().Session().Cache().FileSet(), c.mapper, c.pos, c.pos), } } return c.surrounding diff --git a/internal/lsp/source/completion_literal.go b/internal/lsp/source/completion_literal.go index 004950dd21..95286718d0 100644 --- a/internal/lsp/source/completion_literal.go +++ b/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 diff --git a/internal/lsp/source/folding_range.go b/internal/lsp/source/folding_range.go index 2fcb310a42..ee0d3c1028 100644 --- a/internal/lsp/source/folding_range.go +++ b/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,11 +82,8 @@ 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), - }, - Kind: kind, + mappedRange: newMappedRange(fset, m, start, end), + Kind: kind, } } @@ -171,11 +167,8 @@ 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), - }, - Kind: kind, + mappedRange: newMappedRange(fset, m, start, end), + Kind: kind, } } @@ -189,12 +182,9 @@ 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()), - }, - Kind: protocol.Comment, + // Fold from the end of the first line comment to the end of the comment block. + mappedRange: newMappedRange(fset, m, commentGrp.List[0].End(), commentGrp.End()), + Kind: protocol.Comment, }) } return comments diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 00470f6cd3..1cf8841f1b 100644 --- a/internal/lsp/source/util.go +++ b/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: diff --git a/internal/span/token.go b/internal/span/token.go index 01b5ed2d0a..4028eafa78 100644 --- a/internal/span/token.go +++ b/internal/span/token.go @@ -13,9 +13,10 @@ 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 + 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) + // In the presence of line directives, a single File can have sections from + // multiple file names. + 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 @@ -118,22 +142,16 @@ func (s Span) Range(converter *TokenConverter) (Range, error) { return Range{}, fmt.Errorf("end offset %v is past the end of the file %v", s.End(), converter.file.Size()) } return Range{ - FileSet: converter.fset, - Start: converter.file.Pos(s.Start().Offset()), - End: converter.file.Pos(s.End().Offset()), + 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) { diff --git a/internal/span/token_test.go b/internal/span/token_test.go index c9fce771d7..db11df11f0 100644 --- a/internal/span/token_test.go +++ b/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,16 +49,18 @@ func TestToken(t *testing.T) { for _, test := range tokenTests { f := files[test.URI()] c := span.NewTokenConverter(fset, f) - 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) + 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), + 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) + }) } }