gopls/internal/lsp/source: delete Snapshot.WriteEnv method

This method is called once immediately after View construction.
It used to run 'go version', but with minor tweaks, the View
already has this information via wsInfo.

This change removes it from the Snapshot interface and
eliminates the unnecessary 'go version' subprocess.

Change-Id: I86e8dd37a7a237949c05820ca3e4fdb5035f90f7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/459782
gopls-CI: kokoro <[email protected]>
Run-TryBot: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: adonovan

gopls/internal/lsp/cache/session.go

@@ -286,6 +286,9 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 	// Save one reference in the view.
 	v.releaseSnapshot = v.snapshot.Acquire()
 
+	// Record the environment of the newly created view in the log.
+	event.Log(ctx, viewEnv(v))
+
 	// Initialize the view without blocking.
 	initCtx, initCancel := context.WithCancel(xcontext.Detach(ctx))
 	v.initCancelFirstAttempt = initCancel

gopls/internal/lsp/cache/view.go

@@ -6,10 +6,10 @@
 package cache
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
 	"path"
@@ -123,8 +123,10 @@ type workspaceInformation struct {
 	// The Go version in use: X in Go 1.X.
 	goversion int
 
-	// The Go version reported by go version command. (e.g. go1.19.1, go1.20-rc.1, go1.21-abcdef01)
-	goversionString string
+	// The complete output of the go version command.
+	// (Call gocommand.ParseGoVersionOutput to extract a version
+	// substring such as go1.19.1 or go1.20-rc.1, go1.21-abcdef01.)
+	goversionOutput string
 
 	// hasGopackagesDriver is true if the user has a value set for the
 	// GOPACKAGESDRIVER environment variable or a gopackagesdriver binary on
@@ -346,14 +348,28 @@ func (s *Session) SetViewOptions(ctx context.Context, v *View, options *source.O
 	return newView, err
 }
 
-func (s *snapshot) WriteEnv(ctx context.Context, w io.Writer) error {
-	s.view.optionsMu.Lock()
-	env := s.view.options.EnvSlice()
-	buildFlags := append([]string{}, s.view.options.BuildFlags...)
-	s.view.optionsMu.Unlock()
+// viewEnv returns a string describing the environment of a newly created view.
+func viewEnv(v *View) string {
+	v.optionsMu.Lock()
+	env := v.options.EnvSlice()
+	buildFlags := append([]string{}, v.options.BuildFlags...)
+	v.optionsMu.Unlock()
+
+	var buf bytes.Buffer
+	fmt.Fprintf(&buf, `go env for %v
+(root %s)
+(go version %s)
+(valid build configuration = %v)
+(build flags: %v)
+`,
+		v.folder.Filename(),
+		v.rootURI.Filename(),
+		strings.TrimRight(v.workspaceInformation.goversionOutput, "\n"),
+		v.snapshot.ValidBuildConfiguration(),
+		buildFlags)
 
 	fullEnv := make(map[string]string)
-	for k, v := range s.view.goEnv {
+	for k, v := range v.goEnv {
 		fullEnv[k] = v
 	}
 	for _, v := range env {
@@ -365,29 +381,11 @@ func (s *snapshot) WriteEnv(ctx context.Context, w io.Writer) error {
 			fullEnv[s[0]] = s[1]
 		}
 	}
-	goVersion, err := s.view.gocmdRunner.Run(ctx, gocommand.Invocation{
-		Verb:       "version",
-		Env:        env,
-		WorkingDir: s.view.rootURI.Filename(),
-	})
-	if err != nil {
-		return err
-	}
-	fmt.Fprintf(w, `go env for %v
-(root %s)
-(go version %s)
-(valid build configuration = %v)
-(build flags: %v)
-`,
-		s.view.folder.Filename(),
-		s.view.rootURI.Filename(),
-		strings.TrimRight(goVersion.String(), "\n"),
-		s.ValidBuildConfiguration(),
-		buildFlags)
 	for k, v := range fullEnv {
-		fmt.Fprintf(w, "%s=%s\n", k, v)
+		fmt.Fprintf(&buf, "%s=%s\n", k, v)
 	}
-	return nil
+
+	return buf.String()
 }
 
 func (s *snapshot) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error) error {
@@ -816,7 +814,7 @@ func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI,
 	if err != nil {
 		return nil, err
 	}
-	goversionString, err := gocommand.GoVersionString(ctx, inv, s.gocmdRunner)
+	goversionOutput, err := gocommand.GoVersionOutput(ctx, inv, s.gocmdRunner)
 	if err != nil {
 		return nil, err
 	}
@@ -847,7 +845,7 @@ func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI,
 	return &workspaceInformation{
 		hasGopackagesDriver:  hasGopackagesDriver,
 		goversion:            goversion,
-		goversionString:      goversionString,
+		goversionOutput:      goversionOutput,
 		environmentVariables: envVars,
 		goEnv:                env,
 	}, nil
@@ -1072,7 +1070,7 @@ func (v *View) GoVersion() int {
 }
 
 func (v *View) GoVersionString() string {
-	return v.workspaceInformation.goversionString
+	return gocommand.ParseGoVersionOutput(v.workspaceInformation.goversionOutput)
 }
 
 // Copied from

gopls/internal/lsp/general.go

@@ -5,7 +5,6 @@
 package lsp
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -339,15 +338,6 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol
 		}
 		// Inv: release() must be called once.
 
-		// Print each view's environment.
-		var buf bytes.Buffer
-		if err := snapshot.WriteEnv(ctx, &buf); err != nil {
-			viewErrors[uri] = err
-			release()
-			continue
-		}
-		event.Log(ctx, buf.String())
-
 		// Initialize snapshot asynchronously.
 		initialized := make(chan struct{})
 		nsnapshots.Add(1)

gopls/internal/lsp/source/view.go

@@ -74,9 +74,6 @@ type Snapshot interface {
 	// and their GOPATH.
 	ValidBuildConfiguration() bool
 
-	// WriteEnv writes the view-specific environment to the io.Writer.
-	WriteEnv(ctx context.Context, w io.Writer) error
-
 	// FindFile returns the FileHandle for the given URI, if it is already
 	// in the given snapshot.
 	FindFile(uri span.URI) VersionedFileHandle

internal/gocommand/version.go

@@ -58,22 +58,24 @@ func GoVersion(ctx context.Context, inv Invocation, r *Runner) (int, error) {
 	return 0, fmt.Errorf("no parseable ReleaseTags in %v", tags)
 }
 
-// GoVersionString reports the go version string as shown in `go version` command output.
-// When `go version` outputs in non-standard form, this returns an empty string.
-func GoVersionString(ctx context.Context, inv Invocation, r *Runner) (string, error) {
+// GoVersionOutput returns the complete output of the go version command.
+func GoVersionOutput(ctx context.Context, inv Invocation, r *Runner) (string, error) {
 	inv.Verb = "version"
 	goVersion, err := r.Run(ctx, inv)
 	if err != nil {
 		return "", err
 	}
-	return parseGoVersionOutput(goVersion.Bytes()), nil
+	return goVersion.String(), nil
 }
 
-func parseGoVersionOutput(data []byte) string {
+// ParseGoVersionOutput extracts the Go version string
+// from the output of the "go version" command.
+// Given an unrecognized form, it returns an empty string.
+func ParseGoVersionOutput(data string) string {
 	re := regexp.MustCompile(`^go version (go\S+|devel \S+)`)
-	m := re.FindSubmatch(data)
+	m := re.FindStringSubmatch(data)
 	if len(m) != 2 {
 		return "" // unrecognized version
 	}
-	return string(m[1])
+	return m[1]
 }

internal/gocommand/version_test.go

@@ -23,7 +23,7 @@ func TestParseGoVersionOutput(t *testing.T) {
 	}
 	for i, tt := range tests {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			if got := parseGoVersionOutput([]byte(tt.args)); got != tt.want {
+			if got := ParseGoVersionOutput(tt.args); got != tt.want {
 				t.Errorf("parseGoVersionOutput() = %v, want %v", got, tt.want)
 			}
 		})