internal/lsp/cache: use gopls.mod for the workspace module if it exists
When building the workspace module, prefer a gopls.mod file located at
the root of the view if it exists.
Also do some minor documenting/cleanup along the way.
For golang/go#32394
Change-Id: If87729a766d37e6c834fefe40cfb47b67a36a60c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256582
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go
index 8629917..72c3ec1 100644
--- a/gopls/internal/regtest/workspace_test.go
+++ b/gopls/internal/regtest/workspace_test.go
@@ -358,3 +358,73 @@
)
})
}
+
+func TestUseGoplsMod(t *testing.T) {
+ const multiModule = `
+-- moda/a/go.mod --
+module a.com
+
+require b.com v1.2.3
+
+-- moda/a/a.go --
+package a
+
+import (
+ "b.com/b"
+)
+
+func main() {
+ var x int
+ _ = b.Hello()
+}
+-- modb/go.mod --
+module b.com
+
+-- modb/b/b.go --
+package b
+
+func Hello() int {
+ var x int
+}
+-- gopls.mod --
+module gopls-workspace
+
+require (
+ a.com v0.0.0-goplsworkspace
+ b.com v1.2.3
+)
+
+replace a.com => $SANDBOX_WORKDIR/moda/a
+`
+ withOptions(
+ WithProxyFiles(workspaceModuleProxy),
+ ).run(t, multiModule, func(t *testing.T, env *Env) {
+ env.Await(InitialWorkspaceLoad)
+ env.OpenFile("moda/a/a.go")
+ original, _ := env.GoToDefinition("moda/a/a.go", env.RegexpSearch("moda/a/a.go", "Hello"))
+ if want := "b.com@v1.2.3/b/b.go"; !strings.HasSuffix(original, want) {
+ t.Errorf("expected %s, got %v", want, original)
+ }
+ workdir := env.Sandbox.Workdir.RootURI().SpanURI().Filename()
+ env.WriteWorkspaceFile("gopls.mod", fmt.Sprintf(`module gopls-workspace
+
+require (
+ a.com v0.0.0-goplsworkspace
+ b.com v0.0.0-goplsworkspace
+)
+
+replace a.com => %s/moda/a
+replace b.com => %s/modb
+`, workdir, workdir))
+ env.Await(
+ OnceMet(
+ CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1),
+ env.DiagnosticAtRegexp("modb/b/b.go", "x"),
+ ),
+ )
+ newLocation, _ := env.GoToDefinition("moda/a/a.go", env.RegexpSearch("moda/a/a.go", "Hello"))
+ if want := "modb/b/b.go"; !strings.HasSuffix(newLocation, want) {
+ t.Errorf("expected %s, got %v", want, newLocation)
+ }
+ })
+}
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 86ba0f0..c80cd4c 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -23,6 +23,7 @@
errors "golang.org/x/xerrors"
)
+// metadata holds package metadata extracted from a call to packages.Load.
type metadata struct {
id packageID
pkgPath packagePath
@@ -40,6 +41,8 @@
config *packages.Config
}
+// load calls packages.Load for the given scopes, updating package metadata,
+// import graph, and mapped files with the result.
func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
var query []string
var containsDir bool // for logging
@@ -234,6 +237,9 @@
return span.URIFromPath(filepath.Dir(filename)), cleanup, nil
}
+// setMetadata extracts metadata from pkg and records it in s. It
+// recurses through pkg.Imports to ensure that metadata exists for all
+// dependencies.
func (s *snapshot) setMetadata(ctx context.Context, pkgPath packagePath, pkg *packages.Package, cfg *packages.Config, seen map[packageID]struct{}) (*metadata, error) {
id := packageID(pkg.ID)
if _, ok := seen[id]; ok {
@@ -262,6 +268,7 @@
s.addID(uri, m.id)
}
+ // TODO(rstambler): is this still necessary?
copied := map[packageID]struct{}{
id: {},
}
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 368be08..5cc18bc 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -733,8 +733,11 @@
return s.files[f.URI()]
}
-// GetFile returns a File for the given URI. It will always succeed because it
-// adds the file to the managed set if needed.
+// GetFile returns a File for the given URI. If the file is unknown it is added
+// to the managed set.
+//
+// GetFile succeeds even if the file does not exist. A non-nil error return
+// indicates some type of internal error, for example if ctx is cancelled.
func (s *snapshot) GetFile(ctx context.Context, uri span.URI) (source.VersionedFileHandle, error) {
f, err := s.view.getFile(uri)
if err != nil {
@@ -1426,6 +1429,25 @@
}
fhs = append(fhs, fh)
}
+ goplsModURI := span.URIFromPath(filepath.Join(s.view.Folder().Filename(), "gopls.mod"))
+ goplsModFH, err := s.GetFile(ctx, goplsModURI)
+ if err != nil {
+ return nil, err
+ }
+ _, err = goplsModFH.Read()
+ switch {
+ case err == nil:
+ // We have a gopls.mod. Our handle only depends on it.
+ fhs = []source.FileHandle{goplsModFH}
+ case os.IsNotExist(err):
+ // No gopls.mod, so we must build the workspace mod file automatically.
+ // Defensively ensure that the goplsModFH is nil as this controls automatic
+ // building of the workspace mod file.
+ goplsModFH = nil
+ default:
+ return nil, errors.Errorf("error getting gopls.mod: %w", err)
+ }
+
sort.Slice(fhs, func(i, j int) bool {
return fhs[i].URI() < fhs[j].URI()
})
@@ -1435,6 +1457,13 @@
}
key := workspaceModuleKey(hashContents([]byte(k)))
h := s.generation.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
+ if goplsModFH != nil {
+ parsed, err := s.ParseMod(ctx, goplsModFH)
+ if err != nil {
+ return &workspaceModuleData{err: err}
+ }
+ return &workspaceModuleData{file: parsed.File}
+ }
s := arg.(*snapshot)
data := &workspaceModuleData{}
data.file, data.err = s.BuildWorkspaceModFile(ctx)
@@ -1450,7 +1479,7 @@
}
// BuildWorkspaceModFile generates a workspace module given the modules in the
-// the workspace.
+// the workspace. It does not read gopls.mod.
func (s *snapshot) BuildWorkspaceModFile(ctx context.Context) (*modfile.File, error) {
file := &modfile.File{}
file.AddModuleStmt("gopls-workspace")
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index a8f3b07..abf6f4c 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -314,11 +314,8 @@
copy(bBuildFlags, b.BuildFlags)
sort.Strings(aBuildFlags)
sort.Strings(bBuildFlags)
- if !reflect.DeepEqual(aBuildFlags, bBuildFlags) {
- return false
- }
// the rest of the options are benign
- return true
+ return reflect.DeepEqual(aBuildFlags, bBuildFlags)
}
func (v *View) SetOptions(ctx context.Context, options *source.Options) (source.View, error) {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index c4731fc..f098921 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -407,9 +407,8 @@
URI() span.URI
Kind() FileKind
- // Identity returns a FileIdentity for the file, even if there was an error
- // reading it.
- // It is a fatal error to call Identity on a file that has not yet been read.
+ // FileIdentity returns a FileIdentity for the file, even if there was an
+ // error reading it.
FileIdentity() FileIdentity
// Read reads the contents of a file.
// If the file is not available, returns a nil slice and an error.
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index 2b9a77a..4606e25 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -215,7 +215,7 @@
for _, s := range snapshots {
if s.View() == view {
if snapshot != nil {
- return errors.Errorf("duplicate snapshots for the same view")
+ return errors.New("duplicate snapshots for the same view")
}
snapshot = s
}
