From 1cc945182204dc50f5760d9859d043a3d4e27047 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 17 Sep 2019 11:19:11 -0400 Subject: [PATCH] internal/lsp: distinguish parse errors from actual errors Parse errors need to be treated separately from actual errors when parsing a file. Parse errors are treated more like values, whereas actual errors should not be propagated to the user. This enables us to delete some of the special handling for context.Canceled errors. Change-Id: I93a02f22b3f54beccbd6bcf26f04bb8da0202c25 Reviewed-on: https://go-review.googlesource.com/c/tools/+/195997 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/builtin.go | 4 +- internal/lsp/cache/check.go | 11 ++-- internal/lsp/cache/load.go | 42 +++++++------ internal/lsp/cache/parse.go | 75 ++++++++++++------------ internal/lsp/cache/pkg.go | 13 +++- internal/lsp/diagnostics.go | 5 +- internal/lsp/link.go | 4 +- internal/lsp/source/completion.go | 12 ++-- internal/lsp/source/completion_format.go | 5 +- internal/lsp/source/diagnostics.go | 22 +++---- internal/lsp/source/folding_range.go | 5 +- internal/lsp/source/format.go | 30 ++++------ internal/lsp/source/highlight.go | 7 +-- internal/lsp/source/identifier.go | 15 ++--- internal/lsp/source/rename.go | 15 ++--- internal/lsp/source/signature_help.go | 13 ++-- internal/lsp/source/suggested_fix.go | 4 +- internal/lsp/source/symbols.go | 14 ++--- internal/lsp/source/util.go | 11 ++-- internal/lsp/source/view.go | 5 +- 20 files changed, 142 insertions(+), 170 deletions(-) diff --git a/internal/lsp/cache/builtin.go b/internal/lsp/cache/builtin.go index f70e4746d6..a313290858 100644 --- a/internal/lsp/cache/builtin.go +++ b/internal/lsp/cache/builtin.go @@ -43,8 +43,8 @@ func (view *view) buildBuiltinPackage(ctx context.Context) error { fh := view.session.GetFile(span.FileURI(filename)) ph := view.session.cache.ParseGoHandle(fh, source.ParseFull) view.builtin.files = append(view.builtin.files, ph) - file, _, err := ph.Parse(ctx) - if file == nil { + file, _, _, err := ph.Parse(ctx) + if err != nil { return err } files[filename] = file diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index d724926881..29dbb46a9e 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -258,15 +258,12 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle, m * go func(i int, ph source.ParseGoHandle) { defer wg.Done() - files[i], _, parseErrors[i] = ph.Parse(ctx) + files[i], _, parseErrors[i], _ = ph.Parse(ctx) }(i, ph) } wg.Wait() for _, err := range parseErrors { - if err == context.Canceled { - return nil, errors.Errorf("parsing files for %s: %v", m.pkgPath, err) - } if err != nil { imp.view.session.cache.appendPkgError(pkg, err) } @@ -350,9 +347,9 @@ func (imp *importer) cachePerFile(ctx context.Context, gof *goFile, ph source.Pa } gof.cphs[cph.m.id] = cph - file, _, err := ph.Parse(ctx) - if file == nil { - return errors.Errorf("no AST for %s: %v", ph.File().Identity().URI, err) + file, _, _, err := ph.Parse(ctx) + if err != nil { + return err } gof.imports = file.Imports return nil diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index d153af2ce0..3480366078 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -88,25 +88,33 @@ func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([] // checkMetadata determines if we should run go/packages.Load for this file. // If yes, update the metadata for the file and its package. -func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) ([]*metadata, error) { +func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) (metadata []*metadata, err error) { // Check if we need to re-run go/packages before loading the package. - f.mu.Lock() - runGopackages := v.shouldRunGopackages(ctx, f, fh) - metadata := f.metadata() - f.mu.Unlock() + var runGopackages bool + func() { + f.mu.Lock() + defer f.mu.Unlock() + + runGopackages, err = v.shouldRunGopackages(ctx, f, fh) + metadata = f.metadata() + }() + if err != nil { + return nil, err + } // The package metadata is correct as-is, so just return it. if !runGopackages { return metadata, nil } - // Check if the context has been canceled before calling packages.Load. + // Don't bother running go/packages if the context has been canceled. if ctx.Err() != nil { return nil, ctx.Err() } ctx, done := trace.StartSpan(ctx, "packages.Load", telemetry.File.Of(f.filename())) defer done() + pkgs, err := packages.Load(v.Config(ctx), fmt.Sprintf("file=%s", f.filename())) if len(pkgs) == 0 { if err == nil { @@ -185,18 +193,14 @@ func sameSet(x, y map[packagePath]struct{}) bool { // shouldRunGopackages reparses a file's package and import declarations to // determine if they have changed. // It assumes that the caller holds the lock on the f.mu lock. -func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.FileHandle) (result bool) { +func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.FileHandle) (result bool, err error) { if len(f.meta) == 0 || len(f.missingImports) > 0 { - return true + return true, nil } // Get file content in case we don't already have it. - parsed, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx) - if err == context.Canceled { - log.Error(ctx, "parsing file header", err, tag.Of("file", f.URI())) - return false - } - if parsed == nil { - return true + parsed, _, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx) + if err != nil { + return false, err } // Check if the package's name has changed, by checking if this is a filename // we already know about, and if so, check if its package name has changed. @@ -204,21 +208,21 @@ func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, fh source.Fil for _, uri := range m.files { if span.CompareURI(uri, f.URI()) == 0 { if m.name != parsed.Name.Name { - return true + return true, nil } } } } // If the package's imports have changed, re-run `go list`. if len(f.imports) != len(parsed.Imports) { - return true + return true, nil } for i, importSpec := range f.imports { if importSpec.Path.Value != parsed.Imports[i].Path.Value { - return true + return true, nil } } - return false + return false, nil } type importGraph struct { diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 2dda5b6c4d..b426a2e1b2 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -41,9 +41,10 @@ type parseGoHandle struct { type parseGoData struct { memoize.NoCopy - ast *ast.File - mapper *protocol.ColumnMapper - err error + ast *ast.File + parseError error // errors associated with parsing the file + mapper *protocol.ColumnMapper + err error } func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) source.ParseGoHandle { @@ -53,18 +54,7 @@ func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) sourc } h := c.store.Bind(key, func(ctx context.Context) interface{} { data := &parseGoData{} - data.ast, data.err = parseGo(ctx, c, fh, mode) - tok := c.FileSet().File(data.ast.Pos()) - if tok == nil { - return data - } - uri := fh.Identity().URI - content, _, err := fh.Read(ctx) - if err != nil { - data.err = err - return data - } - data.mapper = newColumnMapper(uri, c.FileSet(), tok, content) + data.ast, data.mapper, data.parseError, data.err = parseGo(ctx, c, fh, mode) return data }) return &parseGoHandle{ @@ -73,13 +63,6 @@ func (c *cache) ParseGoHandle(fh source.FileHandle, mode source.ParseMode) sourc mode: mode, } } -func newColumnMapper(uri span.URI, fset *token.FileSet, tok *token.File, content []byte) *protocol.ColumnMapper { - return &protocol.ColumnMapper{ - URI: uri, - Converter: span.NewTokenConverter(fset, tok), - Content: content, - } -} func (h *parseGoHandle) File() source.FileHandle { return h.file @@ -89,22 +72,22 @@ func (h *parseGoHandle) Mode() source.ParseMode { return h.mode } -func (h *parseGoHandle) Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error) { +func (h *parseGoHandle) Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) { v := h.handle.Get(ctx) if v == nil { - return nil, nil, ctx.Err() + return nil, nil, nil, ctx.Err() } data := v.(*parseGoData) - return data.ast, data.mapper, data.err + return data.ast, data.mapper, data.parseError, data.err } -func (h *parseGoHandle) Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error) { +func (h *parseGoHandle) Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) { v := h.handle.Cached() if v == nil { - return nil, nil, errors.Errorf("no cached value for %s", h.file.Identity().URI) + return nil, nil, nil, errors.Errorf("no cached AST for %s", h.file.Identity().URI) } data := v.(*parseGoData) - return data.ast, data.mapper, data.err + return data.ast, data.mapper, data.parseError, data.err } func hashParseKey(ph source.ParseGoHandle) string { @@ -122,13 +105,13 @@ func hashParseKeys(phs []source.ParseGoHandle) string { return hashContents(b.Bytes()) } -func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.ParseMode) (*ast.File, error) { +func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.ParseMode) (file *ast.File, mapper *protocol.ColumnMapper, parseError error, err error) { ctx, done := trace.StartSpan(ctx, "cache.parseGo", telemetry.File.Of(fh.Identity().URI.Filename())) defer done() buf, _, err := fh.Read(ctx) if err != nil { - return nil, err + return nil, nil, nil, err } parseLimit <- struct{}{} defer func() { <-parseLimit }() @@ -136,21 +119,37 @@ func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.Pa if mode == source.ParseHeader { parserMode = parser.ImportsOnly | parser.ParseComments } - ast, err := parser.ParseFile(c.fset, fh.Identity().URI.Filename(), buf, parserMode) - if ast != nil { + file, parseError = parser.ParseFile(c.fset, fh.Identity().URI.Filename(), buf, parserMode) + if file != nil { if mode == source.ParseExported { - trimAST(ast) + trimAST(file) } // Fix any badly parsed parts of the AST. - tok := c.fset.File(ast.Pos()) - if err := fix(ctx, ast, tok, buf); err != nil { + tok := c.fset.File(file.Pos()) + if err := fix(ctx, file, tok, buf); err != nil { log.Error(ctx, "failed to fix AST", err) } } - if ast == nil { - return nil, err + // If the file is nil only due to parse errors, + // the parse errors are the actual errors. + if file == nil { + return nil, nil, parseError, parseError } - return ast, err + tok := c.FileSet().File(file.Pos()) + if tok == nil { + return nil, nil, parseError, err + } + uri := fh.Identity().URI + content, _, err := fh.Read(ctx) + if err != nil { + return nil, nil, parseError, err + } + m := &protocol.ColumnMapper{ + URI: uri, + Converter: span.NewTokenConverter(c.FileSet(), tok), + Content: content, + } + return file, m, parseError, nil } // 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 ba7fed3a6e..d763fe4691 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -151,11 +151,20 @@ func (pkg *pkg) Files() []source.ParseGoHandle { return pkg.files } +func (pkg *pkg) File(uri span.URI) (source.ParseGoHandle, error) { + for _, ph := range pkg.Files() { + if ph.File().Identity().URI == uri { + return ph, nil + } + } + return nil, errors.Errorf("no ParseGoHandle for %s", uri) +} + func (pkg *pkg) GetSyntax(ctx context.Context) []*ast.File { var syntax []*ast.File for _, ph := range pkg.files { - file, _, _ := ph.Cached(ctx) - if file != nil { + file, _, _, err := ph.Cached(ctx) + if err == nil { syntax = append(syntax, file) } } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index d42e184ae5..a7933077ea 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -23,6 +23,7 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error { defer done() ctx = telemetry.File.With(ctx, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return err @@ -45,8 +46,9 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error { if s.undelivered == nil { s.undelivered = make(map[span.URI][]source.Diagnostic) } - log.Error(ctx, "failed to deliver diagnostic (will retry)", err, telemetry.File) s.undelivered[uri] = diagnostics + + log.Error(ctx, "failed to deliver diagnostic (will retry)", err, telemetry.File) continue } // In case we had old, undelivered diagnostics. @@ -58,6 +60,7 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error { if err := s.publishDiagnostics(ctx, uri, diagnostics); err != nil { log.Error(ctx, "failed to deliver diagnostic for (will not retry)", err, telemetry.File) } + // If we fail to deliver the same diagnostics twice, just give up. delete(s.undelivered, uri) } diff --git a/internal/lsp/link.go b/internal/lsp/link.go index 9b248135f8..cea01ab955 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -28,8 +28,8 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink return nil, err } fh := f.Handle(ctx) - file, m, err := view.Session().Cache().ParseGoHandle(fh, source.ParseFull).Parse(ctx) - if file == nil { + file, m, _, err := view.Session().Cache().ParseGoHandle(fh, source.ParseFull).Parse(ctx) + if err != nil { return nil, err } var links []protocol.DocumentLink diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 17a8bc5a73..87f0a12401 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -376,14 +376,12 @@ func Completion(ctx context.Context, view View, f GoFile, pos protocol.Position, if err != nil { return nil, nil, err } - var ph ParseGoHandle - for _, h := range pkg.Files() { - if h.File().Identity().URI == f.URI() { - ph = h - } + ph, err := pkg.File(f.URI()) + if err != nil { + return nil, nil, err } - file, m, err := ph.Cached(ctx) - if file == nil { + file, m, _, err := ph.Cached(ctx) + if err != nil { return nil, nil, err } spn, err := m.PointSpan(pos) diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 561e5d897a..98a42130e9 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -123,14 +123,13 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { if !pos.IsValid() { return item, nil } - uri := span.FileURI(pos.Filename) ph, pkg, err := c.pkg.FindFile(c.ctx, uri) if err != nil { return CompletionItem{}, err } - file, _, err := ph.Cached(c.ctx) - if file == nil { + file, _, _, err := ph.Cached(c.ctx) + if err != nil { return CompletionItem{}, err } if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) { diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 009d892691..5e57c565af 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -8,7 +8,6 @@ import ( "bytes" "context" "fmt" - "go/ast" "strings" "golang.org/x/tools/go/analysis" @@ -181,22 +180,15 @@ func diagnostics(ctx context.Context, view View, pkg Package, reports map[span.U // spanToRange converts a span.Span to a protocol.Range, // assuming that the span belongs to the package whose diagnostics are being computed. func spanToRange(ctx context.Context, view View, pkg Package, spn span.Span, isTypeError bool) (protocol.Range, error) { - var ( - fh FileHandle - file *ast.File - m *protocol.ColumnMapper - err error - ) - for _, ph := range pkg.Files() { - if ph.File().Identity().URI == spn.URI() { - fh = ph.File() - file, m, err = ph.Cached(ctx) - } - } - if file == nil { + ph, err := pkg.File(spn.URI()) + if err != nil { return protocol.Range{}, err } - data, _, err := fh.Read(ctx) + _, m, _, err := ph.Cached(ctx) + if err != nil { + return protocol.Range{}, err + } + data, _, err := ph.File().Read(ctx) if err != nil { return protocol.Range{}, err } diff --git a/internal/lsp/source/folding_range.go b/internal/lsp/source/folding_range.go index f427979769..f66d958416 100644 --- a/internal/lsp/source/folding_range.go +++ b/internal/lsp/source/folding_range.go @@ -19,9 +19,8 @@ type FoldingRangeInfo struct { func FoldingRange(ctx context.Context, view View, f GoFile, lineFoldingOnly bool) (ranges []*FoldingRangeInfo, err error) { // TODO(suzmue): consider limiting the number of folding ranges returned, and // implement a way to prioritize folding ranges in that case. - fh := f.Handle(ctx) - ph := view.Session().Cache().ParseGoHandle(fh, ParseFull) - file, m, err := ph.Parse(ctx) + ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseFull) + file, m, _, err := ph.Parse(ctx) if err != nil { return nil, err } diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index b750b54082..cf8098c23f 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -37,17 +37,12 @@ func Format(ctx context.Context, view View, f File) ([]protocol.TextEdit, error) if err != nil { return nil, err } - var ph ParseGoHandle - for _, h := range pkg.Files() { - if h.File().Identity().URI == f.URI() { - ph = h - } - } - if ph == nil { + ph, err := pkg.File(f.URI()) + if err != nil { return nil, err } - file, m, err := ph.Parse(ctx) - if file == nil { + file, m, _, err := ph.Parse(ctx) + if err != nil { return nil, err } if hasListErrors(pkg.GetErrors()) || hasParseErrors(pkg, f.URI()) { @@ -102,13 +97,8 @@ func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]protoc if hasListErrors(pkg.GetErrors()) { return nil, errors.Errorf("%s has list errors, not running goimports", f.URI()) } - var ph ParseGoHandle - for _, h := range pkg.Files() { - if h.File().Identity().URI == f.URI() { - ph = h - } - } - if ph == nil { + ph, err := pkg.File(f.URI()) + if err != nil { return nil, err } options := &imports.Options{ @@ -133,8 +123,8 @@ func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]protoc if err != nil { return nil, err } - _, m, err := ph.Parse(ctx) - if m == nil { + _, m, _, err := ph.Parse(ctx) + if err != nil { return nil, err } return computeTextEdits(ctx, ph.File(), m, string(formatted)) @@ -201,8 +191,8 @@ func AllImportsFixes(ctx context.Context, view View, f File) (edits []protocol.T if err != nil { return err } - _, m, err := ph.Parse(ctx) - if m == nil { + _, m, _, err := ph.Parse(ctx) + if err != nil { return err } edits, err = computeTextEdits(ctx, ph.File(), m, string(formatted)) diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index 4193831ea7..992ada5e0b 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -23,10 +23,9 @@ func Highlight(ctx context.Context, view View, uri span.URI, pos protocol.Positi if err != nil { return nil, err } - fh := f.Handle(ctx) - ph := view.Session().Cache().ParseGoHandle(fh, ParseFull) - file, m, err := ph.Parse(ctx) - if file == nil { + ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseFull) + file, m, _, err := ph.Parse(ctx) + if err != nil { return nil, err } spn, err := m.PointSpan(pos) diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 9088e9ab23..ea8bed1302 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -57,15 +57,12 @@ func Identifier(ctx context.Context, view View, f GoFile, pos protocol.Position) if err != nil { return nil, err } - var ph ParseGoHandle - for _, h := range cph.Files() { - if h.File().Identity().URI == f.URI() { - ph = h - break - } + ph, err := pkg.File(f.URI()) + if err != nil { + return nil, err } - file, m, err := ph.Cached(ctx) - if file == nil { + file, m, _, err := ph.Cached(ctx) + if err != nil { return nil, err } spn, err := m.PointSpan(pos) @@ -246,7 +243,7 @@ func objToNode(ctx context.Context, view View, pkg Package, obj types.Object) (a if err != nil { return nil, err } - declAST, _, err := ph.Cached(ctx) + declAST, _, _, err := ph.Cached(ctx) if declAST == nil { return nil, err } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 0d5f3bb131..bb4cebccc4 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -176,16 +176,12 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) // getPkgName gets the pkg name associated with an identifer representing // the import path in an import spec. func (i *IdentifierInfo) getPkgName(ctx context.Context) (*IdentifierInfo, error) { - var ( - file *ast.File - err error - ) - for _, ph := range i.pkg.Files() { - if ph.File().Identity().URI == i.File.File().Identity().URI { - file, _, err = ph.Cached(ctx) - } + ph, err := i.pkg.File(i.URI()) + if err != nil { + return nil, err } - if file == nil { + file, _, _, err := ph.Cached(ctx) + if err != nil { return nil, err } var namePos token.Pos @@ -198,7 +194,6 @@ func (i *IdentifierInfo) getPkgName(ctx context.Context) (*IdentifierInfo, error if !namePos.IsValid() { return nil, errors.Errorf("import spec not found for %q", i.Name) } - // Look for the object defined at NamePos. for _, obj := range i.pkg.GetTypesInfo().Defs { pkgName, ok := obj.(*types.PkgName) diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index 4ede8cc7dd..b582fb6bc2 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -40,15 +40,12 @@ func SignatureHelp(ctx context.Context, view View, f GoFile, pos protocol.Positi if err != nil { return nil, err } - var ph ParseGoHandle - for _, h := range pkg.Files() { - if h.File().Identity().URI == f.URI() { - ph = h - break - } + ph, err := pkg.File(f.URI()) + if err != nil { + return nil, err } - file, m, err := ph.Cached(ctx) - if file == nil { + file, m, _, err := ph.Cached(ctx) + if err != nil { return nil, err } spn, err := m.PointSpan(pos) diff --git a/internal/lsp/source/suggested_fix.go b/internal/lsp/source/suggested_fix.go index d32d47de83..99eab5cb5c 100644 --- a/internal/lsp/source/suggested_fix.go +++ b/internal/lsp/source/suggested_fix.go @@ -18,8 +18,8 @@ func getCodeActions(ctx context.Context, view View, pkg Package, diag analysis.D if err != nil { return nil, err } - _, m, err := ph.Cached(ctx) - if m == nil { + _, m, _, err := ph.Cached(ctx) + if err != nil { return nil, err } mrng, err := posToRange(ctx, view, m, e.Pos, e.End) diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index 05bb6f4387..5d58b372e2 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -27,16 +27,12 @@ func DocumentSymbols(ctx context.Context, view View, f GoFile) ([]protocol.Docum if err != nil { return nil, err } - var ( - file *ast.File - m *protocol.ColumnMapper - ) - for _, ph := range pkg.Files() { - if ph.File().Identity().URI == f.URI() { - file, m, err = ph.Cached(ctx) - } + ph, err := pkg.File(f.URI()) + if err != nil { + return nil, err } - if file == nil { + file, m, _, err := ph.Cached(ctx) + if err != nil { return nil, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 5b563d67b3..1e575ccc1a 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -92,8 +92,8 @@ func IsGenerated(ctx context.Context, view View, uri span.URI) bool { return false } ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseHeader) - parsed, _, err := ph.Parse(ctx) - if parsed == nil { + parsed, _, _, err := ph.Parse(ctx) + if err != nil { return false } tok := view.Session().Cache().FileSet().File(parsed.Pos()) @@ -175,11 +175,8 @@ func posToMapper(ctx context.Context, view View, pkg Package, pos token.Pos) (*p if err != nil { return nil, err } - _, m, err := ph.Cached(ctx) - if m == nil { - return nil, err - } - return m, nil + _, m, _, err := ph.Cached(ctx) + return m, err } // Matches cgo generated comment as well as the proposed standard: diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 0fbdf483fe..2e89c1f32a 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -80,10 +80,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) + Parse(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) // Cached returns the AST for this handle, if it has already been stored. - Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error) + Cached(ctx context.Context) (*ast.File, *protocol.ColumnMapper, error, error) } // ParseMode controls the content of the AST produced when parsing a source file. @@ -288,6 +288,7 @@ type Package interface { ID() string PkgPath() string Files() []ParseGoHandle + File(uri span.URI) (ParseGoHandle, error) GetSyntax(context.Context) []*ast.File GetErrors() []packages.Error GetTypes() *types.Package