internal/lsp/cache: don't delete metadata until it's reloaded
This CL moves to a model where we don't automatically delete invalidated
metadata, but rather preserve it and mark it invalid. This way, we can
continue to use invalid metadata for all features even if there is an
issue with the user's workspace.
To keep track of the metadata's validity, we add an invalid flag to
track the status of the metadata. We still reload at the same rate--the
next CL changes the way we reload data.
We also add a configuration to opt-in (currently, this is off by
default).
In some cases, like switches between GOPATH and module modes, and when a
file is deleted, the metadata *must* be deleted outright.
Updates golang/go#42266
Change-Id: Iff5e10b641fdb4be270af0cd887a10ee97ac1a19
Reviewed-on: https://go-review.googlesource.com/c/tools/+/271477
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md
index f2de5b1..c04ab05 100644
--- a/gopls/doc/settings.md
+++ b/gopls/doc/settings.md
@@ -154,6 +154,17 @@
Default: `false`.
+#### **experimentalUseInvalidMetadata** *bool*
+
+**This setting is experimental and may be deleted.**
+
+experimentalUseInvalidMetadata enables gopls to fall back on outdated
+package metadata to provide editor features if the go command fails to
+load packages for some reason (like an invalid go.mod file). This will
+eventually be the default behavior, and this setting will be removed.
+
+Default: `false`.
+
### Formatting
#### **local** *string*
diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go
index afdd494..88e7e54 100644
--- a/gopls/internal/regtest/completion/completion_test.go
+++ b/gopls/internal/regtest/completion/completion_test.go
@@ -322,7 +322,6 @@
env.AcceptCompletion("main.go", pos, item)
// Await the diagnostics to add example.com/blah to the go.mod file.
- env.SaveBufferWithoutActions("main.go")
env.Await(
env.DiagnosticAtRegexp("main.go", `"example.com/blah"`),
)
diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go
index 019ba65..75cb6bd 100644
--- a/gopls/internal/regtest/diagnostics/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go
@@ -7,7 +7,6 @@
import (
"context"
"fmt"
- "log"
"os/exec"
"testing"
@@ -147,12 +146,14 @@
env.Await(
env.DiagnosticAtRegexp("a.go", "a = 1"),
env.DiagnosticAtRegexp("b.go", "a = 2"),
- env.DiagnosticAtRegexp("c.go", "a = 3"))
+ env.DiagnosticAtRegexp("c.go", "a = 3"),
+ )
env.CloseBuffer("c.go")
env.Await(
env.DiagnosticAtRegexp("a.go", "a = 1"),
env.DiagnosticAtRegexp("b.go", "a = 2"),
- EmptyDiagnostics("c.go"))
+ EmptyDiagnostics("c.go"),
+ )
})
}
@@ -225,7 +226,6 @@
// Tests golang/go#38878: deleting a test file on disk while it's still open
// should not clear its errors.
func TestDeleteTestVariant_DiskOnly(t *testing.T) {
- log.SetFlags(log.Lshortfile)
Run(t, test38878, func(t *testing.T, env *Env) {
env.OpenFile("a_test.go")
env.Await(DiagnosticAt("a_test.go", 5, 3))
@@ -438,7 +438,7 @@
func TestMissingDependency(t *testing.T) {
Run(t, testPackageWithRequire, func(t *testing.T, env *Env) {
env.OpenFile("print.go")
- env.Await(LogMatching(protocol.Error, "initial workspace load failed", 1))
+ env.Await(LogMatching(protocol.Error, "workspace load failed", 1))
})
}
@@ -1294,7 +1294,6 @@
func main() {}
`
Run(t, dir, func(t *testing.T, env *Env) {
- log.SetFlags(log.Lshortfile)
env.OpenFile("main.go")
env.OpenFile("other.go")
x := env.DiagnosticsFor("main.go")
@@ -1936,3 +1935,71 @@
)
})
}
+
+func TestUseOfInvalidMetadata(t *testing.T) {
+ testenv.NeedsGo1Point(t, 13)
+
+ const mod = `
+-- go.mod --
+module mod.com
+
+go 1.12
+-- main.go --
+package main
+
+import (
+ "mod.com/a"
+ //"os"
+)
+
+func _() {
+ a.Hello()
+ os.Getenv("")
+ //var x int
+}
+-- a/a.go --
+package a
+
+func Hello() {}
+`
+ WithOptions(
+ EditorConfig{
+ ExperimentalUseInvalidMetadata: true,
+ },
+ Modes(Singleton),
+ ).Run(t, mod, func(t *testing.T, env *Env) {
+ env.OpenFile("go.mod")
+ env.RegexpReplace("go.mod", "module mod.com", "modul mod.com") // break the go.mod file
+ env.Editor.SaveBuffer(env.Ctx, "go.mod")
+ env.Await(
+ env.DiagnosticAtRegexp("go.mod", "modul"),
+ )
+ // Confirm that language features work with invalid metadata.
+ env.OpenFile("main.go")
+ file, pos := env.GoToDefinition("main.go", env.RegexpSearch("main.go", "Hello"))
+ wantPos := env.RegexpSearch("a/a.go", "Hello")
+ if file != "a/a.go" && pos != wantPos {
+ t.Fatalf("expected a/a.go:%s, got %s:%s", wantPos, file, pos)
+ }
+ // Confirm that new diagnostics appear with invalid metadata by adding
+ // an unused variable to the body of the function.
+ env.RegexpReplace("main.go", "//var x int", "var x int")
+ env.Await(
+ env.DiagnosticAtRegexp("main.go", "x"),
+ )
+ // Add an import and confirm that we get a diagnostic for it, since the
+ // metadata will not have been updated.
+ env.RegexpReplace("main.go", "//\"os\"", "\"os\"")
+ env.Await(
+ env.DiagnosticAtRegexp("main.go", `"os"`),
+ )
+ // Fix the go.mod file and expect the diagnostic to resolve itself.
+ env.RegexpReplace("go.mod", "modul mod.com", "module mod.com")
+ env.SaveBuffer("go.mod")
+ env.Await(
+ env.DiagnosticAtRegexp("main.go", "x"),
+ env.NoDiagnosticAtRegexp("main.go", `"os"`),
+ EmptyDiagnostics("go.mod"),
+ )
+ })
+}
diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go
index 6f0e692..956e59a 100644
--- a/gopls/internal/regtest/workspace/workspace_test.go
+++ b/gopls/internal/regtest/workspace/workspace_test.go
@@ -339,12 +339,15 @@
Modes(Experimental),
).Run(t, multiModule, func(t *testing.T, env *Env) {
env.OpenFile("moda/a/a.go")
+ env.Await(env.DoneWithOpen())
original, _ := env.GoToDefinition("moda/a/a.go", env.RegexpSearch("moda/a/a.go", "Hello"))
if want := "modb/b/b.go"; !strings.HasSuffix(original, want) {
t.Errorf("expected %s, got %v", want, original)
}
env.CloseBuffer(original)
+ env.Await(env.DoneWithClose())
+
env.RemoveWorkspaceFile("modb/b/b.go")
env.RemoveWorkspaceFile("modb/go.mod")
env.Await(
@@ -359,6 +362,8 @@
),
)
env.ApplyQuickFixes("moda/a/go.mod", d.Diagnostics)
+ env.Await(env.DoneWithChangeWatchedFiles())
+
got, _ := 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(got, want) {
t.Errorf("expected %s, got %v", want, got)
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 0cf4c93..13f9d1b 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -41,7 +41,7 @@
mode source.ParseMode
// m is the metadata associated with the package.
- m *metadata
+ m *knownMetadata
// key is the hashed key for the package.
key packageHandleKey
@@ -117,7 +117,7 @@
}
data := &packageData{}
- data.pkg, data.err = typeCheck(ctx, snapshot, m, mode, deps)
+ data.pkg, data.err = typeCheck(ctx, snapshot, m.metadata, mode, deps)
// Make sure that the workers above have finished before we return,
// especially in case of cancellation.
wg.Wait()
@@ -167,7 +167,10 @@
var depKeys []packageHandleKey
for _, depID := range depList {
depHandle, err := s.buildPackageHandle(ctx, depID, s.workspaceParseMode(depID))
- if err != nil {
+ // Don't use invalid metadata for dependencies if the top-level
+ // metadata is valid. We only load top-level packages, so if the
+ // top-level is valid, all of its dependencies should be as well.
+ if err != nil || m.valid && !depHandle.m.valid {
event.Error(ctx, fmt.Sprintf("%s: no dep handle for %s", id, depID), err, tag.Snapshot.Of(s.id))
if ctx.Err() != nil {
return nil, nil, ctx.Err()
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index ae161ca..520c5e6 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -420,7 +420,7 @@
m.missingDeps[importPkgPath] = struct{}{}
continue
}
- if s.getMetadata(importID) == nil {
+ if s.noValidMetadataForID(importID) {
if _, err := s.setMetadata(ctx, importPkgPath, importPkg, cfg, copied); err != nil {
event.Error(ctx, "error in dependency", err)
}
@@ -434,10 +434,13 @@
// TODO: We should make sure not to set duplicate metadata,
// and instead panic here. This can be done by making sure not to
// reset metadata information for packages we've already seen.
- if original, ok := s.metadata[m.id]; ok {
- m = original
+ if original, ok := s.metadata[m.id]; ok && original.valid {
+ m = original.metadata
} else {
- s.metadata[m.id] = m
+ s.metadata[m.id] = &knownMetadata{
+ metadata: m,
+ valid: true,
+ }
}
// Set the workspace packages. If any of the package's files belong to the
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 657a0ee..21b983f 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -221,7 +221,7 @@
generation: s.cache.store.Generation(generationName(v, 0)),
packages: make(map[packageKey]*packageHandle),
ids: make(map[span.URI][]packageID),
- metadata: make(map[packageID]*metadata),
+ metadata: make(map[packageID]*knownMetadata),
files: make(map[span.URI]source.VersionedFileHandle),
goFiles: make(map[parseKey]*parseGoHandle),
importedBy: make(map[packageID][]packageID),
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 3716996..94dca42 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -73,7 +73,7 @@
// metadata maps file IDs to their associated metadata.
// It may invalidated on calls to go/packages.
- metadata map[packageID]*metadata
+ metadata map[packageID]*knownMetadata
// importedBy maps package IDs to the list of packages that import them.
importedBy map[packageID][]packageID
@@ -123,6 +123,15 @@
analyzer *analysis.Analyzer
}
+// knownMetadata is a wrapper around metadata that tracks its validity.
+type knownMetadata struct {
+ *metadata
+
+ // valid is true if the given metadata is valid.
+ // Invalid metadata can still be used if a metadata reload fails.
+ valid bool
+}
+
func (s *snapshot) ID() uint64 {
return s.id
}
@@ -503,13 +512,13 @@
if fh.Kind() != source.Go {
return nil, fmt.Errorf("no packages for non-Go file %s", uri)
}
- ids := s.getIDsForURI(uri)
- reload := len(ids) == 0
- for _, id := range ids {
+ knownIDs := s.getIDsForURI(uri)
+ reload := len(knownIDs) == 0
+ for _, id := range knownIDs {
// Reload package metadata if any of the metadata has missing
// dependencies, in case something has changed since the last time we
// reloaded it.
- if m := s.getMetadata(id); m == nil {
+ if s.noValidMetadataForID(id) {
reload = true
break
}
@@ -518,13 +527,21 @@
// calls to packages.Load. Determine what we should do instead.
}
if reload {
- if err := s.load(ctx, false, fileURI(uri)); err != nil {
+ err = s.load(ctx, false, fileURI(uri))
+
+ if !s.useInvalidMetadata() && err != nil {
+ return nil, err
+ }
+ // We've tried to reload and there are still no known IDs for the URI.
+ // Return the load error, if there was one.
+ knownIDs = s.getIDsForURI(uri)
+ if len(knownIDs) == 0 {
return nil, err
}
}
- // Get the list of IDs from the snapshot again, in case it has changed.
+
var phs []*packageHandle
- for _, id := range s.getIDsForURI(uri) {
+ for _, id := range knownIDs {
var parseModes []source.ParseMode
switch mode {
case source.TypecheckAll:
@@ -547,10 +564,16 @@
phs = append(phs, ph)
}
}
-
return phs, nil
}
+// Only use invalid metadata for Go versions >= 1.13. Go 1.12 and below has
+// issues with overlays that will cause confusing error messages if we reuse
+// old metadata.
+func (s *snapshot) useInvalidMetadata() bool {
+ return s.view.goversion >= 13 && s.view.Options().ExperimentalUseInvalidMetadata
+}
+
func (s *snapshot) GetReverseDependencies(ctx context.Context, id string) ([]source.Package, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
@@ -580,13 +603,15 @@
return ph.check(ctx, s)
}
-// transitiveReverseDependencies populates the uris map with file URIs
+// transitiveReverseDependencies populates the ids map with package IDs
// belonging to the provided package and its transitive reverse dependencies.
func (s *snapshot) transitiveReverseDependencies(id packageID, ids map[packageID]struct{}) {
if _, ok := ids[id]; ok {
return
}
- if s.getMetadata(id) == nil {
+ m := s.getMetadata(id)
+ // Only use invalid metadata if we support it.
+ if m == nil || !(m.valid || s.useInvalidMetadata()) {
return
}
ids[id] = struct{}{}
@@ -927,46 +952,64 @@
return s.ids[uri]
}
-func (s *snapshot) getMetadataForURILocked(uri span.URI) (metadata []*metadata) {
- // TODO(matloob): uri can be a file or directory. Should we update the mappings
- // to map directories to their contained packages?
-
- for _, id := range s.ids[uri] {
- if m, ok := s.metadata[id]; ok {
- metadata = append(metadata, m)
- }
- }
- return metadata
-}
-
-func (s *snapshot) getMetadata(id packageID) *metadata {
+func (s *snapshot) getMetadata(id packageID) *knownMetadata {
s.mu.Lock()
defer s.mu.Unlock()
return s.metadata[id]
}
+// noValidMetadataForURILocked reports whether there is any valid metadata for
+// the given URI.
+func (s *snapshot) noValidMetadataForURILocked(uri span.URI) bool {
+ ids, ok := s.ids[uri]
+ if !ok {
+ return true
+ }
+ for _, id := range ids {
+ if m, ok := s.metadata[id]; ok && m.valid {
+ return false
+ }
+ }
+ return true
+}
+
+// noValidMetadataForID reports whether there is no valid metadata for the
+// given ID.
+func (s *snapshot) noValidMetadataForID(id packageID) bool {
+ m := s.getMetadata(id)
+ return m == nil || !m.valid
+}
+
+// addID adds the given ID to the set of known IDs for the given URI.
+// Any existing invalid IDs are not preserved, and IDs that are not
+// "command-line-arguments" are preferred.
func (s *snapshot) addID(uri span.URI, id packageID) {
s.mu.Lock()
defer s.mu.Unlock()
+ var newIDs []packageID
for i, existingID := range s.ids[uri] {
- // TODO: We should make sure not to set duplicate IDs,
- // and instead panic here. This can be done by making sure not to
- // reset metadata information for packages we've already seen.
if existingID == id {
return
}
- // If we are setting a real ID, when the package had only previously
- // had a command-line-arguments ID, we should just replace it.
+ // If the package previously only had a command-line-arguments ID,
+ // we should just replace it.
if isCommandLineArguments(string(existingID)) {
s.ids[uri][i] = id
// Delete command-line-arguments if it was a workspace package.
delete(s.workspacePackages, existingID)
return
}
+ if m, ok := s.metadata[existingID]; !ok || !m.valid {
+ continue
+ }
+ newIDs = append(newIDs, existingID)
}
- s.ids[uri] = append(s.ids[uri], id)
+ sort.Slice(newIDs, func(i, j int) bool {
+ return newIDs[i] < newIDs[j]
+ })
+ s.ids[uri] = append(newIDs, id)
}
// isCommandLineArguments reports whether a given value denotes
@@ -1057,9 +1100,21 @@
// If we still have absolutely no metadata, check if the view failed to
// initialize and return any errors.
// TODO(rstambler): Should we clear the error after we return it?
+ useInvalidMetadata := s.useInvalidMetadata()
+
s.mu.Lock()
defer s.mu.Unlock()
- if len(s.metadata) == 0 && loadErr != nil {
+
+ // Only return a load error if we have no valid metadata.
+ if useInvalidMetadata && len(s.metadata) > 0 {
+ return nil
+ }
+ for _, m := range s.metadata {
+ if m.valid {
+ return nil
+ }
+ }
+ if loadErr != nil {
return loadErr.MainError
}
return nil
@@ -1169,12 +1224,14 @@
// reloadWorkspace reloads the metadata for all invalidated workspace packages.
func (s *snapshot) reloadWorkspace(ctx context.Context) error {
+ useInvalidMetadata := s.useInvalidMetadata()
+
// See which of the workspace packages are missing metadata.
s.mu.Lock()
missingMetadata := len(s.workspacePackages) == 0 || len(s.metadata) == 0
pkgPathSet := map[packagePath]struct{}{}
for id, pkgPath := range s.workspacePackages {
- if s.metadata[id] != nil {
+ if m, ok := s.metadata[id]; ok && (m.valid || useInvalidMetadata) {
continue
}
missingMetadata = true
@@ -1246,7 +1303,7 @@
s.mu.Lock()
for _, scope := range scopes {
uri := span.URI(scope.(fileURI))
- if s.getMetadataForURILocked(uri) == nil {
+ if s.noValidMetadataForURILocked(uri) {
s.unloadableFiles[uri] = struct{}{}
}
}
@@ -1279,7 +1336,7 @@
if _, ok := s.unloadableFiles[uri]; ok {
continue
}
- if s.getMetadataForURILocked(uri) == nil {
+ if s.noValidMetadataForURILocked(uri) {
files = append(files, fh)
}
}
@@ -1357,7 +1414,7 @@
initializedErr: s.initializedErr,
ids: make(map[span.URI][]packageID, len(s.ids)),
importedBy: make(map[packageID][]packageID, len(s.importedBy)),
- metadata: make(map[packageID]*metadata, len(s.metadata)),
+ metadata: make(map[packageID]*knownMetadata, len(s.metadata)),
packages: make(map[packageKey]*packageHandle, len(s.packages)),
actions: make(map[actionKey]*actionHandle, len(s.actions)),
files: make(map[span.URI]source.VersionedFileHandle, len(s.files)),
@@ -1481,10 +1538,10 @@
// 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.
- transitiveIDs := make(map[packageID]bool)
+ idsToInvalidate := make(map[packageID]bool)
var addRevDeps func(packageID, bool)
addRevDeps = func(id packageID, invalidateMetadata bool) {
- current, seen := transitiveIDs[id]
+ current, seen := idsToInvalidate[id]
newInvalidateMetadata := current || invalidateMetadata
// If we've already seen this ID, and the value of invalidate
@@ -1492,7 +1549,7 @@
if seen && current == newInvalidateMetadata {
return
}
- transitiveIDs[id] = newInvalidateMetadata
+ idsToInvalidate[id] = newInvalidateMetadata
for _, rid := range s.getImportedByLocked(id) {
addRevDeps(rid, invalidateMetadata)
}
@@ -1503,7 +1560,7 @@
// Copy the package type information.
for k, v := range s.packages {
- if _, ok := transitiveIDs[k.id]; ok {
+ if _, ok := idsToInvalidate[k.id]; ok {
continue
}
newGen.Inherit(v.handle)
@@ -1511,26 +1568,69 @@
}
// Copy the package analysis information.
for k, v := range s.actions {
- if _, ok := transitiveIDs[k.pkg.id]; ok {
+ if _, ok := idsToInvalidate[k.pkg.id]; ok {
continue
}
newGen.Inherit(v.handle)
result.actions[k] = v
}
+
+ // If the workspace mode has changed or a file has been deleted, we *must*
+ // delete all metadata, as it is unusable and may produce confusing or
+ // incorrect diagnostics.
+ workspaceModeChanged := s.workspaceMode() != result.workspaceMode()
+ deletedFiles := map[span.URI]struct{}{}
+ for _, c := range changes {
+ if c.exists {
+ continue
+ }
+ deletedFiles[c.fileHandle.URI()] = struct{}{}
+ }
+ skipID := map[packageID]struct{}{}
+ for uri, ids := range s.ids {
+ if _, ok := deletedFiles[uri]; ok {
+ for _, id := range ids {
+ skipID[id] = struct{}{}
+ }
+ }
+ }
+
+ // Copy the URI to package ID mappings, skipping only those URIs whose
+ // metadata will be reloaded in future calls to load.
+ deleteInvalidMetadata := forceReloadMetadata || workspaceModeChanged
+ idsInSnapshot := map[packageID]bool{} // track all known IDs
+ for uri, ids := range s.ids {
+ for _, id := range ids {
+ invalidateMetadata := idsToInvalidate[id]
+ if _, ok := skipID[id]; ok || (invalidateMetadata && deleteInvalidMetadata) {
+ continue
+ }
+ idsInSnapshot[id] = true
+ result.ids[uri] = append(result.ids[uri], id)
+ }
+ }
+
// Copy the package metadata. We only need to invalidate packages directly
// containing the affected file, and only if it changed in a relevant way.
for k, v := range s.metadata {
- if invalidateMetadata, ok := transitiveIDs[k]; invalidateMetadata && ok {
+ if !idsInSnapshot[k] {
+ // Delete metadata for IDs that are no longer reachable from files
+ // in the snapshot.
continue
}
- result.metadata[k] = v
+ invalidateMetadata := idsToInvalidate[k]
+ // Mark invalidated metadata rather than deleting it outright.
+ result.metadata[k] = &knownMetadata{
+ metadata: v.metadata,
+ valid: v.valid && !invalidateMetadata,
+ }
}
// Copy the URI to package ID mappings, skipping only those URIs whose
// metadata will be reloaded in future calls to load.
for k, ids := range s.ids {
var newIDs []packageID
for _, id := range ids {
- if invalidateMetadata, ok := transitiveIDs[id]; invalidateMetadata && ok {
+ if invalidateMetadata, ok := idsToInvalidate[id]; invalidateMetadata && ok {
continue
}
newIDs = append(newIDs, id)
@@ -1539,6 +1639,7 @@
result.ids[k] = newIDs
}
}
+
// Copy the set of initially loaded packages.
for id, pkgPath := range s.workspacePackages {
// Packages with the id "command-line-arguments" are generated by the
@@ -1546,7 +1647,7 @@
// module. Do not cache them as workspace packages for longer than
// necessary.
if isCommandLineArguments(string(id)) {
- if invalidateMetadata, ok := transitiveIDs[id]; invalidateMetadata && ok {
+ if invalidateMetadata, ok := idsToInvalidate[id]; invalidateMetadata && ok {
continue
}
}
@@ -1598,7 +1699,7 @@
// If the snapshot's workspace mode has changed, the packages loaded using
// the previous mode are no longer relevant, so clear them out.
- if s.workspaceMode() != result.workspaceMode() {
+ if workspaceModeChanged {
result.workspacePackages = map[packageID]packagePath{}
}
diff --git a/internal/lsp/fake/edit.go b/internal/lsp/fake/edit.go
index c3f07e2..8b04c39 100644
--- a/internal/lsp/fake/edit.go
+++ b/internal/lsp/fake/edit.go
@@ -18,6 +18,10 @@
Line, Column int
}
+func (p Pos) String() string {
+ return fmt.Sprintf("%v:%v", p.Line, p.Column)
+}
+
// Range corresponds to protocol.Range, but uses the editor friend Pos
// instead of UTF-16 oriented protocol.Position
type Range struct {
diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go
index 501d32c..684b47a 100644
--- a/internal/lsp/fake/editor.go
+++ b/internal/lsp/fake/editor.go
@@ -114,11 +114,10 @@
// Whether to edit files with windows line endings.
WindowsLineEndings bool
- DirectoryFilters []string
-
- VerboseOutput bool
-
- ImportShortcut string
+ ImportShortcut string
+ DirectoryFilters []string
+ VerboseOutput bool
+ ExperimentalUseInvalidMetadata bool
}
// NewEditor Creates a new Editor.
@@ -228,6 +227,9 @@
if e.Config.DirectoryFilters != nil {
config["directoryFilters"] = e.Config.DirectoryFilters
}
+ if e.Config.ExperimentalUseInvalidMetadata {
+ config["experimentalUseInvalidMetadata"] = true
+ }
if e.Config.CodeLenses != nil {
config["codelenses"] = e.Config.CodeLenses
}
diff --git a/internal/lsp/fake/workdir.go b/internal/lsp/fake/workdir.go
index aa9ef84..d836deb 100644
--- a/internal/lsp/fake/workdir.go
+++ b/internal/lsp/fake/workdir.go
@@ -190,6 +190,9 @@
if err := os.RemoveAll(fp); err != nil {
return errors.Errorf("removing %q: %w", path, err)
}
+ w.fileMu.Lock()
+ defer w.fileMu.Unlock()
+
evts := []FileEvent{{
Path: path,
ProtocolEvent: protocol.FileEvent{
@@ -198,6 +201,7 @@
},
}}
w.sendEvents(ctx, evts)
+ delete(w.files, path)
return nil
}
diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go
index 5eeacd8..c245fa7 100644
--- a/internal/lsp/regtest/runner.go
+++ b/internal/lsp/regtest/runner.go
@@ -44,7 +44,7 @@
// SeparateProcess forwards connection to a shared separate gopls process.
SeparateProcess
// Experimental enables all of the experimental configurations that are
- // being developed. Currently, it enables the workspace module.
+ // being developed.
Experimental
)
@@ -240,7 +240,7 @@
{"singleton", Singleton, singletonServer},
{"forwarded", Forwarded, r.forwardedServer},
{"separate_process", SeparateProcess, r.separateProcessServer},
- {"experimental_workspace_module", Experimental, experimentalWorkspaceModule},
+ {"experimental", Experimental, experimentalServer},
}
for _, tc := range tests {
@@ -395,9 +395,12 @@
return lsprpc.NewStreamServer(cache.New(optsHook), false)
}
-func experimentalWorkspaceModule(_ context.Context, _ *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
+func experimentalServer(_ context.Context, t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
options := func(o *source.Options) {
optsHook(o)
+ o.EnableAllExperiments()
+ // ExperimentalWorkspaceModule is not (as of writing) enabled by
+ // source.Options.EnableAllExperiments, but we want to test it.
o.ExperimentalWorkspaceModule = true
}
return lsprpc.NewStreamServer(cache.New(options), false)
diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go
index 3d140a5..ae61031 100755
--- a/internal/lsp/source/api_json.go
+++ b/internal/lsp/source/api_json.go
@@ -145,6 +145,19 @@
Hierarchy: "build",
},
{
+ Name: "experimentalUseInvalidMetadata",
+ Type: "bool",
+ Doc: "experimentalUseInvalidMetadata enables gopls to fall back on outdated\npackage metadata to provide editor features if the go command fails to\nload packages for some reason (like an invalid go.mod file). This will\neventually be the default behavior, and this setting will be removed.\n",
+ EnumKeys: EnumKeys{
+ ValueType: "",
+ Keys: nil,
+ },
+ EnumValues: nil,
+ Default: "false",
+ Status: "experimental",
+ Hierarchy: "build",
+ },
+ {
Name: "hoverKind",
Type: "enum",
Doc: "hoverKind controls the information that appears in the hover text.\nSingleLine and Structured are intended for use only by authors of editor plugins.\n",
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 91e5cb4..dfa631e 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -264,6 +264,12 @@
// downloads rather than requiring user action. This option will eventually
// be removed.
AllowImplicitNetworkAccess bool `status:"experimental"`
+
+ // ExperimentalUseInvalidMetadata enables gopls to fall back on outdated
+ // package metadata to provide editor features if the go command fails to
+ // load packages for some reason (like an invalid go.mod file). This will
+ // eventually be the default behavior, and this setting will be removed.
+ ExperimentalUseInvalidMetadata bool `status:"experimental"`
}
type UIOptions struct {
@@ -606,7 +612,7 @@
for name, value := range opts {
if b, ok := value.(bool); name == "allExperiments" && ok && b {
enableExperiments = true
- options.enableAllExperiments()
+ options.EnableAllExperiments()
}
}
seen := map[string]struct{}{}
@@ -714,13 +720,14 @@
o.StaticcheckAnalyzers[a.Name] = &Analyzer{Analyzer: a, Enabled: enabled}
}
-// enableAllExperiments turns on all of the experimental "off-by-default"
+// EnableAllExperiments turns on all of the experimental "off-by-default"
// features offered by gopls. Any experimental features specified in maps
// should be enabled in enableAllExperimentMaps.
-func (o *Options) enableAllExperiments() {
+func (o *Options) EnableAllExperiments() {
o.SemanticTokens = true
o.ExperimentalPostfixCompletions = true
o.ExperimentalTemplateSupport = true
+ o.ExperimentalUseInvalidMetadata = true
}
func (o *Options) enableAllExperimentMaps() {
@@ -920,6 +927,9 @@
case "allowImplicitNetworkAccess":
result.setBool(&o.AllowImplicitNetworkAccess)
+ case "experimentalUseInvalidMetadata":
+ result.setBool(&o.ExperimentalUseInvalidMetadata)
+
case "allExperiments":
// This setting should be handled before all of the other options are
// processed, so do nothing here.
