diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go index 8629917df6..72c3ec1c61 100644 --- a/gopls/internal/regtest/workspace_test.go +++ b/gopls/internal/regtest/workspace_test.go @@ -358,3 +358,73 @@ func Hello() int { ) }) } + +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 86ba0f04b9..c80cd4cb86 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -23,6 +23,7 @@ import ( 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 @@ type metadata struct { 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 @@ func (s *snapshot) tempWorkspaceModule(ctx context.Context) (_ span.URI, cleanup 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 @@ func (s *snapshot) setMetadata(ctx context.Context, pkgPath packagePath, pkg *pa 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 368be08e59..5cc18bc2a2 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -733,8 +733,11 @@ func (s *snapshot) FindFile(uri span.URI) source.VersionedFileHandle { 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 @@ func (s *snapshot) getWorkspaceModuleHandle(ctx context.Context) (*workspaceModu } 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 @@ func (s *snapshot) getWorkspaceModuleHandle(ctx context.Context) (*workspaceModu } 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 @@ func (s *snapshot) getWorkspaceModuleHandle(ctx context.Context) (*workspaceModu } // 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 a8f3b07d7b..abf6f4c012 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -314,11 +314,8 @@ func minorOptionsChange(a, b *source.Options) bool { 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 c4731fc3ef..f098921b99 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -407,9 +407,8 @@ type FileHandle interface { 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 2b9a77ada2..4606e258cb 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -215,7 +215,7 @@ func (s *Server) didModifyFiles(ctx context.Context, modifications []source.File 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 }