internal/lsp: fix view rebuilding when `go mod init` runs
https://github.com/microsoft/vscode-go/issues/3076#issuecomment-605062933
inspired me to write a regression test for this case. Turns out we
weren't handling it correctly after all...
This change makes sure that we only rebuild the view once a new go.mod
file is saved, not just created. It also preserves the snapshot ID
number when the view is recreated so that diagnostic caching continues
to work as expected.
Change-Id: I63bd559c3bd33b91828171cd7ddb3d099c31cddb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226017
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index bcf8dfd..3917c92 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -91,7 +91,7 @@
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, snapshot, err := s.createView(ctx, name, folder, options)
+ v, snapshot, err := s.createView(ctx, name, folder, options, 0)
if err != nil {
return nil, nil, err
}
@@ -101,7 +101,7 @@
return v, snapshot, nil
}
-func (s *Session) createView(ctx context.Context, name string, folder span.URI, options source.Options) (*view, *snapshot, error) {
+func (s *Session) createView(ctx context.Context, name string, folder span.URI, options source.Options, snapshotID uint64) (*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.
@@ -121,6 +121,7 @@
filesByURI: make(map[span.URI]*fileBase),
filesByBase: make(map[string][]*fileBase),
snapshot: &snapshot{
+ id: snapshotID,
packages: make(map[packageKey]*packageHandle),
ids: make(map[span.URI][]packageID),
metadata: make(map[packageID]*metadata),
@@ -255,7 +256,11 @@
if err != nil {
return nil, nil, err
}
- v, snapshot, err := s.createView(ctx, view.name, view.folder, options)
+ // Preserve the snapshot ID if we are recreating the view.
+ view.snapshotMu.Lock()
+ snapshotID := view.snapshot.id
+ view.snapshotMu.Unlock()
+ v, snapshot, err := s.createView(ctx, view.name, view.folder, options, snapshotID)
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
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index c0170e8..d0f1711 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -61,7 +61,7 @@
return nil
}
if err != nil {
- event.Error(ctx, "warning: diagnose go.mod", err, tag.Directory.Of(snapshot.View().Folder))
+ event.Error(ctx, "warning: diagnose go.mod", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
}
// Ensure that the reports returned from mod.Diagnostics are only related to the
// go.mod file for the module.
@@ -143,7 +143,6 @@
if ctx.Err() != nil {
break
}
-
// Pre-sort diagnostics to avoid extra work when we compare them.
source.SortDiagnostics(diagnostics)
toSend := sentDiagnostics{
@@ -181,7 +180,6 @@
// Do not update the delivered map since it already contains better diagnostics.
continue
}
-
if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
Diagnostics: toProtocolDiagnostics(diagnostics),
URI: protocol.URIFromSpanURI(key.id.URI),
diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go
index 3cd6305..2a4557b 100644
--- a/internal/lsp/fake/editor.go
+++ b/internal/lsp/fake/editor.go
@@ -118,7 +118,6 @@
// TODO: set client capabilities.
params.Trace = "messages"
// TODO: support workspace folders.
-
if e.server != nil {
resp, err := e.server.Initialize(ctx, params)
if err != nil {
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go
index d4ad606..a4c62ac 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -26,7 +26,6 @@
if realURI == "" || tempURI == "" {
return nil, nil, nil
}
-
ctx, done := event.StartSpan(ctx, "mod.Diagnostics", tag.URI.Of(realURI))
defer done()
@@ -42,7 +41,6 @@
if err != nil {
return nil, nil, err
}
-
reports := map[source.FileIdentity][]source.Diagnostic{
realfh.Identity(): {},
}
diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go
index 84a7512..7123db3 100644
--- a/internal/lsp/regtest/diagnostics_test.go
+++ b/internal/lsp/regtest/diagnostics_test.go
@@ -142,3 +142,44 @@
)
})
}
+
+const noMod = `
+-- main.go --
+package main
+
+import "mod.com/bob"
+
+func main() {
+ bob.Hello()
+}
+`
+
+// TestNoMod confirms that gopls continues to work when a user adds a go.mod
+// file to their workspace.
+func TestNoMod(t *testing.T) {
+ runner.Run(t, noMod, func(env *Env) {
+ env.Await(
+ env.DiagnosticAtRegexp("main.go", `"mod.com/bob"`),
+ )
+ env.CreateBuffer("bob/bob.go", `package bob
+
+func Hello() {
+ var x int
+}
+`)
+ env.Await(
+ env.DiagnosticAtRegexp("bob/bob.go", "x"),
+ )
+ // Save this file because otherwise, the go command will not be able to
+ // resolve mod.com/bob as a package.
+ env.SaveBuffer("bob/bob.go")
+ env.CreateBuffer("go.mod", `module mod.com
+
+go 1.12
+`)
+ env.SaveBuffer("go.mod")
+ env.Await(
+ EmptyDiagnostics("main.go"),
+ )
+ })
+}
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index 1ebe8be..6ef7a61 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -203,27 +203,29 @@
snapshotSet[snapshot] = append(snapshotSet[snapshot], uri)
}
for snapshot, uris := range snapshotSet {
- for _, uri := range uris {
- fh, err := snapshot.GetFile(uri)
- if err != nil {
- return nil, err
- }
- // If a modification comes in for the view's go.mod file and the view
- // was never properly initialized, or the view does not have
- // a go.mod file, try to recreate the associated view.
- switch fh.Identity().Kind {
- case source.Mod:
- modfile, _ := snapshot.View().ModFiles()
- if modfile != "" || fh.Identity().URI != modfile {
+ // If a modification comes in for the view's go.mod file and the view
+ // was never properly initialized, or the view does not have
+ // a go.mod file, try to recreate the associated view.
+ if modfile, _ := snapshot.View().ModFiles(); modfile == "" {
+ for _, uri := range uris {
+ // Don't rebuild the view until the go.mod is on disk.
+ if !snapshot.IsSaved(uri) {
continue
}
- newSnapshot, err := snapshot.View().Rebuild(ctx)
+ fh, err := snapshot.GetFile(uri)
if err != nil {
return nil, err
}
- // Update the snapshot to the rebuilt one.
- snapshot = newSnapshot
- snapshotByURI[uri] = snapshot
+ switch fh.Identity().Kind {
+ case source.Mod:
+ newSnapshot, err := snapshot.View().Rebuild(ctx)
+ if err != nil {
+ return nil, err
+ }
+ // Update the snapshot to the rebuilt one.
+ snapshot = newSnapshot
+ snapshotByURI[uri] = newSnapshot
+ }
}
}
go s.diagnoseSnapshot(snapshot)
