internal/lsp/cache: remove support for invalid metadata
Remove support for keeping track of invalid metadata, as discussed in
golang/go#55333.
Fixes golang/go#55333
Change-Id: I7727f43e2cffef610096d20af4898f326c5a8450
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447741
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 70c26f2..656d08f 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -729,8 +729,6 @@
// If experimentalUseInvalidMetadata is set, this function may return package
// IDs with invalid metadata.
func (s *snapshot) getOrLoadIDsForURI(ctx context.Context, uri span.URI) ([]PackageID, error) {
- useInvalidMetadata := s.useInvalidMetadata()
-
s.mu.Lock()
// Start with the set of package associations derived from the last load.
@@ -741,7 +739,7 @@
for _, id := range ids {
// TODO(rfindley): remove the defensiveness here. s.meta.metadata[id] must
// exist.
- if m, ok := s.meta.metadata[id]; ok && m.Valid {
+ if _, ok := s.meta.metadata[id]; ok {
hasValidID = true
}
if len(s.shouldLoad[id]) > 0 {
@@ -757,16 +755,6 @@
s.mu.Unlock()
- // Special case: if experimentalUseInvalidMetadata is set and we have any
- // ids, just return them.
- //
- // This is arguably wrong: if the metadata is invalid we should try reloading
- // it. However, this was the pre-existing behavior, and
- // experimentalUseInvalidMetadata will be removed in a future release.
- if !shouldLoad && useInvalidMetadata && len(ids) > 0 {
- return ids, nil
- }
-
// Reload if loading is likely to improve the package associations for uri:
// - uri is not contained in any valid packages
// - ...or one of the packages containing uri is marked 'shouldLoad'
@@ -800,28 +788,19 @@
s.mu.Lock()
ids = s.meta.ids[uri]
- if !useInvalidMetadata {
- var validIDs []PackageID
- for _, id := range ids {
- // TODO(rfindley): remove the defensiveness here as well.
- if m, ok := s.meta.metadata[id]; ok && m.Valid {
- validIDs = append(validIDs, id)
- }
+ var validIDs []PackageID
+ for _, id := range ids {
+ // TODO(rfindley): remove the defensiveness here as well.
+ if _, ok := s.meta.metadata[id]; ok {
+ validIDs = append(validIDs, id)
}
- ids = validIDs
}
+ ids = validIDs
s.mu.Unlock()
return ids, 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 PackageID) ([]source.Package, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
@@ -829,7 +808,7 @@
s.mu.Lock()
meta := s.meta
s.mu.Unlock()
- ids := meta.reverseTransitiveClosure(s.useInvalidMetadata(), id)
+ ids := meta.reverseTransitiveClosure(id)
// Make sure to delete the original package ID from the map.
delete(ids, id)
@@ -1216,9 +1195,7 @@
var meta []source.Metadata
for _, m := range s.meta.metadata {
- if m.Valid {
- meta = append(meta, m)
- }
+ meta = append(meta, m)
}
return meta, nil
}
@@ -1273,7 +1250,7 @@
return match
}
-func (s *snapshot) getMetadata(id PackageID) *KnownMetadata {
+func (s *snapshot) getMetadata(id PackageID) *Metadata {
s.mu.Lock()
defer s.mu.Unlock()
@@ -1319,7 +1296,7 @@
return true
}
for _, id := range ids {
- if m, ok := s.meta.metadata[id]; ok && m.Valid {
+ if _, ok := s.meta.metadata[id]; ok {
return false
}
}
@@ -1409,19 +1386,8 @@
func (s *snapshot) awaitLoaded(ctx context.Context) error {
loadErr := s.awaitLoadedAllErrors(ctx)
- s.mu.Lock()
- defer s.mu.Unlock()
-
- // If we still have absolutely no metadata, check if the view failed to
- // initialize and return any errors.
- if s.useInvalidMetadata() && len(s.meta.metadata) > 0 {
- return nil
- }
- for _, m := range s.meta.metadata {
- if m.Valid {
- return nil
- }
- }
+ // TODO(rfindley): eliminate this function as part of simplifying
+ // CriticalErrors.
if loadErr != nil {
return loadErr.MainError
}
@@ -1968,27 +1934,10 @@
result.shouldLoad[k] = v
}
- // TODO(rfindley): consolidate the this workspace mode detection with
- // workspace invalidation.
- workspaceModeChanged := s.workspaceMode() != result.workspaceMode()
-
- // We delete invalid metadata in the following cases:
- // - If we are forcing a reload of metadata.
- // - If the workspace mode has changed, as stale metadata may produce
- // confusing or incorrect diagnostics.
- //
- // TODO(rfindley): we should probably also clear metadata if we are
- // reinitializing the workspace, as otherwise we could leave around a bunch
- // of irrelevant and duplicate metadata (for example, if the module path
- // changed). However, this breaks the "experimentalUseInvalidMetadata"
- // feature, which relies on stale metadata when, for example, a go.mod file
- // is broken via invalid syntax.
- deleteInvalidMetadata := forceReloadMetadata || workspaceModeChanged
-
// Compute which metadata updates are required. We only need to invalidate
// packages directly containing the affected file, and only if it changed in
// a relevant way.
- metadataUpdates := make(map[PackageID]*KnownMetadata)
+ metadataUpdates := make(map[PackageID]*Metadata)
for k, v := range s.meta.metadata {
invalidateMetadata := idsToInvalidate[k]
@@ -2012,20 +1961,10 @@
}
// Check whether the metadata should be deleted.
- if skipID[k] || (invalidateMetadata && deleteInvalidMetadata) {
+ if skipID[k] || invalidateMetadata {
metadataUpdates[k] = nil
continue
}
-
- // Check if the metadata has changed.
- valid := v.Valid && !invalidateMetadata
- if valid != v.Valid {
- // Mark invalidated metadata rather than deleting it outright.
- metadataUpdates[k] = &KnownMetadata{
- Metadata: v.Metadata,
- Valid: valid,
- }
- }
}
// Update metadata, if necessary.
@@ -2042,6 +1981,10 @@
// Don't bother copying the importedBy graph,
// as it changes each time we update metadata.
+ // TODO(rfindley): consolidate the this workspace mode detection with
+ // workspace invalidation.
+ workspaceModeChanged := s.workspaceMode() != result.workspaceMode()
+
// If the snapshot's workspace mode has changed, the packages loaded using
// the previous mode are no longer relevant, so clear them out.
if workspaceModeChanged {
