Skip to content

Commit 19e0367

Browse files
committed
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 <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> Trust: Robert Findley <[email protected]>
1 parent 50ab967 commit 19e0367

File tree

6 files changed

+113
-11
lines changed

6 files changed

+113
-11
lines changed

gopls/internal/regtest/workspace_test.go

+70
Original file line numberDiff line numberDiff line change
@@ -358,3 +358,73 @@ func Hello() int {
358358
)
359359
})
360360
}
361+
362+
func TestUseGoplsMod(t *testing.T) {
363+
const multiModule = `
364+
-- moda/a/go.mod --
365+
module a.com
366+
367+
require b.com v1.2.3
368+
369+
-- moda/a/a.go --
370+
package a
371+
372+
import (
373+
"b.com/b"
374+
)
375+
376+
func main() {
377+
var x int
378+
_ = b.Hello()
379+
}
380+
-- modb/go.mod --
381+
module b.com
382+
383+
-- modb/b/b.go --
384+
package b
385+
386+
func Hello() int {
387+
var x int
388+
}
389+
-- gopls.mod --
390+
module gopls-workspace
391+
392+
require (
393+
a.com v0.0.0-goplsworkspace
394+
b.com v1.2.3
395+
)
396+
397+
replace a.com => $SANDBOX_WORKDIR/moda/a
398+
`
399+
withOptions(
400+
WithProxyFiles(workspaceModuleProxy),
401+
).run(t, multiModule, func(t *testing.T, env *Env) {
402+
env.Await(InitialWorkspaceLoad)
403+
env.OpenFile("moda/a/a.go")
404+
original, _ := env.GoToDefinition("moda/a/a.go", env.RegexpSearch("moda/a/a.go", "Hello"))
405+
if want := "[email protected]/b/b.go"; !strings.HasSuffix(original, want) {
406+
t.Errorf("expected %s, got %v", want, original)
407+
}
408+
workdir := env.Sandbox.Workdir.RootURI().SpanURI().Filename()
409+
env.WriteWorkspaceFile("gopls.mod", fmt.Sprintf(`module gopls-workspace
410+
411+
require (
412+
a.com v0.0.0-goplsworkspace
413+
b.com v0.0.0-goplsworkspace
414+
)
415+
416+
replace a.com => %s/moda/a
417+
replace b.com => %s/modb
418+
`, workdir, workdir))
419+
env.Await(
420+
OnceMet(
421+
CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1),
422+
env.DiagnosticAtRegexp("modb/b/b.go", "x"),
423+
),
424+
)
425+
newLocation, _ := env.GoToDefinition("moda/a/a.go", env.RegexpSearch("moda/a/a.go", "Hello"))
426+
if want := "modb/b/b.go"; !strings.HasSuffix(newLocation, want) {
427+
t.Errorf("expected %s, got %v", want, newLocation)
428+
}
429+
})
430+
}

internal/lsp/cache/load.go

+7
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
errors "golang.org/x/xerrors"
2424
)
2525

26+
// metadata holds package metadata extracted from a call to packages.Load.
2627
type metadata struct {
2728
id packageID
2829
pkgPath packagePath
@@ -40,6 +41,8 @@ type metadata struct {
4041
config *packages.Config
4142
}
4243

44+
// load calls packages.Load for the given scopes, updating package metadata,
45+
// import graph, and mapped files with the result.
4346
func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
4447
var query []string
4548
var containsDir bool // for logging
@@ -234,6 +237,9 @@ func (s *snapshot) tempWorkspaceModule(ctx context.Context) (_ span.URI, cleanup
234237
return span.URIFromPath(filepath.Dir(filename)), cleanup, nil
235238
}
236239

240+
// setMetadata extracts metadata from pkg and records it in s. It
241+
// recurses through pkg.Imports to ensure that metadata exists for all
242+
// dependencies.
237243
func (s *snapshot) setMetadata(ctx context.Context, pkgPath packagePath, pkg *packages.Package, cfg *packages.Config, seen map[packageID]struct{}) (*metadata, error) {
238244
id := packageID(pkg.ID)
239245
if _, ok := seen[id]; ok {
@@ -262,6 +268,7 @@ func (s *snapshot) setMetadata(ctx context.Context, pkgPath packagePath, pkg *pa
262268
s.addID(uri, m.id)
263269
}
264270

271+
// TODO(rstambler): is this still necessary?
265272
copied := map[packageID]struct{}{
266273
id: {},
267274
}

internal/lsp/cache/snapshot.go

+32-3
Original file line numberDiff line numberDiff line change
@@ -733,8 +733,11 @@ func (s *snapshot) FindFile(uri span.URI) source.VersionedFileHandle {
733733
return s.files[f.URI()]
734734
}
735735

736-
// GetFile returns a File for the given URI. It will always succeed because it
737-
// adds the file to the managed set if needed.
736+
// GetFile returns a File for the given URI. If the file is unknown it is added
737+
// to the managed set.
738+
//
739+
// GetFile succeeds even if the file does not exist. A non-nil error return
740+
// indicates some type of internal error, for example if ctx is cancelled.
738741
func (s *snapshot) GetFile(ctx context.Context, uri span.URI) (source.VersionedFileHandle, error) {
739742
f, err := s.view.getFile(uri)
740743
if err != nil {
@@ -1426,6 +1429,25 @@ func (s *snapshot) getWorkspaceModuleHandle(ctx context.Context) (*workspaceModu
14261429
}
14271430
fhs = append(fhs, fh)
14281431
}
1432+
goplsModURI := span.URIFromPath(filepath.Join(s.view.Folder().Filename(), "gopls.mod"))
1433+
goplsModFH, err := s.GetFile(ctx, goplsModURI)
1434+
if err != nil {
1435+
return nil, err
1436+
}
1437+
_, err = goplsModFH.Read()
1438+
switch {
1439+
case err == nil:
1440+
// We have a gopls.mod. Our handle only depends on it.
1441+
fhs = []source.FileHandle{goplsModFH}
1442+
case os.IsNotExist(err):
1443+
// No gopls.mod, so we must build the workspace mod file automatically.
1444+
// Defensively ensure that the goplsModFH is nil as this controls automatic
1445+
// building of the workspace mod file.
1446+
goplsModFH = nil
1447+
default:
1448+
return nil, errors.Errorf("error getting gopls.mod: %w", err)
1449+
}
1450+
14291451
sort.Slice(fhs, func(i, j int) bool {
14301452
return fhs[i].URI() < fhs[j].URI()
14311453
})
@@ -1435,6 +1457,13 @@ func (s *snapshot) getWorkspaceModuleHandle(ctx context.Context) (*workspaceModu
14351457
}
14361458
key := workspaceModuleKey(hashContents([]byte(k)))
14371459
h := s.generation.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
1460+
if goplsModFH != nil {
1461+
parsed, err := s.ParseMod(ctx, goplsModFH)
1462+
if err != nil {
1463+
return &workspaceModuleData{err: err}
1464+
}
1465+
return &workspaceModuleData{file: parsed.File}
1466+
}
14381467
s := arg.(*snapshot)
14391468
data := &workspaceModuleData{}
14401469
data.file, data.err = s.BuildWorkspaceModFile(ctx)
@@ -1450,7 +1479,7 @@ func (s *snapshot) getWorkspaceModuleHandle(ctx context.Context) (*workspaceModu
14501479
}
14511480

14521481
// BuildWorkspaceModFile generates a workspace module given the modules in the
1453-
// the workspace.
1482+
// the workspace. It does not read gopls.mod.
14541483
func (s *snapshot) BuildWorkspaceModFile(ctx context.Context) (*modfile.File, error) {
14551484
file := &modfile.File{}
14561485
file.AddModuleStmt("gopls-workspace")

internal/lsp/cache/view.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,8 @@ func minorOptionsChange(a, b *source.Options) bool {
314314
copy(bBuildFlags, b.BuildFlags)
315315
sort.Strings(aBuildFlags)
316316
sort.Strings(bBuildFlags)
317-
if !reflect.DeepEqual(aBuildFlags, bBuildFlags) {
318-
return false
319-
}
320317
// the rest of the options are benign
321-
return true
318+
return reflect.DeepEqual(aBuildFlags, bBuildFlags)
322319
}
323320

324321
func (v *View) SetOptions(ctx context.Context, options *source.Options) (source.View, error) {

internal/lsp/source/view.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -407,9 +407,8 @@ type FileHandle interface {
407407
URI() span.URI
408408
Kind() FileKind
409409

410-
// Identity returns a FileIdentity for the file, even if there was an error
411-
// reading it.
412-
// It is a fatal error to call Identity on a file that has not yet been read.
410+
// FileIdentity returns a FileIdentity for the file, even if there was an
411+
// error reading it.
413412
FileIdentity() FileIdentity
414413
// Read reads the contents of a file.
415414
// If the file is not available, returns a nil slice and an error.

internal/lsp/text_synchronization.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ func (s *Server) didModifyFiles(ctx context.Context, modifications []source.File
215215
for _, s := range snapshots {
216216
if s.View() == view {
217217
if snapshot != nil {
218-
return errors.Errorf("duplicate snapshots for the same view")
218+
return errors.New("duplicate snapshots for the same view")
219219
}
220220
snapshot = s
221221
}

0 commit comments

Comments
 (0)