diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go index 5676a7814a2..dfb09951494 100644 --- a/gopls/internal/lsp/cache/analysis.go +++ b/gopls/internal/lsp/cache/analysis.go @@ -192,7 +192,7 @@ func (snapshot *snapshot) Analyze(ctx context.Context, pkgs map[PackageID]unit, toSrc := make(map[*analysis.Analyzer]*source.Analyzer) var enabled []*analysis.Analyzer // enabled subset + transitive requirements for _, a := range analyzers { - if a.IsEnabled(snapshot.view.Options()) { + if a.IsEnabled(snapshot.options) { toSrc[a.Analyzer] = a enabled = append(enabled, a.Analyzer) } @@ -309,7 +309,7 @@ func (snapshot *snapshot) Analyze(ctx context.Context, pkgs map[PackageID]unit, // Now that we have read all files, // we no longer need the snapshot. // (but options are needed for progress reporting) - options := snapshot.view.Options() + options := snapshot.options snapshot = nil // Progress reporting. If supported, gopls reports progress on analysis diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go index b7267e983ec..438da1604e6 100644 --- a/gopls/internal/lsp/cache/check.go +++ b/gopls/internal/lsp/cache/check.go @@ -1345,8 +1345,8 @@ func (s *snapshot) typeCheckInputs(ctx context.Context, m *source.Metadata) (typ depsByImpPath: m.DepsByImpPath, goVersion: goVersion, - relatedInformation: s.view.Options().RelatedInformationSupported, - linkTarget: s.view.Options().LinkTarget, + relatedInformation: s.options.RelatedInformationSupported, + linkTarget: s.options.LinkTarget, moduleMode: s.view.moduleMode(), }, nil } diff --git a/gopls/internal/lsp/cache/imports.go b/gopls/internal/lsp/cache/imports.go index f22d42c21a8..028607608cc 100644 --- a/gopls/internal/lsp/cache/imports.go +++ b/gopls/internal/lsp/cache/imports.go @@ -54,15 +54,13 @@ func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *snapshot // view.goEnv is immutable -- changes make a new view. Options can change. // We can't compare build flags directly because we may add -modfile. - snapshot.view.optionsMu.Lock() - localPrefix := snapshot.view.options.Local - currentBuildFlags := snapshot.view.options.BuildFlags - currentDirectoryFilters := snapshot.view.options.DirectoryFilters + localPrefix := snapshot.options.Local + currentBuildFlags := snapshot.options.BuildFlags + currentDirectoryFilters := snapshot.options.DirectoryFilters changed := !reflect.DeepEqual(currentBuildFlags, s.cachedBuildFlags) || - snapshot.view.options.VerboseOutput != (s.processEnv.Logf != nil) || + snapshot.options.VerboseOutput != (s.processEnv.Logf != nil) || modFileHash != s.cachedModFileHash || - !reflect.DeepEqual(snapshot.view.options.DirectoryFilters, s.cachedDirectoryFilters) - snapshot.view.optionsMu.Unlock() + !reflect.DeepEqual(snapshot.options.DirectoryFilters, s.cachedDirectoryFilters) // If anything relevant to imports has changed, clear caches and // update the processEnv. Clearing caches blocks on any background @@ -120,7 +118,7 @@ func populateProcessEnvFromSnapshot(ctx context.Context, pe *imports.ProcessEnv, ctx, done := event.Start(ctx, "cache.populateProcessEnvFromSnapshot") defer done() - if snapshot.view.Options().VerboseOutput { + if snapshot.options.VerboseOutput { pe.Logf = func(format string, args ...interface{}) { event.Log(ctx, fmt.Sprintf(format, args...)) } diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go index df68ba7429d..331e0e7ebc7 100644 --- a/gopls/internal/lsp/cache/load.go +++ b/gopls/internal/lsp/cache/load.go @@ -75,7 +75,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc if err != nil { continue } - if isStandaloneFile(contents, s.view.Options().StandaloneTags) { + if isStandaloneFile(contents, s.options.StandaloneTags) { standalone = true query = append(query, uri.Filename()) } else { @@ -160,7 +160,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc } moduleErrs := make(map[string][]packages.Error) // module path -> errors - filterFunc := s.view.filterFunc() + filterFunc := s.filterFunc() newMetadata := make(map[PackageID]*source.Metadata) for _, pkg := range pkgs { // The Go command returns synthetic list results for module queries that @@ -178,7 +178,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc continue } - if !containsDir || s.view.Options().VerboseOutput { + if !containsDir || s.options.VerboseOutput { event.Log(ctx, eventName, append( source.SnapshotLabels(s), tag.Package.Of(pkg.ID), @@ -359,7 +359,7 @@ func (s *snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, fi for _, fh := range files { // Place the diagnostics on the package or module declarations. var rng protocol.Range - switch s.view.FileKind(fh) { + switch s.FileKind(fh) { case source.Go: if pgf, err := s.ParseGo(ctx, fh, source.ParseHeader); err == nil { // Check that we have a valid `package foo` range to use for positioning the error. @@ -616,7 +616,7 @@ func containsPackageLocked(s *snapshot, m *source.Metadata) bool { uris[uri] = struct{}{} } - filterFunc := s.view.filterFunc() + filterFunc := s.filterFunc() for uri := range uris { // Don't use view.contains here. go.work files may include modules // outside of the workspace folder. @@ -671,7 +671,7 @@ func containsFileInWorkspaceLocked(s *snapshot, m *source.Metadata) bool { // The package's files are in this view. It may be a workspace package. // Vendored packages are not likely to be interesting to the user. - if !strings.Contains(string(uri), "/vendor/") && s.view.contains(uri) { + if !strings.Contains(string(uri), "/vendor/") && s.contains(uri) { return true } } diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go index edfb78ec979..e2869dd6e29 100644 --- a/gopls/internal/lsp/cache/session.go +++ b/gopls/internal/lsp/cache/session.go @@ -117,9 +117,9 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, v := &View{ id: strconv.FormatInt(index, 10), gocmdRunner: s.gocmdRunner, + lastOptions: options, initialWorkspaceLoad: make(chan struct{}), initializationSema: make(chan struct{}, 1), - options: options, baseCtx: baseCtx, name: name, moduleUpgrades: map[span.URI]map[string]string{}, @@ -167,6 +167,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, workspaceModFiles: wsModFiles, workspaceModFilesErr: wsModFilesErr, pkgIndex: typerefs.NewPackageIndex(), + options: options, } // Save one reference in the view. v.releaseSnapshot = v.snapshot.Acquire() @@ -255,9 +256,15 @@ func bestViewForURI(uri span.URI, views []*View) *View { } // TODO(rfindley): this should consider the workspace layout (i.e. // go.work). - if view.contains(uri) { + snapshot, release, err := view.getSnapshot() + if err != nil { + // view is shutdown + continue + } + if snapshot.contains(uri) { longest = view } + release() } if longest != nil { return longest @@ -420,7 +427,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif // synchronously to change processing? Can we assume that the env did not // change, and derive go.work using a combination of the configured // GOWORK value and filesystem? - info, err := s.getWorkspaceInformation(ctx, view.folder, view.Options()) + info, err := s.getWorkspaceInformation(ctx, view.folder, view.lastOptions) if err != nil { // Catastrophic failure, equivalent to a failure of session // initialization and therefore should almost never happen. One @@ -434,7 +441,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif } if info != view.workspaceInformation { - if err := s.updateViewLocked(ctx, view, view.Options()); err != nil { + if err := s.updateViewLocked(ctx, view, view.lastOptions); err != nil { // More catastrophic failure. The view may or may not still exist. // The best we can do is log and move on. event.Error(ctx, "recreating view", err) @@ -492,7 +499,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif var releases []func() viewToSnapshot := map[*View]*snapshot{} for view, changed := range views { - snapshot, release := view.invalidateContent(ctx, changed, forceReloadMetadata) + snapshot, release := view.invalidateContent(ctx, changed, nil, forceReloadMetadata) releases = append(releases, release) viewToSnapshot[view] = snapshot } diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go index 4010c985747..9d659f046dc 100644 --- a/gopls/internal/lsp/cache/snapshot.go +++ b/gopls/internal/lsp/cache/snapshot.go @@ -187,6 +187,10 @@ type snapshot struct { // active packages. Running type checking batches in parallel after an // invalidation can cause redundant calculation of this shared state. typeCheckMu sync.Mutex + + // options holds the user configuration at the time this snapshot was + // created. + options *source.Options } var globalSnapshotID uint64 @@ -275,12 +279,39 @@ func (s *snapshot) View() source.View { return s.view } -func (s *snapshot) FileKind(h source.FileHandle) source.FileKind { - return s.view.FileKind(h) +func (s *snapshot) FileKind(fh source.FileHandle) source.FileKind { + // The kind of an unsaved buffer comes from the + // TextDocumentItem.LanguageID field in the didChange event, + // not from the file name. They may differ. + if o, ok := fh.(*Overlay); ok { + if o.kind != source.UnknownKind { + return o.kind + } + } + + fext := filepath.Ext(fh.URI().Filename()) + switch fext { + case ".go": + return source.Go + case ".mod": + return source.Mod + case ".sum": + return source.Sum + case ".work": + return source.Work + } + exts := s.options.TemplateExtensions + for _, ext := range exts { + if fext == ext || fext == "."+ext { + return source.Tmpl + } + } + // and now what? This should never happen, but it does for cgo before go1.15 + return source.Go } func (s *snapshot) Options() *source.Options { - return s.view.Options() // temporarily return view options. + return s.options // temporarily return view options. } func (s *snapshot) BackgroundContext() context.Context { @@ -306,7 +337,7 @@ func (s *snapshot) Templates() map[span.URI]source.FileHandle { tmpls := map[span.URI]source.FileHandle{} s.files.Range(func(k span.URI, fh source.FileHandle) { - if s.view.FileKind(fh) == source.Tmpl { + if s.FileKind(fh) == source.Tmpl { tmpls[k] = fh } }) @@ -354,8 +385,7 @@ func (s *snapshot) workspaceMode() workspaceMode { return mode } mode |= moduleMode - options := s.view.Options() - if options.TempModfile { + if s.options.TempModfile { mode |= tempModfile } return mode @@ -368,9 +398,6 @@ func (s *snapshot) workspaceMode() workspaceMode { // multiple modules in on config, so buildOverlay needs to filter overlays by // module. func (s *snapshot) config(ctx context.Context, inv *gocommand.Invocation) *packages.Config { - s.view.optionsMu.Lock() - verboseOutput := s.view.options.VerboseOutput - s.view.optionsMu.Unlock() cfg := &packages.Config{ Context: ctx, @@ -393,7 +420,7 @@ func (s *snapshot) config(ctx context.Context, inv *gocommand.Invocation) *packa panic("go/packages must not be used to parse files") }, Logf: func(format string, args ...interface{}) { - if verboseOutput { + if s.options.VerboseOutput { event.Log(ctx, fmt.Sprintf(format, args...)) } }, @@ -475,18 +502,16 @@ func (s *snapshot) RunGoCommands(ctx context.Context, allowNetwork bool, wd stri // it used only after call to tempModFile. Clarify that it is only // non-nil on success. func (s *snapshot) goCommandInvocation(ctx context.Context, flags source.InvocationFlags, inv *gocommand.Invocation) (tmpURI span.URI, updatedInv *gocommand.Invocation, cleanup func(), err error) { - s.view.optionsMu.Lock() - allowModfileModificationOption := s.view.options.AllowModfileModifications - allowNetworkOption := s.view.options.AllowImplicitNetworkAccess + allowModfileModificationOption := s.options.AllowModfileModifications + allowNetworkOption := s.options.AllowImplicitNetworkAccess // TODO(rfindley): this is very hard to follow, and may not even be doing the // right thing: should inv.Env really trample view.options? Do we ever invoke // this with a non-empty inv.Env? // // We should refactor to make it clearer that the correct env is being used. - inv.Env = append(append(append(os.Environ(), s.view.options.EnvSlice()...), inv.Env...), "GO111MODULE="+s.view.GO111MODULE()) - inv.BuildFlags = append([]string{}, s.view.options.BuildFlags...) - s.view.optionsMu.Unlock() + inv.Env = append(append(append(os.Environ(), s.options.EnvSlice()...), inv.Env...), "GO111MODULE="+s.view.GO111MODULE()) + inv.BuildFlags = append([]string{}, s.options.BuildFlags...) cleanup = func() {} // fallback // All logic below is for module mode. @@ -993,8 +1018,7 @@ func (s *snapshot) workspaceDirs(ctx context.Context) []string { // Code) that do not send notifications for individual files in a directory // when the entire directory is deleted. func (s *snapshot) watchSubdirs() bool { - opts := s.view.Options() - switch p := opts.SubdirWatchPatterns; p { + switch p := s.options.SubdirWatchPatterns; p { case source.SubdirWatchPatternsOn: return true case source.SubdirWatchPatternsOff: @@ -1007,7 +1031,7 @@ func (s *snapshot) watchSubdirs() bool { // requirements that client names do not change. We should update the VS // Code extension to set a default value of "subdirWatchPatterns" to "on", // so that this workaround is only temporary. - if opts.ClientInfo != nil && opts.ClientInfo.Name == "Visual Studio Code" { + if s.options.ClientInfo != nil && s.options.ClientInfo.Name == "Visual Studio Code" { return true } return false @@ -1505,7 +1529,7 @@ func (s *snapshot) reloadOrphanedOpenFiles(ctx context.Context) error { var files []*Overlay for _, o := range open { uri := o.URI() - if s.IsBuiltin(uri) || s.view.FileKind(o) != source.Go { + if s.IsBuiltin(uri) || s.FileKind(o) != source.Go { continue } if len(meta.ids[uri]) == 0 { @@ -1606,7 +1630,7 @@ func (s *snapshot) OrphanedFileDiagnostics(ctx context.Context) (map[span.URI]*s searchOverlays: for _, o := range s.overlays() { uri := o.URI() - if s.IsBuiltin(uri) || s.view.FileKind(o) != source.Go { + if s.IsBuiltin(uri) || s.FileKind(o) != source.Go { continue } md, err := s.MetadataForFile(ctx, uri) @@ -1805,7 +1829,7 @@ func inVendor(uri span.URI) bool { return found && strings.Contains(after, "/") } -func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]source.FileHandle, forceReloadMetadata bool) (*snapshot, func()) { +func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]source.FileHandle, newOptions *source.Options, forceReloadMetadata bool) (*snapshot, func()) { ctx, done := event.Start(ctx, "cache.snapshot.clone") defer done() @@ -1838,6 +1862,11 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]source workspaceModFilesErr: s.workspaceModFilesErr, importGraph: s.importGraph, pkgIndex: s.pkgIndex, + options: s.options, + } + + if newOptions != nil { + result.options = newOptions } // Create a lease on the new snapshot. diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go index a71fc1d6acd..7c2eda1743b 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -46,8 +46,11 @@ type View struct { // name is the user-specified name of this view. name string - optionsMu sync.Mutex - options *source.Options + // lastOptions holds the most recent options on this view, used for detecting + // major changes. + // + // Guarded by Session.viewMu. + lastOptions *source.Options // Workspace information. The fields below are immutable, and together with // options define the build list. Any change to these fields results in a new @@ -412,48 +415,18 @@ func (v *View) Folder() span.URI { return v.folder } -func (v *View) Options() *source.Options { - v.optionsMu.Lock() - defer v.optionsMu.Unlock() - return v.options -} - -func (v *View) FileKind(fh source.FileHandle) source.FileKind { - // The kind of an unsaved buffer comes from the - // TextDocumentItem.LanguageID field in the didChange event, - // not from the file name. They may differ. - if o, ok := fh.(*Overlay); ok { - if o.kind != source.UnknownKind { - return o.kind - } - } - - fext := filepath.Ext(fh.URI().Filename()) - switch fext { - case ".go": - return source.Go - case ".mod": - return source.Mod - case ".sum": - return source.Sum - case ".work": - return source.Work - } - exts := v.Options().TemplateExtensions - for _, ext := range exts { - if fext == ext || fext == "."+ext { - return source.Tmpl - } - } - // and now what? This should never happen, but it does for cgo before go1.15 - return source.Go -} - func minorOptionsChange(a, b *source.Options) bool { // TODO(rfindley): this function detects whether a view should be recreated, // but this is also checked by the getWorkspaceInformation logic. // // We should eliminate this redundancy. + // + // Additionally, this function has existed for a long time, but git history + // suggests that it was added arbitrarily, not due to an actual performance + // problem. + // + // Especially now that we have optimized reinitialization of the session, we + // should consider just always creating a new view on any options change. // Check if any of the settings that modify our understanding of files have // been changed. @@ -503,13 +476,12 @@ func (s *Session) SetFolderOptions(ctx context.Context, uri span.URI, options *s func (s *Session) setViewOptions(ctx context.Context, v *View, options *source.Options) error { // no need to rebuild the view if the options were not materially changed - v.optionsMu.Lock() - if minorOptionsChange(v.options, options) { - v.options = options - v.optionsMu.Unlock() + if minorOptionsChange(v.lastOptions, options) { + _, release := v.invalidateContent(ctx, nil, options, false) + release() + v.lastOptions = options return nil } - v.optionsMu.Unlock() return s.updateViewLocked(ctx, v, options) } @@ -517,8 +489,8 @@ func (s *Session) setViewOptions(ctx context.Context, v *View, options *source.O // // It must not be called concurrently with any other view methods. func viewEnv(v *View) string { - env := v.options.EnvSlice() - buildFlags := append([]string{}, v.options.BuildFlags...) + env := v.snapshot.options.EnvSlice() + buildFlags := append([]string{}, v.snapshot.options.BuildFlags...) var buf bytes.Buffer fmt.Fprintf(&buf, `go info for %v @@ -567,13 +539,13 @@ func fileHasExtension(path string, suffixes []string) bool { // locateTemplateFiles ensures that the snapshot has mapped template files // within the workspace folder. func (s *snapshot) locateTemplateFiles(ctx context.Context) { - if len(s.view.Options().TemplateExtensions) == 0 { + if len(s.options.TemplateExtensions) == 0 { return } - suffixes := s.view.Options().TemplateExtensions + suffixes := s.options.TemplateExtensions searched := 0 - filterFunc := s.view.filterFunc() + filterFunc := s.filterFunc() err := filepath.WalkDir(s.view.folder.Filename(), func(path string, entry os.DirEntry, err error) error { if err != nil { return err @@ -607,33 +579,33 @@ func (s *snapshot) locateTemplateFiles(ctx context.Context) { } } -func (v *View) contains(uri span.URI) bool { +func (s *snapshot) contains(uri span.URI) bool { // If we've expanded the go dir to a parent directory, consider if the // expanded dir contains the uri. // TODO(rfindley): should we ignore the root here? It is not provided by the // user. It would be better to explicitly consider the set of active modules // wherever relevant. inGoDir := false - if source.InDir(v.goCommandDir.Filename(), v.folder.Filename()) { - inGoDir = source.InDir(v.goCommandDir.Filename(), uri.Filename()) + if source.InDir(s.view.goCommandDir.Filename(), s.view.folder.Filename()) { + inGoDir = source.InDir(s.view.goCommandDir.Filename(), uri.Filename()) } - inFolder := source.InDir(v.folder.Filename(), uri.Filename()) + inFolder := source.InDir(s.view.folder.Filename(), uri.Filename()) if !inGoDir && !inFolder { return false } - return !v.filterFunc()(uri) + return !s.filterFunc()(uri) } // filterFunc returns a func that reports whether uri is filtered by the currently configured // directoryFilters. -func (v *View) filterFunc() func(span.URI) bool { - filterer := buildFilterer(v.folder.Filename(), v.gomodcache, v.Options()) +func (s *snapshot) filterFunc() func(span.URI) bool { + filterer := buildFilterer(s.view.folder.Filename(), s.view.gomodcache, s.options) return func(uri span.URI) bool { // Only filter relative to the configured root directory. - if source.InDir(v.folder.Filename(), uri.Filename()) { - return pathExcludedByFilter(strings.TrimPrefix(uri.Filename(), v.folder.Filename()), filterer) + if source.InDir(s.view.folder.Filename(), uri.Filename()) { + return pathExcludedByFilter(strings.TrimPrefix(uri.Filename(), s.view.folder.Filename()), filterer) } return false } @@ -660,7 +632,12 @@ func (v *View) relevantChange(c source.FileModification) bool { // had neither test nor associated issue, and cited only emacs behavior, this // logic was deleted. - return v.contains(c.URI) + snapshot, release, err := v.getSnapshot() + if err != nil { + return false // view was shut down + } + defer release() + return snapshot.contains(c.URI) } func (v *View) markKnown(uri span.URI) { @@ -938,7 +915,9 @@ func (s *snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) (loadEr // // invalidateContent returns a non-nil snapshot for the new content, along with // a callback which the caller must invoke to release that snapshot. -func (v *View) invalidateContent(ctx context.Context, changes map[span.URI]source.FileHandle, forceReloadMetadata bool) (*snapshot, func()) { +// +// newOptions may be nil, in which case options remain unchanged. +func (v *View) invalidateContent(ctx context.Context, changes map[span.URI]source.FileHandle, newOptions *source.Options, forceReloadMetadata bool) (*snapshot, func()) { // Detach the context so that content invalidation cannot be canceled. ctx = xcontext.Detach(ctx) @@ -960,7 +939,7 @@ func (v *View) invalidateContent(ctx context.Context, changes map[span.URI]sourc prevSnapshot.AwaitInitialized(ctx) // Save one lease of the cloned snapshot in the view. - v.snapshot, v.releaseSnapshot = prevSnapshot.clone(ctx, v.baseCtx, changes, forceReloadMetadata) + v.snapshot, v.releaseSnapshot = prevSnapshot.clone(ctx, v.baseCtx, changes, newOptions, forceReloadMetadata) prevReleaseSnapshot() v.destroy(prevSnapshot, "View.invalidateContent") diff --git a/gopls/internal/lsp/cmd/capabilities_test.go b/gopls/internal/lsp/cmd/capabilities_test.go index ddf56851937..02ff0d950f6 100644 --- a/gopls/internal/lsp/cmd/capabilities_test.go +++ b/gopls/internal/lsp/cmd/capabilities_test.go @@ -15,6 +15,7 @@ import ( "golang.org/x/tools/gopls/internal/lsp" "golang.org/x/tools/gopls/internal/lsp/cache" "golang.org/x/tools/gopls/internal/lsp/protocol" + "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/internal/testenv" ) @@ -49,7 +50,8 @@ func TestCapabilities(t *testing.T) { // Send an initialize request to the server. ctx := context.Background() client := newClient(app, nil) - server := lsp.NewServer(cache.NewSession(ctx, cache.New(nil)), client, app.options) + options := source.DefaultOptions(app.options) + server := lsp.NewServer(cache.NewSession(ctx, cache.New(nil)), client, options) result, err := server.Initialize(ctx, params) if err != nil { t.Fatal(err) diff --git a/gopls/internal/lsp/cmd/cmd.go b/gopls/internal/lsp/cmd/cmd.go index 2e1e6229edd..9a9d627880a 100644 --- a/gopls/internal/lsp/cmd/cmd.go +++ b/gopls/internal/lsp/cmd/cmd.go @@ -316,7 +316,8 @@ func (app *Application) connect(ctx context.Context, onProgress func(*protocol.P switch { case app.Remote == "": client := newClient(app, onProgress) - server := lsp.NewServer(cache.NewSession(ctx, cache.New(nil)), client, app.options) + options := source.DefaultOptions(app.options) + server := lsp.NewServer(cache.NewSession(ctx, cache.New(nil)), client, options) conn := newConnection(server, client) if err := conn.initialize(protocol.WithClient(ctx, client), app.options); err != nil { return nil, err @@ -326,10 +327,7 @@ func (app *Application) connect(ctx context.Context, onProgress func(*protocol.P case strings.HasPrefix(app.Remote, "internal@"): internalMu.Lock() defer internalMu.Unlock() - opts := source.DefaultOptions().Clone() - if app.options != nil { - app.options(opts) - } + opts := source.DefaultOptions(app.options) key := fmt.Sprintf("%s %v %v %v", app.wd, opts.PreferredContentFormat, opts.HierarchicalDocumentSymbolSupport, opts.SymbolMatcher) if c := internalConnections[key]; c != nil { return c, nil @@ -376,10 +374,7 @@ func (c *connection) initialize(ctx context.Context, options func(*source.Option params.Capabilities.Workspace.Configuration = true // Make sure to respect configured options when sending initialize request. - opts := source.DefaultOptions().Clone() - if options != nil { - options(opts) - } + opts := source.DefaultOptions(options) // If you add an additional option here, you must update the map key in connect. params.Capabilities.TextDocument.Hover = &protocol.HoverClientCapabilities{ ContentFormat: []protocol.MarkupKind{opts.PreferredContentFormat}, diff --git a/gopls/internal/lsp/cmd/info.go b/gopls/internal/lsp/cmd/info.go index d63e24b6bfc..b0f08bbef67 100644 --- a/gopls/internal/lsp/cmd/info.go +++ b/gopls/internal/lsp/cmd/info.go @@ -299,8 +299,7 @@ gopls also includes software made available under these licenses: ` func (l *licenses) Run(ctx context.Context, args ...string) error { - opts := source.DefaultOptions() - l.app.options(opts) + opts := source.DefaultOptions(l.app.options) txt := licensePreamble if opts.LicensesText == "" { txt += "(development gopls, license information not available)" diff --git a/gopls/internal/lsp/completion_test.go b/gopls/internal/lsp/completion_test.go index 1da7ade563d..48ec9ea094e 100644 --- a/gopls/internal/lsp/completion_test.go +++ b/gopls/internal/lsp/completion_test.go @@ -171,13 +171,12 @@ func (r *runner) toggleOptions(t *testing.T, uri span.URI, options func(*source. } folder := view.Folder() - original := view.Options() - modified := view.Options().Clone() + modified := r.server.Options().Clone() options(modified) if err = r.server.session.SetFolderOptions(r.ctx, folder, modified); err != nil { t.Fatal(err) } return func() { - r.server.session.SetFolderOptions(r.ctx, folder, original) + r.server.session.SetFolderOptions(r.ctx, folder, r.server.Options()) } } diff --git a/gopls/internal/lsp/debug/serve.go b/gopls/internal/lsp/debug/serve.go index c0bfd9e7b50..8aa2938c228 100644 --- a/gopls/internal/lsp/debug/serve.go +++ b/gopls/internal/lsp/debug/serve.go @@ -846,8 +846,6 @@ var ViewTmpl = template.Must(template.Must(BaseTemplate.Clone()).Parse(` {{define "body"}} Name: {{.Name}}
Folder: {{.Folder}}
-

Environment

- {{end}} `)) diff --git a/gopls/internal/lsp/general.go b/gopls/internal/lsp/general.go index e2f8ddbbf67..61c16c0531a 100644 --- a/gopls/internal/lsp/general.go +++ b/gopls/internal/lsp/general.go @@ -623,7 +623,7 @@ func (s *Server) beginFileRequest(ctx context.Context, pURI protocol.DocumentURI release() return nil, nil, false, func() {}, err } - if expectKind != source.UnknownKind && view.FileKind(fh) != expectKind { + if expectKind != source.UnknownKind && snapshot.FileKind(fh) != expectKind { // Wrong kind of file. Nothing to do. release() return nil, nil, false, func() {}, nil diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go index 285f78a7f40..88f6a3940f1 100644 --- a/gopls/internal/lsp/lsp_test.go +++ b/gopls/internal/lsp/lsp_test.go @@ -52,8 +52,7 @@ func testLSP(t *testing.T, datum *tests.Data) { ctx = debug.WithInstance(ctx, "", "off") session := cache.NewSession(ctx, cache.New(nil)) - options := source.DefaultOptions().Clone() - tests.DefaultOptions(options) + options := source.DefaultOptions(tests.DefaultOptions) options.SetEnvSlice(datum.Config.Env) view, snapshot, release, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), options) if err != nil { @@ -112,7 +111,7 @@ func testLSP(t *testing.T, datum *tests.Data) { editRecv: make(chan map[span.URI][]byte, 1), } - r.server = NewServer(session, testClient{runner: r}, nil) + r.server = NewServer(session, testClient{runner: r}, options) tests.Run(t, r, datum) } diff --git a/gopls/internal/lsp/lsprpc/lsprpc.go b/gopls/internal/lsp/lsprpc/lsprpc.go index ef2c3ed0c54..7dc709291fb 100644 --- a/gopls/internal/lsp/lsprpc/lsprpc.go +++ b/gopls/internal/lsp/lsprpc/lsprpc.go @@ -59,7 +59,8 @@ func (s *StreamServer) Binder() *ServerBinder { session := cache.NewSession(ctx, s.cache) server := s.serverForTest if server == nil { - server = lsp.NewServer(session, client, s.optionsOverrides) + options := source.DefaultOptions(s.optionsOverrides) + server = lsp.NewServer(session, client, options) if instance := debug.GetInstance(ctx); instance != nil { instance.AddService(server, session) } @@ -76,7 +77,8 @@ func (s *StreamServer) ServeStream(ctx context.Context, conn jsonrpc2.Conn) erro session := cache.NewSession(ctx, s.cache) server := s.serverForTest if server == nil { - server = lsp.NewServer(session, client, s.optionsOverrides) + options := source.DefaultOptions(s.optionsOverrides) + server = lsp.NewServer(session, client, options) if instance := debug.GetInstance(ctx); instance != nil { instance.AddService(server, session) } diff --git a/gopls/internal/lsp/server.go b/gopls/internal/lsp/server.go index bddf86458ea..a236779962f 100644 --- a/gopls/internal/lsp/server.go +++ b/gopls/internal/lsp/server.go @@ -26,11 +26,7 @@ const concurrentAnalyses = 1 // NewServer creates an LSP server and binds it to handle incoming client // messages on the supplied stream. -func NewServer(session *cache.Session, client protocol.ClientCloser, optionsOverrides func(*source.Options)) *Server { - options := source.DefaultOptions().Clone() - if optionsOverrides != nil { - optionsOverrides(options) - } +func NewServer(session *cache.Session, client protocol.ClientCloser, options *source.Options) *Server { return &Server{ diagnostics: map[span.URI]*fileReports{}, gcOptimizationDetails: make(map[source.PackageID]struct{}), diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go index 2b91f834d6a..a0802361bf2 100644 --- a/gopls/internal/lsp/source/options.go +++ b/gopls/internal/lsp/source/options.go @@ -81,7 +81,7 @@ var ( // DefaultOptions is the options that are used for Gopls execution independent // of any externally provided configuration (LSP initialization, command // invocation, etc.). -func DefaultOptions() *Options { +func DefaultOptions(overrides ...func(*Options)) *Options { optionsOnce.Do(func() { var commands []string for _, c := range command.Commands { @@ -189,7 +189,13 @@ func DefaultOptions() *Options { }, } }) - return defaultOptions + options := defaultOptions.Clone() + for _, override := range overrides { + if override != nil { + override(options) + } + } + return options } // Options holds various configuration that affects Gopls execution, organized