diff --git a/internal/lsp/definition.go b/internal/lsp/definition.go index 599228a89f..9487c68432 100644 --- a/internal/lsp/definition.go +++ b/internal/lsp/definition.go @@ -6,6 +6,7 @@ package lsp import ( "context" + "fmt" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" @@ -54,6 +55,9 @@ func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefini if err != nil { return nil, err } + if ident.Type.Object == nil { + return nil, fmt.Errorf("no type definition for %s", ident.Name) + } identRange, err := ident.Type.Range() if err != nil { return nil, err diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 641e463769..8e4069499c 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -198,7 +198,7 @@ func (r *renamer) update() (map[span.URI][]diff.TextEdit, error) { return nil, err } for _, ref := range r.refs { - refSpan, err := ref.spanRange.Span() + refSpan, err := ref.Span() if err != nil { return nil, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index a76c9162d1..ccade4aed2 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -18,6 +18,7 @@ import ( "strings" "golang.org/x/mod/modfile" + "golang.org/x/tools/internal/lsp/bug" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" ) @@ -25,25 +26,45 @@ import ( // MappedRange provides mapped protocol.Range for a span.Range, accounting for // UTF-16 code points. type MappedRange struct { - spanRange span.Range - m *protocol.ColumnMapper + spanRange span.Range // the range in the compiled source (package.CompiledGoFiles) + m *protocol.ColumnMapper // a mapper of the edited source (package.GoFiles) // protocolRange is the result of converting the spanRange using the mapper. - // It is computed on-demand. + // It is computed lazily. protocolRange *protocol.Range } // NewMappedRange returns a MappedRange for the given start and end token.Pos. +// +// By convention, start and end are assumed to be positions in the compiled (== +// type checked) source, whereas the column mapper m maps positions in the +// user-edited source. Note that these may not be the same, as when using CGo: +// CompiledGoFiles contains generated files, whose positions (via +// token.File.Position) point to locations in the edited file -- the file +// containing `import "C"`. func NewMappedRange(fset *token.FileSet, m *protocol.ColumnMapper, start, end token.Pos) MappedRange { + if tf := fset.File(start); tf == nil { + bug.Report("nil file", nil) + } else { + mapped := m.Converter.TokFile.Name() + adjusted := tf.PositionFor(start, true) // adjusted position + if adjusted.Filename != mapped { + bug.Reportf("mapped file %q does not match start position file %q", mapped, adjusted.Filename) + } + } return MappedRange{ spanRange: span.NewRange(fset, start, end), m: m, } } +// Range returns the LSP range in the edited source. +// +// See the documentation of NewMappedRange for information on edited vs +// compiled source. func (s MappedRange) Range() (protocol.Range, error) { if s.protocolRange == nil { - spn, err := s.spanRange.Span() + spn, err := s.Span() if err != nil { return protocol.Range{}, err } @@ -56,14 +77,33 @@ func (s MappedRange) Range() (protocol.Range, error) { return *s.protocolRange, nil } +// Span returns the span corresponding to the mapped range in the edited +// source. +// +// See the documentation of NewMappedRange for information on edited vs +// compiled source. func (s MappedRange) Span() (span.Span, error) { - return s.spanRange.Span() + // In the past, some code-paths have relied on Span returning an error if s + // is the zero value (i.e. s.m is nil). But this should be treated as a bug: + // observe that s.URI() would panic in this case. + if s.m == nil { + return span.Span{}, bug.Errorf("invalid range") + } + return span.FileSpan(s.spanRange.TokFile, s.m.Converter, s.spanRange.Start, s.spanRange.End) } +// SpanRange returns a span.Range in the compiled source. +// +// See the documentation of NewMappedRange for information on edited vs +// compiled source. func (s MappedRange) SpanRange() span.Range { return s.spanRange } +// URI returns the URI of the edited file. +// +// See the documentation of NewMappedRange for information on edited vs +// compiled source. func (s MappedRange) URI() span.URI { return s.m.URI } diff --git a/internal/lsp/source/util_test.go b/internal/lsp/source/util_test.go new file mode 100644 index 0000000000..2920774adb --- /dev/null +++ b/internal/lsp/source/util_test.go @@ -0,0 +1,83 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package source + +import ( + "bytes" + "go/scanner" + "go/token" + "testing" + + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/span" +) + +func TestMappedRangeAdjustment(t *testing.T) { + // Test that mapped range adjusts positions in compiled files to positions in + // the corresponding edited file. + + compiled := []byte(`// Generated. DO NOT EDIT. + +package p + +//line edited.go:3:1 +const a𐐀b = 42`) + edited := []byte(`package p + +const a𐐀b = 42`) + + fset := token.NewFileSet() + cf := scanFile(fset, "compiled.go", compiled) + ef := scanFile(fset, "edited.go", edited) + eURI := span.URIFromPath(ef.Name()) + + mapper := &protocol.ColumnMapper{ + URI: eURI, + Converter: span.NewTokenConverter(ef), + Content: edited, + } + + start := cf.Pos(bytes.Index(compiled, []byte("a𐐀b"))) + end := start + token.Pos(len("a𐐀b")) + mr := NewMappedRange(fset, mapper, start, end) + gotRange, err := mr.Range() + if err != nil { + t.Fatal(err) + } + wantRange := protocol.Range{ + Start: protocol.Position{Line: 2, Character: 6}, + End: protocol.Position{Line: 2, Character: 10}, + } + if gotRange != wantRange { + t.Errorf("NewMappedRange(...).Range(): got %v, want %v", gotRange, wantRange) + } + + // Verify that the mapped span is also in the edited file. + gotSpan, err := mr.Span() + if err != nil { + t.Fatal(err) + } + if gotURI := gotSpan.URI(); gotURI != eURI { + t.Errorf("mr.Span().URI() = %v, want %v", gotURI, eURI) + } + wantOffset := bytes.Index(edited, []byte("a𐐀b")) + if gotOffset := gotSpan.Start().Offset(); gotOffset != wantOffset { + t.Errorf("mr.Span().Start().Offset() = %d, want %d", gotOffset, wantOffset) + } +} + +// scanFile scans the a file into fset, in order to honor line directives. +func scanFile(fset *token.FileSet, name string, content []byte) *token.File { + f := fset.AddFile(name, -1, len(content)) + var s scanner.Scanner + s.Init(f, content, nil, scanner.ScanComments) + for { + _, tok, _ := s.Scan() + if tok == token.EOF { + break + } + } + return f +} diff --git a/internal/span/token.go b/internal/span/token.go index a64e5e88e3..6e62776582 100644 --- a/internal/span/token.go +++ b/internal/span/token.go @@ -5,7 +5,6 @@ package span import ( - "errors" "fmt" "go/token" @@ -18,12 +17,20 @@ import ( type Range struct { Start token.Pos End token.Pos - file *token.File // possibly nil, if Start or End is invalid + + // TokFile may be nil if Start or End is invalid. + // TODO: Eventually we should guarantee that it is non-nil. + TokFile *token.File } // TokenConverter converts between offsets and (col, row) using a token.File. +// +// TODO(rfindley): eliminate TokenConverter in favor of just operating on +// token.File. type TokenConverter struct { - file *token.File + // TokFile is exported for invariant checking; it may be nil in the case of + // an invalid converter. + TokFile *token.File } // NewRange creates a new Range from a FileSet and two positions. @@ -34,9 +41,9 @@ func NewRange(fset *token.FileSet, start, end token.Pos) Range { bug.Reportf("nil file") } return Range{ - Start: start, - End: end, - file: file, + Start: start, + End: end, + TokFile: file, } } @@ -46,7 +53,7 @@ func NewTokenConverter(f *token.File) *TokenConverter { if f == nil { bug.Reportf("nil file") } - return &TokenConverter{file: f} + return &TokenConverter{TokFile: f} } // NewContentConverter returns an implementation of Converter for the @@ -67,18 +74,22 @@ func (r Range) IsPoint() bool { // It will fill in all the members of the Span, calculating the line and column // information. func (r Range) Span() (Span, error) { - if !r.Start.IsValid() { - return Span{}, fmt.Errorf("start pos is not valid") - } - if r.file == nil { - return Span{}, errors.New("range missing file association") - } - return FileSpan(r.file, NewTokenConverter(r.file), r.Start, r.End) + return FileSpan(r.TokFile, NewTokenConverter(r.TokFile), r.Start, r.End) } // FileSpan returns a span within tok, using converter to translate between // offsets and positions. +// +// If non-nil, the converter must be a converter for the source file pointed to +// by start, after accounting for //line directives, as it will be used to +// compute offsets for the resulting Span. func FileSpan(tok *token.File, converter *TokenConverter, start, end token.Pos) (Span, error) { + if !start.IsValid() { + return Span{}, fmt.Errorf("start pos is not valid") + } + if tok == nil { + return Span{}, bug.Errorf("missing file association") // should never get here with a nil file + } var s Span var err error var startFilename string @@ -102,13 +113,13 @@ func FileSpan(tok *token.File, converter *TokenConverter, start, end token.Pos) s.v.Start.clean() s.v.End.clean() s.v.clean() - if converter != nil { - return s.WithOffset(converter) + if converter == nil { + converter = &TokenConverter{tok} } - if startFilename != tok.Name() { - return Span{}, fmt.Errorf("must supply Converter for file %q containing lines from %q", tok.Name(), startFilename) + if startFilename != converter.TokFile.Name() { + return Span{}, bug.Errorf("must supply Converter for file %q containing lines from %q", tok.Name(), startFilename) } - return s.WithOffset(&TokenConverter{tok}) + return s.WithOffset(converter) } func position(f *token.File, pos token.Pos) (string, int, int, error) { @@ -149,7 +160,7 @@ func (s Span) Range(converter *TokenConverter) (Range, error) { if err != nil { return Range{}, err } - file := converter.file + file := converter.TokFile // go/token will panic if the offset is larger than the file's size, // so check here to avoid panicking. if s.Start().Offset() > file.Size() { @@ -159,14 +170,14 @@ func (s Span) Range(converter *TokenConverter) (Range, error) { return Range{}, bug.Errorf("end offset %v is past the end of the file %v", s.End(), file.Size()) } return Range{ - Start: file.Pos(s.Start().Offset()), - End: file.Pos(s.End().Offset()), - file: file, + Start: file.Pos(s.Start().Offset()), + End: file.Pos(s.End().Offset()), + TokFile: file, }, nil } func (l *TokenConverter) ToPosition(offset int) (int, int, error) { - _, line, col, err := positionFromOffset(l.file, offset) + _, line, col, err := positionFromOffset(l.TokFile, offset) return line, col, err } @@ -174,7 +185,7 @@ func (l *TokenConverter) ToOffset(line, col int) (int, error) { if line < 0 { return -1, fmt.Errorf("line is not valid") } - lineMax := l.file.LineCount() + 1 + lineMax := l.TokFile.LineCount() + 1 if line > lineMax { return -1, fmt.Errorf("line is beyond end of file %v", lineMax) } else if line == lineMax { @@ -182,14 +193,14 @@ func (l *TokenConverter) ToOffset(line, col int) (int, error) { 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(), nil + return l.TokFile.Size(), nil } - pos := l.file.LineStart(line) + pos := l.TokFile.LineStart(line) if !pos.IsValid() { 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 offset(l.file, pos) + return offset(l.TokFile, pos) }