diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 062df68631..ccc24b6520 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -76,20 +76,20 @@ func (s *session) Cache() source.Cache { return s.cache } -func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.Options) (source.View, []source.PackageHandle, error) { +func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.Options) (source.View, source.Snapshot, error) { s.viewMu.Lock() defer s.viewMu.Unlock() - v, phs, err := s.createView(ctx, name, folder, options) + v, snapshot, err := s.createView(ctx, name, folder, options) if err != nil { return nil, nil, err } s.views = append(s.views, v) // we always need to drop the view map s.viewMap = make(map[span.URI]source.View) - return v, phs, nil + return v, snapshot, nil } -func (s *session) createView(ctx context.Context, name string, folder span.URI, options source.Options) (*view, []source.PackageHandle, error) { +func (s *session) createView(ctx context.Context, name string, folder span.URI, options source.Options) (*view, *snapshot, error) { index := atomic.AddInt64(&viewIndex, 1) // We want a true background context and not a detached context here // the spans need to be unrelated and no tag values should pollute it. @@ -131,8 +131,6 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI, // Preemptively load everything in this directory. // TODO(matloob): Determine if this can be done in parallel with something else. // Perhaps different calls to NewView can be run in parallel? - // TODO(matloob): By default when a new file is opened, its data is invalidated - // and it's loaded again. Determine if the redundant reload can be avoided. v.snapshotMu.Lock() defer v.snapshotMu.Unlock() // The code after the snapshot is used isn't expensive. m, err := v.snapshot.load(ctx, source.DirectoryURI(folder)) @@ -141,19 +139,17 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI, log.Error(ctx, "failed to load snapshot", err, telemetry.Directory.Of(folder)) return v, nil, nil } - // Prepare CheckPackageHandles for every package that's been loaded. // (*snapshot).CheckPackageHandle makes the assumption that every package that's // been loaded has an existing checkPackageHandle. - phs, err := v.snapshot.checkWorkspacePackages(ctx, m) - if err != nil { + if _, err := v.snapshot.checkWorkspacePackages(ctx, m); err != nil { // Suppress all errors. log.Error(ctx, "failed to check snapshot", err, telemetry.Directory.Of(folder)) return v, nil, nil } debug.AddView(debugView{v}) - return v, phs, nil + return v, v.snapshot, nil } // View returns the view by name. @@ -248,14 +244,14 @@ func (s *session) removeView(ctx context.Context, view *view) error { return nil } -func (s *session) updateView(ctx context.Context, view *view, options source.Options) (*view, []source.PackageHandle, error) { +func (s *session) updateView(ctx context.Context, view *view, options source.Options) (*view, *snapshot, error) { s.viewMu.Lock() defer s.viewMu.Unlock() i, err := s.dropView(ctx, view) if err != nil { return nil, nil, err } - v, phs, err := s.createView(ctx, view.name, view.folder, options) + v, snapshot, err := s.createView(ctx, view.name, view.folder, options) if err != nil { // we have dropped the old view, but could not create the new one // this should not happen and is very bad, but we still need to clean @@ -266,7 +262,7 @@ func (s *session) updateView(ctx context.Context, view *view, options source.Opt } // substitute the new view into the array where the old view was s.views[i] = v - return v, phs, nil + return v, snapshot, nil } func (s *session) dropView(ctx context.Context, view *view) (int, error) { diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 15cd0c3077..0ebfc70c45 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -250,6 +250,16 @@ func (s *snapshot) checkWorkspacePackages(ctx context.Context, m []*metadata) ([ return phs, nil } +func (s *snapshot) WorkspacePackageIDs(ctx context.Context) (ids []string) { + s.mu.Lock() + defer s.mu.Unlock() + + for id := range s.workspacePackages { + ids = append(ids, string(id)) + } + return ids +} + func (s *snapshot) KnownPackages(ctx context.Context) []source.Package { // TODO(matloob): This function exists because KnownImportPaths can't // determine the import paths of all packages. Remove this function diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 62a0511ee5..af7b59b165 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -16,12 +16,17 @@ import ( "golang.org/x/tools/internal/telemetry/trace" ) -func (s *Server) diagnoseSnapshot(snapshot source.Snapshot, phs []source.PackageHandle) { +func (s *Server) diagnoseSnapshot(snapshot source.Snapshot) { ctx := snapshot.View().BackgroundContext() ctx, done := trace.StartSpan(ctx, "lsp:background-worker") defer done() - for _, ph := range phs { + for _, id := range snapshot.WorkspacePackageIDs(ctx) { + ph, err := snapshot.PackageHandle(ctx, id) + if err != nil { + log.Error(ctx, "diagnoseSnapshot: no PackageHandle for workspace package", err, telemetry.Package.Of(id)) + continue + } if len(ph.CompiledGoFiles()) == 0 { continue } diff --git a/internal/lsp/general.go b/internal/lsp/general.go index c8c11aeddf..3da24d1c4b 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -165,12 +165,12 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol for _, folder := range folders { uri := span.NewURI(folder.URI) - view, workspacePackages, err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)) + _, snapshot, err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)) if err != nil { viewErrors[uri] = err continue } - go s.diagnoseSnapshot(view.Snapshot(), workspacePackages) + go s.diagnoseSnapshot(snapshot) } if len(viewErrors) > 0 { errMsg := fmt.Sprintf("Error loading workspace folders (expected %v, got %v)\n", len(folders), len(s.session.Views())-originalViews) diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 56e2419ae7..162a4a5ea2 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -40,6 +40,10 @@ type Snapshot interface { // that this file belongs to. PackageHandles(ctx context.Context, fh FileHandle) ([]PackageHandle, error) + // WorkspacePackageIDs returns the ids of the packages at the top-level + // of the snapshot's view. + WorkspacePackageIDs(ctx context.Context) []string + // GetActiveReverseDeps returns the active files belonging to the reverse // dependencies of this file's package. GetReverseDependencies(id string) []string @@ -128,6 +132,8 @@ type View interface { // belong to or be part of a dependency of the given package. FindPosInPackage(pkg Package, pos token.Pos) (*ast.File, Package, error) + // FindMapperInPackage returns the mapper associated with a file that may belong to + // the given package or one of its dependencies. FindMapperInPackage(pkg Package, uri span.URI) (*protocol.ColumnMapper, error) // Snapshot returns the current snapshot for the view. @@ -140,7 +146,7 @@ type View interface { // A session may have many active views at any given time. type Session interface { // NewView creates a new View and returns it. - NewView(ctx context.Context, name string, folder span.URI, options Options) (View, []PackageHandle, error) + NewView(ctx context.Context, name string, folder span.URI, options Options) (View, Snapshot, error) // Cache returns the cache that created this session. Cache() Cache diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go index 87e6bb9e4c..0b234991dd 100644 --- a/internal/lsp/workspace.go +++ b/internal/lsp/workspace.go @@ -26,7 +26,7 @@ func (s *Server) changeFolders(ctx context.Context, event protocol.WorkspaceFold return nil } -func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source.View, []source.PackageHandle, error) { +func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source.View, source.Snapshot, error) { s.stateMu.Lock() state := s.state s.stateMu.Unlock() @@ -35,8 +35,9 @@ func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source } options := s.session.Options() - s.fetchConfig(ctx, name, uri, &options) - + if err := s.fetchConfig(ctx, name, uri, &options); err != nil { + return nil, nil, err + } return s.session.NewView(ctx, name, uri, options) } @@ -44,8 +45,13 @@ func (s *Server) updateConfiguration(ctx context.Context, changed interface{}) e // go through all the views getting the config for _, view := range s.session.Views() { options := s.session.Options() - s.fetchConfig(ctx, view.Name(), view.Folder(), &options) - view.SetOptions(ctx, options) + if err := s.fetchConfig(ctx, view.Name(), view.Folder(), &options); err != nil { + return err + } + if _, err := view.SetOptions(ctx, options); err != nil { + return err + } + go s.diagnoseSnapshot(view.Snapshot()) } return nil }