From 49b8ac185c84c5092be0953fb92b7660db9b8162 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Mon, 10 Feb 2020 20:10:59 -0800 Subject: [PATCH] internal/lsp/cache: add file contents to ParseGoHandle Currently there is no need for this because the file contents are part of the file handle. This change is in preparation for an impending improvement that tweaks the source code during the parse stage to fix certain kind of terminal parse errors. Any code that wants to use an *ast.File or *token.File in conjunction with the file contents needs access to the doctored source code so things line up. Change-Id: I59d83d3d6150aa1264761aa2c1f6c1269075a2ce Reviewed-on: https://go-review.googlesource.com/c/tools/+/218979 Run-TryBot: Muir Manders Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cache/check.go | 2 +- internal/lsp/cache/errors.go | 8 +++--- internal/lsp/cache/parse.go | 36 ++++++++++++++----------- internal/lsp/cache/pkg.go | 2 +- internal/lsp/cache/snapshot.go | 4 +-- internal/lsp/cache/view.go | 2 +- internal/lsp/link.go | 2 +- internal/lsp/source/completion.go | 10 +++---- internal/lsp/source/diagnostics.go | 2 +- internal/lsp/source/folding_range.go | 2 +- internal/lsp/source/format.go | 6 ++--- internal/lsp/source/highlight.go | 2 +- internal/lsp/source/identifier.go | 2 +- internal/lsp/source/implementation.go | 2 +- internal/lsp/source/signature_help.go | 2 +- internal/lsp/source/symbols.go | 2 +- internal/lsp/source/util.go | 6 ++--- internal/lsp/source/view.go | 4 +-- internal/lsp/source/workspace_symbol.go | 2 +- 19 files changed, 50 insertions(+), 48 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 37979db1c2..d09d69adb8 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -295,7 +295,7 @@ func typeCheck(ctx context.Context, fset *token.FileSet, m *metadata, mode sourc for i, ph := range pkg.compiledGoFiles { wg.Add(1) go func(i int, ph source.ParseGoHandle) { - files[i], _, parseErrors[i], actualErrors[i] = ph.Parse(ctx) + files[i], _, _, parseErrors[i], actualErrors[i] = ph.Parse(ctx) wg.Done() }(i, ph) } diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 46edd90539..787a43111a 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -181,7 +181,7 @@ func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos toke if err != nil { return span.Span{}, err } - _, m, _, err := ph.Cached() + _, _, m, _, err := ph.Cached() if err != nil { return span.Span{}, err } @@ -217,7 +217,7 @@ func scannerErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, posn if err != nil { return span.Span{}, err } - file, _, _, err := ph.Cached() + file, _, _, _, err := ph.Cached() if err != nil { return span.Span{}, err } @@ -236,7 +236,7 @@ func spanToRange(ctx context.Context, pkg *pkg, spn span.Span) (protocol.Range, if err != nil { return protocol.Range{}, err } - _, m, _, err := ph.Cached() + _, _, m, _, err := ph.Cached() if err != nil { return protocol.Range{}, err } @@ -276,7 +276,7 @@ func parseGoListImportCycleError(ctx context.Context, fset *token.FileSet, e pac // Imports have quotation marks around them. circImp := strconv.Quote(importList[1]) for _, ph := range pkg.compiledGoFiles { - fh, _, _, err := ph.Parse(ctx) + fh, _, _, _, err := ph.Parse(ctx) if err != nil { continue } diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 9404559504..6947c836e2 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -41,6 +41,7 @@ type parseGoHandle struct { type parseGoData struct { memoize.NoCopy + src []byte ast *ast.File parseError error // errors associated with parsing the file mapper *protocol.ColumnMapper @@ -54,9 +55,7 @@ func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) sourc } fset := c.fset h := c.store.Bind(key, func(ctx context.Context) interface{} { - data := &parseGoData{} - data.ast, data.mapper, data.parseError, data.err = parseGo(ctx, fset, fh, mode) - return data + return parseGo(ctx, fset, fh, mode) }) return &parseGoHandle{ handle: h, @@ -77,22 +76,22 @@ func (pgh *parseGoHandle) Mode() source.ParseMode { return pgh.mode } -func (pgh *parseGoHandle) Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) { +func (pgh *parseGoHandle) Parse(ctx context.Context) (*ast.File, []byte, *protocol.ColumnMapper, error, error) { v := pgh.handle.Get(ctx) if v == nil { - return nil, nil, nil, errors.Errorf("no parsed file for %s", pgh.File().Identity().URI) + return nil, nil, nil, nil, errors.Errorf("no parsed file for %s", pgh.File().Identity().URI) } data := v.(*parseGoData) - return data.ast, data.mapper, data.parseError, data.err + return data.ast, data.src, data.mapper, data.parseError, data.err } -func (pgh *parseGoHandle) Cached() (*ast.File, *protocol.ColumnMapper, error, error) { +func (pgh *parseGoHandle) Cached() (*ast.File, []byte, *protocol.ColumnMapper, error, error) { v := pgh.handle.Cached() if v == nil { - return nil, nil, nil, errors.Errorf("no cached AST for %s", pgh.file.Identity().URI) + return nil, nil, nil, nil, errors.Errorf("no cached AST for %s", pgh.file.Identity().URI) } data := v.(*parseGoData) - return data.ast, data.mapper, data.parseError, data.err + return data.ast, data.src, data.mapper, data.parseError, data.err } func hashParseKey(ph source.ParseGoHandle) string { @@ -110,16 +109,16 @@ func hashParseKeys(phs []source.ParseGoHandle) string { return hashContents(b.Bytes()) } -func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mode source.ParseMode) (file *ast.File, mapper *protocol.ColumnMapper, parseError error, err error) { +func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mode source.ParseMode) *parseGoData { ctx, done := trace.StartSpan(ctx, "cache.parseGo", telemetry.File.Of(fh.Identity().URI.Filename())) defer done() if fh.Identity().Kind != source.Go { - return nil, nil, nil, errors.Errorf("cannot parse non-Go file %s", fh.Identity().URI) + return &parseGoData{err: errors.Errorf("cannot parse non-Go file %s", fh.Identity().URI)} } buf, _, err := fh.Read(ctx) if err != nil { - return nil, nil, nil, err + return &parseGoData{err: err} } parseLimit <- struct{}{} defer func() { <-parseLimit }() @@ -127,13 +126,13 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod if mode == source.ParseHeader { parserMode = parser.ImportsOnly | parser.ParseComments } - file, parseError = parser.ParseFile(fset, fh.Identity().URI.Filename(), buf, parserMode) + file, parseError := parser.ParseFile(fset, fh.Identity().URI.Filename(), buf, parserMode) var tok *token.File if file != nil { // Fix any badly parsed parts of the AST. tok = fset.File(file.Pos()) if tok == nil { - return nil, nil, nil, errors.Errorf("successfully parsed but no token.File for %s (%v)", fh.Identity().URI, parseError) + return &parseGoData{err: errors.Errorf("successfully parsed but no token.File for %s (%v)", fh.Identity().URI, parseError)} } if mode == source.ParseExported { trimAST(file) @@ -149,7 +148,7 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod if err == nil { err = errors.Errorf("no AST for %s", fh.Identity().URI) } - return nil, nil, parseError, err + return &parseGoData{parseError: parseError, err: err} } m := &protocol.ColumnMapper{ URI: fh.Identity().URI, @@ -157,7 +156,12 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod Content: buf, } - return file, m, parseError, nil + return &parseGoData{ + src: buf, + ast: file, + mapper: m, + parseError: parseError, + } } // trimAST clears any part of the AST not relevant to type checking diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index 2272f823dd..e4b7ee64da 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -71,7 +71,7 @@ func (p *pkg) File(uri span.URI) (source.ParseGoHandle, error) { func (p *pkg) GetSyntax() []*ast.File { var syntax []*ast.File for _, ph := range p.compiledGoFiles { - file, _, _, err := ph.Cached() + file, _, _, _, err := ph.Cached() if err == nil { syntax = append(syntax, file) } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 8a96add2ab..897abc21c9 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -734,8 +734,8 @@ func (s *snapshot) shouldInvalidateMetadata(ctx context.Context, originalFH, cur return originalFH.Identity().URI == modfile } // Get the original and current parsed files in order to check package name and imports. - original, _, _, originalErr := s.view.session.cache.ParseGoHandle(originalFH, source.ParseHeader).Parse(ctx) - current, _, _, currentErr := s.view.session.cache.ParseGoHandle(currentFH, source.ParseHeader).Parse(ctx) + original, _, _, _, originalErr := s.view.session.cache.ParseGoHandle(originalFH, source.ParseHeader).Parse(ctx) + current, _, _, _, currentErr := s.view.session.cache.ParseGoHandle(currentFH, source.ParseHeader).Parse(ctx) if originalErr != nil || currentErr != nil { return (originalErr == nil) != (currentErr == nil) } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 29a6910c44..a1baf9288b 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -245,7 +245,7 @@ func (v *view) buildBuiltinPackage(ctx context.Context, goFiles []string) error fset := v.session.cache.fset h := v.session.cache.store.Bind(pgh.File().Identity(), func(ctx context.Context) interface{} { data := &builtinPackageData{} - file, _, _, err := pgh.Parse(ctx) + file, _, _, _, err := pgh.Parse(ctx) if err != nil { data.err = err return data diff --git a/internal/lsp/link.go b/internal/lsp/link.go index ddcb334abc..30e5a9ca5f 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -34,7 +34,7 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink if fh.Identity().Kind == source.Mod { return nil, nil } - file, m, _, err := view.Session().Cache().ParseGoHandle(fh, source.ParseFull).Parse(ctx) + file, _, m, _, err := view.Session().Cache().ParseGoHandle(fh, source.ParseFull).Parse(ctx) if err != nil { return nil, err } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 3bf3fed756..47d1f6765b 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -433,7 +433,7 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, pos proto if err != nil { return nil, nil, fmt.Errorf("getting file for Completion: %v", err) } - file, m, _, err := pgh.Cached() + file, src, m, _, err := pgh.Cached() if err != nil { return nil, nil, err } @@ -500,11 +500,9 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, pos proto // Otherwise, manually extract the prefix if our containing token // is a keyword. This improves completion after an "accidental // keyword", e.g. completing to "variance" in "someFunc(var<>)". - if contents, _, err := pgh.File().Read(ctx); err == nil { - id := scanKeyword(c.pos, c.snapshot.View().Session().Cache().FileSet().File(c.pos), contents) - if id != nil { - c.setSurrounding(id) - } + id := scanKeyword(c.pos, c.snapshot.View().Session().Cache().FileSet().File(c.pos), src) + if id != nil { + c.setSurrounding(id) } } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 2c8f3e9396..aff128ace8 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -223,7 +223,7 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m if err != nil { return err } - file, m, _, err := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseHeader).Parse(ctx) + file, _, m, _, err := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseHeader).Parse(ctx) if err != nil { log.Error(ctx, "could not parse go file when checking for missing modules", err) return err diff --git a/internal/lsp/source/folding_range.go b/internal/lsp/source/folding_range.go index e7b06e34cf..21cfda1a44 100644 --- a/internal/lsp/source/folding_range.go +++ b/internal/lsp/source/folding_range.go @@ -19,7 +19,7 @@ func FoldingRange(ctx context.Context, snapshot Snapshot, fh FileHandle, lineFol // TODO(suzmue): consider limiting the number of folding ranges returned, and // implement a way to prioritize folding ranges in that case. pgh := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseFull) - file, m, _, err := pgh.Parse(ctx) + file, _, m, _, err := pgh.Parse(ctx) if err != nil { return nil, err } diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 2149fea434..54f6d4a652 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -28,7 +28,7 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T defer done() pgh := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseFull) - file, m, parseErrors, err := pgh.Parse(ctx) + file, _, m, parseErrors, err := pgh.Parse(ctx) if err != nil { return nil, err } @@ -108,7 +108,7 @@ func computeImportEdits(ctx context.Context, view View, ph ParseGoHandle, option if err != nil { return nil, nil, err } - origAST, origMapper, _, err := ph.Parse(ctx) + origAST, _, origMapper, _, err := ph.Parse(ctx) if err != nil { return nil, nil, err } @@ -144,7 +144,7 @@ func computeOneImportFixEdits(ctx context.Context, view View, ph ParseGoHandle, if err != nil { return nil, err } - origAST, origMapper, _, err := ph.Parse(ctx) + origAST, _, origMapper, _, err := ph.Parse(ctx) if err != nil { return nil, err } diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index 07a2ac6161..6b78fa4292 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -27,7 +27,7 @@ func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protoc if err != nil { return nil, fmt.Errorf("getting file for Highlight: %v", err) } - file, m, _, err := pgh.Parse(ctx) + file, _, m, _, err := pgh.Parse(ctx) if err != nil { return nil, err } diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index bd3c8f4dc8..afd0b3f3be 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -56,7 +56,7 @@ func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, pos proto if err != nil { return nil, fmt.Errorf("getting file for Identifier: %v", err) } - file, m, _, err := pgh.Cached() + file, _, m, _, err := pgh.Cached() if err != nil { return nil, err } diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go index b2db818af1..9cd10ca376 100644 --- a/internal/lsp/source/implementation.go +++ b/internal/lsp/source/implementation.go @@ -241,7 +241,7 @@ func getASTFile(pkg Package, f FileHandle, pos protocol.Position) (*ast.File, to return nil, 0, err } - file, m, _, err := pgh.Cached() + file, _, m, _, err := pgh.Cached() if err != nil { return nil, 0, err } diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index e1b32ac3ad..05cad6afc9 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -26,7 +26,7 @@ func SignatureHelp(ctx context.Context, snapshot Snapshot, fh FileHandle, pos pr if err != nil { return nil, 0, fmt.Errorf("getting file for SignatureHelp: %v", err) } - file, m, _, err := pgh.Cached() + file, _, m, _, err := pgh.Cached() if err != nil { return nil, 0, err } diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index 2f964e4fe4..12afcb3b99 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -22,7 +22,7 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p if err != nil { return nil, fmt.Errorf("getting file for DocumentSymbols: %v", err) } - file, _, _, err := pgh.Cached() + file, _, _, _, err := pgh.Cached() if err != nil { return nil, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 6a17839883..1a66ad71b8 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -147,7 +147,7 @@ func IsGenerated(ctx context.Context, snapshot Snapshot, uri span.URI) bool { return false } ph := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseHeader) - parsed, _, _, err := ph.Parse(ctx) + parsed, _, _, _, err := ph.Parse(ctx) if err != nil { return false } @@ -647,7 +647,7 @@ func findPosInPackage(v View, searchpkg Package, pos token.Pos) (*ast.File, Pack if err != nil { return nil, nil, err } - file, _, _, err := ph.Cached() + file, _, _, _, err := ph.Cached() if err != nil { return nil, nil, err } @@ -671,7 +671,7 @@ func findMapperInPackage(v View, searchpkg Package, uri span.URI) (*protocol.Col if err != nil { return nil, err } - _, m, _, err := ph.Cached() + _, _, m, _, err := ph.Cached() if err != nil { return nil, err } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 6e956925ab..e535208d3b 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -248,10 +248,10 @@ type ParseGoHandle interface { // Parse returns the parsed AST for the file. // If the file is not available, returns nil and an error. - Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) + Parse(ctx context.Context) (file *ast.File, src []byte, m *protocol.ColumnMapper, parseErr error, err error) // Cached returns the AST for this handle, if it has already been stored. - Cached() (*ast.File, *protocol.ColumnMapper, error, error) + Cached() (file *ast.File, src []byte, m *protocol.ColumnMapper, parseErr error, err error) } // ModTidyHandle represents a handle to the modfile for a go.mod. diff --git a/internal/lsp/source/workspace_symbol.go b/internal/lsp/source/workspace_symbol.go index 76e7bcdd49..f9ca36735b 100644 --- a/internal/lsp/source/workspace_symbol.go +++ b/internal/lsp/source/workspace_symbol.go @@ -42,7 +42,7 @@ func WorkspaceSymbols(ctx context.Context, views []View, query string) ([]protoc } seen[pkg.PkgPath()] = struct{}{} for _, fh := range pkg.CompiledGoFiles() { - file, _, _, err := fh.Cached() + file, _, _, _, err := fh.Cached() if err != nil { return nil, err }