internal/lsp: remove all but one use of the view's modURI field
The view reinitialization logic appears to be broken, and so needs one
remaining use of the modURI, which be fixed in a follow-up. Every other
use of view.modURI is removed.
Updates golang/go#32394
Change-Id: Ic051ed848c30e6981d42a576fb35f40efbeb17a6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257417
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go
index 43d4ec6..704f598 100644
--- a/gopls/internal/regtest/workspace_test.go
+++ b/gopls/internal/regtest/workspace_test.go
@@ -338,7 +338,9 @@
var x int
}
`
- run(t, multiModule, func(t *testing.T, env *Env) {
+ withOptions(
+ WithProxyFiles(workspaceModuleProxy),
+ ).run(t, multiModule, func(t *testing.T, env *Env) {
env.OpenFile("modb/go.mod")
env.Await(
OnceMet(
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index e9aa1dd..7de3bc8 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -227,14 +227,7 @@
modules: modules,
}
- // TODO(rstambler): Change this function to work without a snapshot.
- // Set the first snapshot's workspace directories. The view's modURI was
- // set by setBuildInformation.
- var fh source.FileHandle
- if v.modURI != "" {
- fh, _ = s.GetFile(ctx, v.modURI)
- }
- v.snapshot.workspaceDirectories = v.snapshot.findWorkspaceDirectories(ctx, fh)
+ v.snapshot.workspaceDirectories = v.snapshot.findWorkspaceDirectories(ctx)
// Initialize the view without blocking.
initCtx, initCancel := context.WithCancel(xcontext.Detach(ctx))
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 01e7856..1021133 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -809,8 +809,8 @@
// reloadWorkspace reloads the metadata for all invalidated workspace packages.
func (s *snapshot) reloadWorkspace(ctx context.Context) error {
- // If the view's build configuration is invalid, we cannot reload by package path.
- // Just reload the directory instead.
+ // If the view's build configuration is invalid, we cannot reload by
+ // package path. Just reload the directory instead.
if !s.view.hasValidBuildConfiguration {
return s.load(ctx, viewLoadScope("LOAD_INVALID_VIEW"))
}
@@ -1001,6 +1001,8 @@
result.modules[k] = v
}
+ var modulesChanged, shouldReinitializeView bool
+
// transitiveIDs keeps track of transitive reverse dependencies.
// If an ID is present in the map, invalidate its types.
// If an ID's value is true, invalidate its metadata too.
@@ -1044,6 +1046,8 @@
currentMod := currentExists && currentFH.Kind() == source.Mod
originalMod := originalFH != nil && originalFH.Kind() == source.Mod
if currentMod || originalMod {
+ modulesChanged = true
+
// If the view's go.mod file's contents have changed, invalidate
// the metadata for every known package in the snapshot.
if invalidateMetadata {
@@ -1061,16 +1065,15 @@
rootURI := span.URIFromPath(filepath.Dir(withoutURI.Filename()))
if currentMod {
if _, ok := result.modules[rootURI]; !ok {
- m := newModule(ctx, currentFH.URI())
- result.modules[m.rootURI] = m
- result.view.definitelyReinitialize()
+ result.modules[rootURI] = newModule(ctx, currentFH.URI())
+ shouldReinitializeView = true
}
} else if originalMod {
// Similarly, we need to retry the IWL if a go.mod in the workspace
// was deleted.
if _, ok := result.modules[rootURI]; ok {
delete(result.modules, rootURI)
- result.view.definitelyReinitialize()
+ shouldReinitializeView = true
}
}
}
@@ -1091,12 +1094,6 @@
}
}
}
- if withoutURI == s.view.modURI {
- // The go.mod's replace directives may have changed. We may
- // need to update our set of workspace directories. Use the new
- // snapshot, as it can be locked without causing issues.
- result.workspaceDirectories = result.findWorkspaceDirectories(ctx, currentFH)
- }
// If this is a file we don't yet know about,
// then we do not yet know what packages it should belong to.
@@ -1147,6 +1144,14 @@
// Make sure to remove the changed file from the unloadable set.
delete(result.unloadableFiles, withoutURI)
}
+
+ // When modules change, we need to recompute their workspace directories,
+ // as replace directives may have changed.
+ if modulesChanged {
+ // Use the new snapshot, as it can be locked without causing issues.
+ result.workspaceDirectories = result.findWorkspaceDirectories(ctx)
+ }
+
// Copy the package type information.
for k, v := range s.packages {
if _, ok := transitiveIDs[k.id]; ok {
@@ -1208,6 +1213,10 @@
result.workspacePackages[id] = pkgPath
}
+ if shouldReinitializeView && s.view.hasValidBuildConfiguration {
+ s.view.definitelyReinitialize()
+ }
+
// Inherit all of the go.mod-related handles.
for _, v := range result.modTidyHandles {
newGen.Inherit(v.handle)
@@ -1309,35 +1318,35 @@
//
// It assumes that the file handle is the view's go.mod file, if it has one.
// The caller need not be holding the snapshot's mutex, but it might be.
-func (s *snapshot) findWorkspaceDirectories(ctx context.Context, modFH source.FileHandle) map[span.URI]struct{} {
- m := map[span.URI]struct{}{
- s.view.rootURI: {},
- }
+func (s *snapshot) findWorkspaceDirectories(ctx context.Context) map[span.URI]struct{} {
// If the view does not have a go.mod file, only the root directory
// is known. In GOPATH mode, we should really watch the entire GOPATH,
// but that's too expensive.
- modURI := s.view.modURI
- if modURI == "" {
- return m
+ dirs := map[span.URI]struct{}{
+ s.view.rootURI: {},
}
- if modFH == nil {
- return m
- }
- // Ignore parse errors. An invalid go.mod is not fatal.
- mod, err := s.ParseMod(ctx, modFH)
- if err != nil {
- return m
- }
- for _, r := range mod.File.Replace {
- // We may be replacing a module with a different version, not a path
- // on disk.
- if r.New.Version != "" {
+ for _, m := range s.modules {
+ fh, err := s.GetFile(ctx, m.modURI)
+ if err != nil {
continue
}
- uri := span.URIFromPath(r.New.Path)
- m[uri] = struct{}{}
+ // Ignore parse errors. An invalid go.mod is not fatal.
+ // TODO(rstambler): Try to preserve existing watched directories as
+ // much as possible, otherwise we will thrash when a go.mod is edited.
+ mod, err := s.ParseMod(ctx, fh)
+ if err != nil {
+ continue
+ }
+ for _, r := range mod.File.Replace {
+ // We may be replacing a module with a different version, not a path
+ // on disk.
+ if r.New.Version != "" {
+ continue
+ }
+ dirs[span.URIFromPath(r.New.Path)] = struct{}{}
+ }
}
- return m
+ return dirs
}
func (s *snapshot) BuiltinPackage(ctx context.Context) (*source.BuiltinPackage, error) {
