diff options
author | Rob Findley <rfindley@google.com> | 2021-05-10 11:22:03 -0400 |
---|---|---|
committer | Robert Findley <rfindley@google.com> | 2021-05-10 23:22:37 +0000 |
commit | 79d39ff544a3eea8246545cccc8add1739703f51 (patch) | |
tree | 5c8428ffe602f5bd215e163972235aea3d6c7f95 | |
parent | fa055457156641b311b331af56f3d2b3b4d103af (diff) | |
download | golang-x-tools-79d39ff544a3eea8246545cccc8add1739703f51.tar.gz |
internal/lsp/source/completion: avoid a panic in package completion
For golang/vscode-go#1486
Change-Id: I48939224778155964712192faf5a437ee10cd2e1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/318370
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
-rw-r--r-- | internal/lsp/debug/serve.go | 11 | ||||
-rw-r--r-- | internal/lsp/source/completion/package.go | 27 |
2 files changed, 20 insertions, 18 deletions
diff --git a/internal/lsp/debug/serve.go b/internal/lsp/debug/serve.go index d7dd5445d..b3699e18c 100644 --- a/internal/lsp/debug/serve.go +++ b/internal/lsp/debug/serve.go @@ -82,15 +82,14 @@ type State struct { bugs map[string]string } -func Bug(ctx context.Context, desc string) { - labels := [3]label.Label{ - tag.Bug.Of(desc), - } +func Bug(ctx context.Context, desc, format string, args ...interface{}) { + labels := []label.Label{tag.Bug.Of(desc)} _, file, line, ok := runtime.Caller(1) if ok { - labels[1] = tag.Callsite.Of(fmt.Sprintf("%s:%d", file, line)) + labels = append(labels, tag.Callsite.Of(fmt.Sprintf("%s:%d", file, line))) } - core.Export(ctx, core.MakeEvent(labels, nil)) + msg := fmt.Sprintf(format, args...) + event.Log(ctx, msg, labels...) } type bug struct { diff --git a/internal/lsp/source/completion/package.go b/internal/lsp/source/completion/package.go index aea414eec..d927fef97 100644 --- a/internal/lsp/source/completion/package.go +++ b/internal/lsp/source/completion/package.go @@ -17,6 +17,7 @@ import ( "strings" "unicode" + "golang.org/x/tools/internal/lsp/debug" "golang.org/x/tools/internal/lsp/fuzzy" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" @@ -43,7 +44,7 @@ func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh return nil, nil, err } - surrounding, err := packageCompletionSurrounding(snapshot.FileSet(), fh, pgf, rng.Start) + surrounding, err := packageCompletionSurrounding(ctx, snapshot.FileSet(), pgf, rng.Start) if err != nil { return nil, nil, errors.Errorf("invalid position for package completion: %w", err) } @@ -70,23 +71,25 @@ func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh // packageCompletionSurrounding returns surrounding for package completion if a // package completions can be suggested at a given position. A valid location // for package completion is above any declarations or import statements. -func packageCompletionSurrounding(fset *token.FileSet, fh source.FileHandle, pgf *source.ParsedGoFile, pos token.Pos) (*Selection, error) { - src, err := fh.Read() - if err != nil { - return nil, err - } +func packageCompletionSurrounding(ctx context.Context, fset *token.FileSet, pgf *source.ParsedGoFile, pos token.Pos) (*Selection, error) { // If the file lacks a package declaration, the parser will return an empty // AST. As a work-around, try to parse an expression from the file contents. - expr, _ := parser.ParseExprFrom(fset, fh.URI().Filename(), src, parser.Mode(0)) + filename := pgf.URI.Filename() + expr, _ := parser.ParseExprFrom(fset, filename, pgf.Src, parser.Mode(0)) if expr == nil { - return nil, fmt.Errorf("unparseable file (%s)", fh.URI()) + return nil, fmt.Errorf("unparseable file (%s)", pgf.URI) } tok := fset.File(expr.Pos()) + offset := pgf.Tok.Offset(pos) + if offset > tok.Size() { + debug.Bug(ctx, "out of bounds cursor", "cursor offset (%d) out of bounds for %s (size: %d)", offset, pgf.URI, tok.Size()) + return nil, fmt.Errorf("cursor out of bounds") + } cursor := tok.Pos(pgf.Tok.Offset(pos)) m := &protocol.ColumnMapper{ URI: pgf.URI, - Content: src, - Converter: span.NewContentConverter(fh.URI().Filename(), src), + Content: pgf.Src, + Converter: span.NewContentConverter(filename, pgf.Src), } // If we were able to parse out an identifier as the first expression from @@ -114,7 +117,7 @@ func packageCompletionSurrounding(fset *token.FileSet, fh source.FileHandle, pgf // *ast.BadDecl since it is a keyword. This logic would allow "package" to // appear on any line of the file as long as it's the first code expression // in the file. - lines := strings.Split(string(src), "\n") + lines := strings.Split(string(pgf.Src), "\n") cursorLine := tok.Line(cursor) if cursorLine <= 0 || cursorLine > len(lines) { return nil, fmt.Errorf("invalid line number") @@ -150,7 +153,7 @@ func packageCompletionSurrounding(fset *token.FileSet, fh source.FileHandle, pgf } // If the cursor is in a comment, don't offer any completions. - if cursorInComment(fset, cursor, src) { + if cursorInComment(fset, cursor, pgf.Src) { return nil, fmt.Errorf("cursor in comment") } |