diff --git a/gopls/internal/regtest/modfile_test.go b/gopls/internal/regtest/modfile_test.go index 34e4080e05..338296e78c 100644 --- a/gopls/internal/regtest/modfile_test.go +++ b/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", "example.com@v1.2.3"), + ReadDiagnostics("go.mod", &d), + ), + ) + env.ApplyQuickFixes("go.mod", d.Diagnostics) env.Await( EmptyDiagnostics("go.mod"), diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go index 1f584adc28..512114b9d9 100644 --- a/gopls/internal/regtest/workspace_test.go +++ b/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", "b.com@v1.2.3"), + 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 := "b.com@v1.2.3/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"), )) diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 6a579fa04b..a127c69cf8 100644 --- a/internal/lsp/cache/load.go +++ b/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 { diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index 87a4baefd7..a5f0e1e134 100644 --- a/internal/lsp/cache/mod.go +++ b/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, diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go index 54f2180719..b613b838e7 100644 --- a/internal/lsp/cache/mod_tidy.go +++ b/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} } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 956daf1aef..79313176d5 100644 --- a/internal/lsp/cache/snapshot.go +++ b/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 diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 132acd1d59..0a54ee3e2b 100644 --- a/internal/lsp/cache/view.go +++ b/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 } diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 037433570f..6044d1305a 100644 --- a/internal/lsp/command.go +++ b/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, diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index 23f00599de..4ab70762cf 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/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 diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index a196b2fb4e..56a8c77d4b 100644 --- a/internal/lsp/source/view.go +++ b/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.