internal/lsp: disable network access for some go commands

For users with poor network connections, go command invocations that
access the network may hang up for long periods of time. Even for
users with good network connections, accessing proxy.golang.org or
github can take a few seconds, which is a long time to lock up an
editor.

Disable network access via GOPROXY=off when running most go commands.
There are two notable exceptions. First, the initial workspace load is
allowed, though subsequent loads are not. My reasoning is that if the
user is opening a project they've just downloaded, they probably expect
to fetch its dependencies. (Also, it's convenient to keep the regtests
going.) The second is the go mod tidy diagnostics, which I hope to
address in a later change. All the other commands that access the
network are at the user's request.

When the workspace load fails due to a dependency that hasn't been
downloaded, we present a quick fix on the go.mod line to do so. The only
way that happens is if there's a manual modification to the go.mod file,
since all of the other quick fixes will do the download. So I don't
think there's a need to present the fix anywhere else.

Updates golang/go#38462.

Change-Id: I470bc1ba790db7c1afee54d0b28fa0c6fd203fb9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274120
Trust: Heschi Kreinick <[email protected]>
Run-TryBot: Heschi Kreinick <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: heschi

gopls/internal/regtest/modfile_test.go

@@ -444,18 +444,14 @@ func main() {
 			env.RegexpReplace("go.mod", "v1.2.2", "v1.2.3")
 			env.Editor.SaveBuffer(env.Ctx, "go.mod") // go.mod changes must be on disk
 
-			// Bring the go.sum file back into sync. Multi-module workspace
-			// mode currently doesn't set -mod=readonly, so won't need to.
-			if !strings.Contains(t.Name(), "experimental") {
-				d := protocol.PublishDiagnosticsParams{}
-				env.Await(
-					OnceMet(
-						env.DiagnosticAtRegexp("go.mod", "module"),
-						ReadDiagnostics("go.mod", &d),
-					),
-				)
-				env.ApplyQuickFixes("go.mod", d.Diagnostics)
-			}
+			d := protocol.PublishDiagnosticsParams{}
+			env.Await(
+				OnceMet(
+					env.DiagnosticAtRegexpWithMessage("go.mod", "example.com v1.2.3", "[email protected]"),
+					ReadDiagnostics("go.mod", &d),
+				),
+			)
+			env.ApplyQuickFixes("go.mod", d.Diagnostics)
 
 			env.Await(
 				EmptyDiagnostics("go.mod"),

gopls/internal/regtest/workspace_test.go

@@ -14,6 +14,7 @@ import (
 
 	"golang.org/x/tools/internal/lsp"
 	"golang.org/x/tools/internal/lsp/fake"
+	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/testenv"
 )
 
@@ -272,6 +273,18 @@ func Hello() int {
 		env.Await(
 			CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 2),
 		)
+		if testenv.Go1Point() < 14 {
+			// On 1.14 and above, the go mod tidy diagnostics accidentally
+			// download for us. This is the behavior we actually want.
+			d := protocol.PublishDiagnosticsParams{}
+			env.Await(
+				OnceMet(
+					env.DiagnosticAtRegexpWithMessage("moda/a/go.mod", "require b.com v1.2.3", "[email protected]"),
+					ReadDiagnostics("moda/a/go.mod", &d),
+				),
+			)
+			env.ApplyQuickFixes("moda/a/go.mod", d.Diagnostics)
+		}
 		got, _ := env.GoToDefinition("moda/a/a.go", env.RegexpSearch("moda/a/a.go", "Hello"))
 		if want := "[email protected]/b/b.go"; !strings.HasSuffix(got, want) {
 			t.Errorf("expected %s, got %v", want, got)
@@ -448,16 +461,20 @@ require (
 replace a.com => %s/moda/a
 replace b.com => %s/modb
 `, workdir, workdir))
+		env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1))
+		// Check that go.mod diagnostics picked up the newly active mod file.
+		// The local version of modb has an extra dependency we need to download.
+		env.OpenFile("modb/go.mod")
+		var d protocol.PublishDiagnosticsParams
 		env.Await(
 			OnceMet(
-				CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1),
-				env.DiagnosticAtRegexp("modb/b/b.go", "x"),
+				env.DiagnosticAtRegexp("modb/go.mod", `require example.com v1.2.3`),
+				ReadDiagnostics("modb/go.mod", &d),
 			),
 		)
-		env.OpenFile("modb/go.mod")
-		// Check that go.mod diagnostics picked up the newly active mod file.
-		env.Await(env.DiagnosticAtRegexp("modb/go.mod", `require example.com v1.2.3`))
-		// ...and that jumping to definition now goes to b.com in the workspace.
+		env.ApplyQuickFixes("modb/go.mod", d.Diagnostics)
+		env.Await(env.DiagnosticAtRegexp("modb/b/b.go", "x"))
+		// Jumping to definition should now go to b.com in the workspace.
 		location, _ = env.GoToDefinition("moda/a/a.go", env.RegexpSearch("moda/a/a.go", "Hello"))
 		if want := "modb/b/b.go"; !strings.HasSuffix(location, want) {
 			t.Errorf("expected %s, got %v", want, location)
@@ -474,9 +491,8 @@ require (
 
 replace a.com => %s/moda/a
 `, workdir))
-		env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChange), 1))
 		env.Await(OnceMet(
-			CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChange), 1),
+			CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChange), 2),
 			EmptyDiagnostics("modb/go.mod"),
 		))
 

internal/lsp/cache/load.go

@@ -47,7 +47,7 @@ type metadata struct {
 
 // load calls packages.Load for the given scopes, updating package metadata,
 // import graph, and mapped files with the result.
-func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
+func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...interface{}) error {
 	var query []string
 	var containsDir bool // for logging
 	for _, scope := range scopes {
@@ -94,7 +94,11 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 	ctx, done := event.Start(ctx, "cache.view.load", tag.Query.Of(query))
 	defer done()
 
-	_, inv, cleanup, err := s.goCommandInvocation(ctx, source.LoadWorkspace, &gocommand.Invocation{
+	flags := source.LoadWorkspace
+	if allowNetwork {
+		flags |= source.AllowNetwork
+	}
+	_, inv, cleanup, err := s.goCommandInvocation(ctx, flags, &gocommand.Invocation{
 		WorkingDir: s.view.rootURI.Filename(),
 	})
 	if err != nil {

internal/lsp/cache/mod.go

@@ -290,7 +290,7 @@ func (s *snapshot) ModUpgrade(ctx context.Context, fh source.FileHandle) (map[st
 			// (see golang/go#38711).
 			inv.ModFlag = "readonly"
 		}
-		stdout, err := snapshot.RunGoCommandDirect(ctx, source.Normal, inv)
+		stdout, err := snapshot.RunGoCommandDirect(ctx, source.Normal|source.AllowNetwork, inv)
 		if err != nil {
 			return &modUpgradeData{err: err}
 		}
@@ -387,6 +387,26 @@ func (s *snapshot) extractGoCommandError(ctx context.Context, snapshot source.Sn
 		if err != nil {
 			return nil
 		}
+		if v.Path != "" && strings.Contains(goCmdError, "disabled by GOPROXY=off") {
+			args, err := source.MarshalArgs(fh.URI(), false, []string{fmt.Sprintf("%v@%v", v.Path, v.Version)})
+			if err != nil {
+				return nil
+			}
+			return &source.Error{
+				Message: fmt.Sprintf("%v@%v has not been downloaded", v.Path, v.Version),
+				Kind:    source.ListError,
+				Range:   rng,
+				URI:     fh.URI(),
+				SuggestedFixes: []source.SuggestedFix{{
+					Title: fmt.Sprintf("Download %v@%v", v.Path, v.Version),
+					Command: &protocol.Command{
+						Title:     source.CommandAddDependency.Title,
+						Command:   source.CommandAddDependency.ID(),
+						Arguments: args,
+					},
+				}},
+			}
+		}
 		return &source.Error{
 			Message: goCmdError,
 			Range:   rng,

internal/lsp/cache/mod_tidy.go

@@ -105,7 +105,7 @@ func (s *snapshot) ModTidy(ctx context.Context, pm *source.ParsedModule) (*sourc
 			Args:       []string{"tidy"},
 			WorkingDir: filepath.Dir(fh.URI().Filename()),
 		}
-		tmpURI, inv, cleanup, err := snapshot.goCommandInvocation(ctx, source.WriteTemporaryModFile, inv)
+		tmpURI, inv, cleanup, err := snapshot.goCommandInvocation(ctx, source.WriteTemporaryModFile|source.AllowNetwork, inv)
 		if err != nil {
 			return &modTidyData{err: err}
 		}

internal/lsp/cache/snapshot.go

@@ -228,7 +228,7 @@ func (s *snapshot) config(ctx context.Context, inv *gocommand.Invocation) *packa
 	return cfg
 }
 
-func (s *snapshot) RunGoCommandDirect(ctx context.Context, mode source.InvocationMode, inv *gocommand.Invocation) (*bytes.Buffer, error) {
+func (s *snapshot) RunGoCommandDirect(ctx context.Context, mode source.InvocationFlags, inv *gocommand.Invocation) (*bytes.Buffer, error) {
 	_, inv, cleanup, err := s.goCommandInvocation(ctx, mode, inv)
 	if err != nil {
 		return nil, err
@@ -238,7 +238,7 @@ func (s *snapshot) RunGoCommandDirect(ctx context.Context, mode source.Invocatio
 	return s.view.session.gocmdRunner.Run(ctx, *inv)
 }
 
-func (s *snapshot) RunGoCommandPiped(ctx context.Context, mode source.InvocationMode, inv *gocommand.Invocation, stdout, stderr io.Writer) error {
+func (s *snapshot) RunGoCommandPiped(ctx context.Context, mode source.InvocationFlags, inv *gocommand.Invocation, stdout, stderr io.Writer) error {
 	_, inv, cleanup, err := s.goCommandInvocation(ctx, mode, inv)
 	if err != nil {
 		return err
@@ -247,7 +247,7 @@ func (s *snapshot) RunGoCommandPiped(ctx context.Context, mode source.Invocation
 	return s.view.session.gocmdRunner.RunPiped(ctx, *inv, stdout, stderr)
 }
 
-func (s *snapshot) goCommandInvocation(ctx context.Context, mode source.InvocationMode, inv *gocommand.Invocation) (tmpURI span.URI, updatedInv *gocommand.Invocation, cleanup func(), err error) {
+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()
 	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...)
@@ -259,6 +259,11 @@ func (s *snapshot) goCommandInvocation(ctx context.Context, mode source.Invocati
 		return "", inv, cleanup, nil
 	}
 
+	mode, allowNetwork := flags.Mode(), flags.AllowNetwork()
+	if !allowNetwork {
+		inv.Env = append(inv.Env, "GOPROXY=off")
+	}
+
 	var modURI span.URI
 	// Select the module context to use.
 	// If we're type checking, we need to use the workspace context, meaning
@@ -453,7 +458,7 @@ func (s *snapshot) packageHandlesForFile(ctx context.Context, uri span.URI, mode
 		// calls to packages.Load. Determine what we should do instead.
 	}
 	if reload {
-		if err := s.load(ctx, fileURI(uri)); err != nil {
+		if err := s.load(ctx, false, fileURI(uri)); err != nil {
 			return nil, err
 		}
 	}
@@ -1008,7 +1013,7 @@ func (s *snapshot) reloadWorkspace(ctx context.Context) error {
 	// If the view's build configuration is invalid, we cannot reload by
 	// package path. Just reload the directory instead.
 	if missingMetadata && !s.ValidBuildConfiguration() {
-		return s.load(ctx, viewLoadScope("LOAD_INVALID_VIEW"))
+		return s.load(ctx, false, viewLoadScope("LOAD_INVALID_VIEW"))
 	}
 
 	if len(pkgPathSet) == 0 {
@@ -1019,7 +1024,7 @@ func (s *snapshot) reloadWorkspace(ctx context.Context) error {
 	for pkgPath := range pkgPathSet {
 		pkgPaths = append(pkgPaths, pkgPath)
 	}
-	return s.load(ctx, pkgPaths...)
+	return s.load(ctx, false, pkgPaths...)
 }
 
 func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
@@ -1032,7 +1037,7 @@ func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
 		return nil
 	}
 
-	err := s.load(ctx, scopes...)
+	err := s.load(ctx, false, scopes...)
 
 	// If we failed to load some files, i.e. they have no metadata,
 	// mark the failures so we don't bother retrying until the file's

internal/lsp/cache/view.go

@@ -548,7 +548,7 @@ func (s *snapshot) initialize(ctx context.Context, firstAttempt bool) {
 		if len(scopes) == 0 {
 			scopes = append(scopes, viewLoadScope("LOAD_VIEW"))
 		}
-		err := s.load(ctx, append(scopes, packagePath("builtin"))...)
+		err := s.load(ctx, firstAttempt, append(scopes, packagePath("builtin"))...)
 		if ctx.Err() != nil {
 			return
 		}

internal/lsp/command.go

@@ -203,7 +203,7 @@ func (s *Server) runCommand(ctx context.Context, work *workDone, command *source
 		if command == source.CommandVendor {
 			action = "vendor"
 		}
-		return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile, uri.SpanURI(), "mod", []string{action})
+		return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, uri.SpanURI(), "mod", []string{action})
 	case source.CommandUpdateGoSum:
 		var uri protocol.DocumentURI
 		if err := source.UnmarshalArgs(args, &uri); err != nil {
@@ -214,7 +214,7 @@ func (s *Server) runCommand(ctx context.Context, work *workDone, command *source
 		if !ok {
 			return err
 		}
-		return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile, uri.SpanURI(), "list", []string{"all"})
+		return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, uri.SpanURI(), "list", []string{"all"})
 	case source.CommandAddDependency, source.CommandUpgradeDependency, source.CommandRemoveDependency:
 		var uri protocol.DocumentURI
 		var goCmdArgs []string
@@ -397,10 +397,10 @@ func (s *Server) runGoGetModule(ctx context.Context, snapshot source.Snapshot, u
 			return err
 		}
 	}
-	return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile, uri, "get", append([]string{"-d"}, args...))
+	return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, uri, "get", append([]string{"-d"}, args...))
 }
 
-func runSimpleGoCommand(ctx context.Context, snapshot source.Snapshot, mode source.InvocationMode, uri span.URI, verb string, args []string) error {
+func runSimpleGoCommand(ctx context.Context, snapshot source.Snapshot, mode source.InvocationFlags, uri span.URI, verb string, args []string) error {
 	_, err := snapshot.RunGoCommandDirect(ctx, mode, &gocommand.Invocation{
 		Verb:       verb,
 		Args:       args,

internal/lsp/mod/diagnostics.go

@@ -13,6 +13,7 @@ import (
 	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
+	errors "golang.org/x/xerrors"
 )
 
 func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.VersionedFileIdentity][]*source.Diagnostic, error) {
@@ -36,7 +37,7 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.Vers
 				Range:   e.Range,
 				Source:  e.Category,
 			}
-			if e.Category == "syntax" {
+			if e.Category == "syntax" || e.Kind == source.ListError {
 				d.Severity = protocol.SeverityError
 			} else {
 				d.Severity = protocol.SeverityWarning
@@ -65,6 +66,12 @@ func ErrorsForMod(ctx context.Context, snapshot source.Snapshot, fh source.FileH
 		return nil, nil
 	}
 	if err != nil {
+		// Some error messages can also be displayed as diagnostics.
+		if criticalErr := (*source.CriticalError)(nil); errors.As(err, &criticalErr) {
+			return criticalErr.ErrorList, nil
+		} else if srcErrList := (source.ErrorList)(nil); errors.As(err, &srcErrList) {
+			return srcErrList, nil
+		}
 		return nil, err
 	}
 	return tidied.Errors, nil

internal/lsp/source/view.go

@@ -84,11 +84,11 @@ type Snapshot interface {
 
 	// RunGoCommandPiped runs the given `go` command, writing its output
 	// to stdout and stderr. Verb, Args, and WorkingDir must be specified.
-	RunGoCommandPiped(ctx context.Context, mode InvocationMode, inv *gocommand.Invocation, stdout, stderr io.Writer) error
+	RunGoCommandPiped(ctx context.Context, mode InvocationFlags, inv *gocommand.Invocation, stdout, stderr io.Writer) error
 
 	// RunGoCommandDirect runs the given `go` command. Verb, Args, and
 	// WorkingDir must be specified.
-	RunGoCommandDirect(ctx context.Context, mode InvocationMode, inv *gocommand.Invocation) (*bytes.Buffer, error)
+	RunGoCommandDirect(ctx context.Context, mode InvocationFlags, inv *gocommand.Invocation) (*bytes.Buffer, error)
 
 	// RunProcessEnvFunc runs fn with the process env for this snapshot's view.
 	// Note: the process env contains cached module and filesystem state.
@@ -161,13 +161,14 @@ const (
 	WidestPackage
 )
 
-// InvocationMode represents the goal of a particular go command invocation.
-type InvocationMode int
+// InvocationFlags represents the settings of a particular go command invocation.
+// It is a mode, plus a set of flag bits.
+type InvocationFlags int
 
 const (
 	// Normal is appropriate for commands that might be run by a user and don't
 	// deliberately modify go.mod files, e.g. `go test`.
-	Normal InvocationMode = iota
+	Normal InvocationFlags = iota
 	// UpdateUserModFile is for commands that intend to update the user's real
 	// go.mod file, e.g. `go mod tidy` in response to a user's request to tidy.
 	UpdateUserModFile
@@ -178,8 +179,20 @@ const (
 	// LoadWorkspace is for packages.Load, and other operations that should
 	// consider the whole workspace at once.
 	LoadWorkspace
+
+	// AllowNetwork is a flag bit that indicates the invocation should be
+	// allowed to access the network.
+	AllowNetwork = 1 << 10
 )
 
+func (m InvocationFlags) Mode() InvocationFlags {
+	return m & (AllowNetwork - 1)
+}
+
+func (m InvocationFlags) AllowNetwork() bool {
+	return m&AllowNetwork != 0
+}
+
 // View represents a single workspace.
 // This is the level at which we maintain configuration like working directory
 // and build tags.