internal/lsp: un-export (*snapshot).Config to limit it to cache
Limiting the Config to the view seems reasonable, considering that it is
only used to run the `go` command. I prefer just having the cache run
go commands, so that source doesn't have to deal with the environment.
This also enables CL 237517.
Change-Id: I639c082592de30e9682dc25cdd12c7751ddb4f97
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237600
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 1068784..64dbf16 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -83,7 +83,7 @@
ctx, done := event.Start(ctx, "cache.view.load", tag.Query.Of(query))
defer done()
- cfg := s.Config(ctx)
+ cfg := s.config(ctx)
pkgs, err := packages.Load(cfg, query...)
// If the context was canceled, return early. Otherwise, we might be
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index abbd98b..b9d41a8 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -138,7 +138,8 @@
}
realURI, tempURI := s.view.ModFiles()
folder := s.View().Folder().Filename()
- cfg := s.Config(ctx)
+ cfg := s.config(ctx)
+
key := modKey{
sessionID: s.view.session.id,
cfg: hashConfig(cfg),
@@ -288,7 +289,7 @@
}
realURI, tempURI := s.view.ModFiles()
- cfg := s.Config(ctx)
+ cfg := s.config(ctx)
options := s.View().Options()
folder := s.View().Folder().Filename()
gocmdRunner := s.view.gocmdRunner
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index cc975bb..0d400c8 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -5,11 +5,13 @@
package cache
import (
+ "bytes"
"context"
"fmt"
"go/ast"
"go/token"
"go/types"
+ "io"
"os"
"path/filepath"
"sort"
@@ -19,6 +21,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/packagesinternal"
@@ -91,7 +94,7 @@
// Config returns the configuration used for the snapshot's interaction with the
// go/packages API.
-func (s *snapshot) Config(ctx context.Context) *packages.Config {
+func (s *snapshot) config(ctx context.Context) *packages.Config {
s.view.optionsMu.Lock()
env, buildFlags := s.view.envLocked()
verboseOutput := s.view.options.VerboseOutput
@@ -130,6 +133,32 @@
return cfg
}
+func (s *snapshot) RunGoCommand(ctx context.Context, verb string, args []string) (*bytes.Buffer, error) {
+ cfg := s.config(ctx)
+ inv := gocommand.Invocation{
+ Verb: verb,
+ Args: args,
+ Env: cfg.Env,
+ BuildFlags: cfg.BuildFlags,
+ WorkingDir: s.view.folder.Filename(),
+ }
+ runner := packagesinternal.GetGoCmdRunner(cfg)
+ return runner.Run(ctx, inv)
+}
+
+func (s *snapshot) RunGoCommandPiped(ctx context.Context, verb string, args []string, stdout, stderr io.Writer) error {
+ cfg := s.config(ctx)
+ inv := gocommand.Invocation{
+ Verb: verb,
+ Args: args,
+ Env: cfg.Env,
+ BuildFlags: cfg.BuildFlags,
+ WorkingDir: s.view.folder.Filename(),
+ }
+ runner := packagesinternal.GetGoCmdRunner(cfg)
+ return runner.RunPiped(ctx, inv, stdout, stderr)
+}
+
func (s *snapshot) buildOverlay() map[string][]byte {
s.mu.Lock()
defer s.mu.Unlock()
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index a5bf043..5b17f7c 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -417,6 +417,8 @@
return processEnv, nil
}
+// envLocked returns the environment and build flags for the current view.
+// It assumes that the caller is holding the view's optionsMu.
func (v *View) envLocked() ([]string, []string) {
// We want to run the go commands with the -modfile flag if the version of go
// that we are using supports it.
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 159df2a..6693e30 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -7,15 +7,11 @@
import (
"context"
"io"
- "path/filepath"
"strings"
- "golang.org/x/tools/internal/event"
- "golang.org/x/tools/internal/gocommand"
- "golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
- "golang.org/x/tools/internal/packagesinternal"
+ "golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/xcontext"
errors "golang.org/x/xerrors"
)
@@ -40,12 +36,11 @@
if err != nil {
return nil, err
}
- snapshot, fh, ok, err := s.beginFileRequest(ctx, protocol.DocumentURI(uri), source.Go)
- if !ok {
+ view, err := s.session.ViewOf(uri)
+ if err != nil {
return nil, err
}
- dir := filepath.Dir(fh.URI().Filename())
- go s.runTest(ctx, funcName, dir, snapshot)
+ go s.runTest(ctx, view.Snapshot(), funcName)
case source.CommandGenerate:
dir, recursive, err := getGenerateRequest(params.Arguments)
if err != nil {
@@ -90,44 +85,26 @@
if err != nil {
return err
}
- snapshot := view.Snapshot()
- cfg := snapshot.Config(ctx)
- inv := gocommand.Invocation{
- Verb: verb,
- Args: args,
- Env: cfg.Env,
- WorkingDir: view.Folder().Filename(),
- }
- gocmdRunner := packagesinternal.GetGoCmdRunner(cfg)
- _, err = gocmdRunner.Run(ctx, inv)
+ _, err = view.Snapshot().RunGoCommand(ctx, verb, args)
return err
}
-func (s *Server) runTest(ctx context.Context, funcName string, dir string, snapshot source.Snapshot) error {
+func (s *Server) runTest(ctx context.Context, snapshot source.Snapshot, funcName string) error {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
- er := &eventWriter{ctx: ctx, operation: "test"}
+ ew := &eventWriter{ctx: ctx, operation: "test"}
wc := s.newProgressWriter(ctx, "test", "running "+funcName, cancel)
defer wc.Close()
messageType := protocol.Info
message := "test passed"
- stderr := io.MultiWriter(er, wc)
+ stderr := io.MultiWriter(ew, wc)
- cfg := snapshot.Config(ctx)
- inv := gocommand.Invocation{
- Verb: "test",
- Args: []string{"-run", funcName, dir},
- Env: cfg.Env,
- WorkingDir: dir,
- }
- runner := packagesinternal.GetGoCmdRunner(cfg)
- if err := runner.RunPiped(ctx, inv, er, stderr); err != nil {
+ if err := snapshot.RunGoCommandPiped(ctx, "test", []string{"-run", funcName}, ew, stderr); err != nil {
if errors.Is(err, context.Canceled) {
- return nil
+ return err
}
- event.Error(ctx, "test: command error", err, tag.Directory.Of(dir))
messageType = protocol.Error
message = "test failed"
}
@@ -137,22 +114,19 @@
})
}
-func getRunTestArguments(args []interface{}) (string, string, error) {
+func getRunTestArguments(args []interface{}) (string, span.URI, error) {
if len(args) != 2 {
return "", "", errors.Errorf("expected one test func name and one file path, got %v", args)
}
-
funcName, ok := args[0].(string)
if !ok {
return "", "", errors.Errorf("expected func name to be a string, got %T", args[0])
}
-
- file, ok := args[1].(string)
+ filename, ok := args[1].(string)
if !ok {
return "", "", errors.Errorf("expected file to be a string, got %T", args[1])
}
-
- return funcName, file, nil
+ return funcName, span.URIFromPath(filename), nil
}
func getGenerateRequest(args []interface{}) (string, bool, error) {
diff --git a/internal/lsp/generate.go b/internal/lsp/generate.go
index 970142e..d850dec 100644
--- a/internal/lsp/generate.go
+++ b/internal/lsp/generate.go
@@ -9,10 +9,8 @@
"io"
"golang.org/x/tools/internal/event"
- "golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/protocol"
- "golang.org/x/tools/internal/packagesinternal"
"golang.org/x/tools/internal/span"
errors "golang.org/x/xerrors"
)
@@ -40,15 +38,7 @@
return err
}
snapshot := view.Snapshot()
- cfg := snapshot.Config(ctx)
- inv := gocommand.Invocation{
- Verb: "generate",
- Args: args,
- Env: cfg.Env,
- WorkingDir: dir,
- }
- runner := packagesinternal.GetGoCmdRunner(cfg)
- if err := runner.RunPiped(ctx, inv, er, stderr); err != nil {
+ if err := snapshot.RunGoCommandPiped(ctx, "generate", args, er, stderr); err != nil {
if errors.Is(err, context.Canceled) {
return nil
}
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index cf0b4c5..e75deae 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -12,7 +12,6 @@
"os/exec"
"path/filepath"
"sort"
- "strings"
"testing"
"golang.org/x/tools/go/packages/packagestest"
@@ -69,11 +68,9 @@
// Check to see if the -modfile flag is available, this is basically a check
// to see if the go version >= 1.14. Otherwise, the modfile specific tests
// will always fail if this flag is not available.
- for _, flag := range view.Snapshot().Config(ctx).BuildFlags {
- if strings.Contains(flag, "-modfile=") {
- datum.ModfileFlagAvailable = true
- break
- }
+ if _, tmp := view.ModFiles(); tmp != "" {
+ datum.ModfileFlagAvailable = true
+ break
}
var modifications []source.FileModification
for filename, content := range datum.Config.Overlay {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index f1649e5..385000f 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -5,6 +5,7 @@
package source
import (
+ "bytes"
"context"
"fmt"
"go/ast"
@@ -28,9 +29,6 @@
// View returns the View associated with this snapshot.
View() View
- // Config returns the configuration for the view.
- Config(ctx context.Context) *packages.Config
-
// FindFile returns the FileHandle for the given URI, if it is already
// in the given snapshot.
FindFile(uri span.URI) FileHandle
@@ -48,6 +46,13 @@
// Analyze runs the analyses for the given package at this snapshot.
Analyze(ctx context.Context, pkgID string, analyzers ...*analysis.Analyzer) ([]*Error, error)
+ // RunGoCommandPiped runs the given `go` command in the view, using the
+ // provided stdout and stderr.
+ RunGoCommandPiped(ctx context.Context, verb string, args []string, stdout, stderr io.Writer) error
+
+ // RunGoCommand runs the given `go` command in the view.
+ RunGoCommand(ctx context.Context, verb string, args []string) (*bytes.Buffer, error)
+
// ModTidyHandle returns a ModTidyHandle for the given go.mod file handle.
// This function can have no data or error if there is no modfile detected.
ModTidyHandle(ctx context.Context, fh FileHandle) (ModTidyHandle, error)
@@ -110,7 +115,8 @@
// Folder returns the root folder for this view.
Folder() span.URI
- // ModFiles returns the URIs of the go.mod files attached to the view associated with this snapshot.
+ // ModFiles returns the URIs of the go.mod files attached to the view
+ // associated with this snapshot.
ModFiles() (span.URI, span.URI)
// BuiltinPackage returns the go/ast.Object for the given name in the builtin package.
