Skip to content

Commit ad5dd98

Browse files
committed
gopls: fix a few bugs related to the new modcache imports source
Fix the following bugs related to the new "gopls" imports source and module cache index: - Only construct the modcacheState if the imports source is "gopls", which is not yet the default. This was causing memory regressions, as the modcache table is non-trivial. - Add missing error handling. - Don't call modcacheState.stopTimer if the modcacheState is nil, which may already have been the case with "importsSource": "off". For golang/go#71607 Change-Id: I33c90ee4b97c8675b342cb0c045eef183a1ef365 Reviewed-on: https://go-review.googlesource.com/c/tools/+/650397 Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 4b3fdfd commit ad5dd98

File tree

3 files changed

+19
-3
lines changed

3 files changed

+19
-3
lines changed

gopls/internal/cache/session.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,12 @@ func (s *Session) createView(ctx context.Context, def *viewDefinition) (*View, *
238238
viewDefinition: def,
239239
importsState: newImportsState(backgroundCtx, s.cache.modCache, pe),
240240
}
241-
if def.folder.Options.ImportsSource != settings.ImportsSourceOff {
241+
242+
// Keep this in sync with golang.computeImportEdits.
243+
//
244+
// TODO(rfindley): encapsulate the imports state logic so that the handling
245+
// for Options.ImportsSource is in a single location.
246+
if def.folder.Options.ImportsSource == settings.ImportsSourceGopls {
242247
v.modcacheState = newModcacheState(def.folder.Env.GOMODCACHE)
243248
}
244249

gopls/internal/cache/view.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ type View struct {
109109
// importsState is for the old imports code
110110
importsState *importsState
111111

112-
// maintain the current module cache index
112+
// modcacheState is the replacement for importsState, to be used for
113+
// goimports operations when the imports source is "gopls".
114+
//
115+
// It may be nil, if the imports source is not "gopls".
113116
modcacheState *modcacheState
114117

115118
// pkgIndex is an index of package IDs, for efficient storage of typerefs.
@@ -492,7 +495,9 @@ func (v *View) shutdown() {
492495
// Cancel the initial workspace load if it is still running.
493496
v.cancelInitialWorkspaceLoad()
494497
v.importsState.stopTimer()
495-
v.modcacheState.stopTimer()
498+
if v.modcacheState != nil {
499+
v.modcacheState.stopTimer()
500+
}
496501

497502
v.snapshotMu.Lock()
498503
if v.snapshot != nil {

gopls/internal/golang/format.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,13 @@ func computeImportEdits(ctx context.Context, pgf *parsego.File, snapshot *cache.
137137

138138
// Build up basic information about the original file.
139139
isource, err := imports.NewProcessEnvSource(options.Env, filename, pgf.File.Name.Name)
140+
if err != nil {
141+
return nil, nil, err
142+
}
140143
var source imports.Source
144+
145+
// Keep this in sync with [cache.Session.createView] (see the TODO there: we
146+
// should factor out the handling of the ImportsSource setting).
141147
switch snapshot.Options().ImportsSource {
142148
case settings.ImportsSourceGopls:
143149
source = snapshot.NewGoplsSource(isource)

0 commit comments

Comments
 (0)