internal/lsp: 'go get' packages instead of modules

Previously, we were running `go get` only on modules, which led to us
not downloading dependencies of packages. This resulted in further
go.mod diagnostics that users could not resolve with quick fixes. Now,
download packages directly so that dependencies are downloaded.

Fixes golang/go#44307

Change-Id: Id764ea5a2f7028e238eadaaba0ca3cfc765b85b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/293729
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

gopls/internal/regtest/modfile/modfile_test.go

@@ -1045,3 +1045,68 @@ example.com v1.2.3/go.mod h1:Y2Rc5rVWjWur0h3pd9aEvK5Pof8YKDANh9gHA2Maujo=
 		}
 	})
 }
+
+func TestDownloadDeps(t *testing.T) {
+	testenv.NeedsGo1Point(t, 14)
+
+	const proxy = `
+-- example.com@v1.2.3/go.mod --
+module example.com
+
+go 1.12
+
+require random.org v1.2.3
+-- example.com@v1.2.3/blah/blah.go --
+package blah
+
+import "random.org/bye"
+
+func SaySomething() {
+	bye.Goodbye()
+}
+-- random.org@v1.2.3/go.mod --
+module random.org
+
+go 1.12
+-- random.org@v1.2.3/bye/bye.go --
+package bye
+
+func Goodbye() {
+	println("Bye")
+}
+`
+
+	const mod = `
+-- go.mod --
+module mod.com
+
+go 1.12
+-- go.sum --
+-- main.go --
+package main
+
+import (
+	"example.com/blah"
+)
+
+func main() {
+	blah.SaySomething()
+}
+`
+	WithOptions(
+		ProxyFiles(proxy),
+		Modes(Singleton),
+	).Run(t, mod, func(t *testing.T, env *Env) {
+		env.OpenFile("main.go")
+		d := &protocol.PublishDiagnosticsParams{}
+		env.Await(
+			env.DiagnosticAtRegexpWithMessage("main.go", `"example.com/blah"`, `could not import example.com/blah (no required module provides package "example.com/blah")`),
+			ReadDiagnostics("main.go", d),
+		)
+		env.ApplyQuickFixes("main.go", d.Diagnostics)
+		env.Await(
+			EmptyDiagnostics("main.go"),
+			NoDiagnostics("go.mod"),
+		)
+	})
+}

internal/lsp/command.go

@@ -468,7 +468,13 @@ func (c *commandHandler) GoGetPackage(ctx context.Context, args command.GoGetPac
 		}
 		ver := strings.TrimSpace(stdout.String())
 		return c.s.runGoModUpdateCommands(ctx, deps.snapshot, args.URI.SpanURI(), func(invoke func(...string) (*bytes.Buffer, error)) error {
-			return runGoGetModule(invoke, args.AddRequire, []string{ver})
+			if args.AddRequire {
+				if err := addModuleRequire(invoke, []string{ver}); err != nil {
+					return err
+				}
+			}
+			_, err := invoke(append([]string{"get", "-d"}, args.Pkg)...)
+			return err
 		})
 	})
 }
@@ -556,11 +562,7 @@ func applyFileEdits(ctx context.Context, snapshot source.Snapshot, uri span.URI,
 
 func runGoGetModule(invoke func(...string) (*bytes.Buffer, error), addRequire bool, args []string) error {
 	if addRequire {
-		// Using go get to create a new dependency results in an
-		// `// indirect` comment we may not want. The only way to avoid it
-		// is to add the require as direct first. Then we can use go get to
-		// update go.sum and tidy up.
-		if _, err := invoke(append([]string{"mod", "edit", "-require"}, args...)...); err != nil {
+		if err := addModuleRequire(invoke, args); err != nil {
 			return err
 		}
 	}
@@ -568,6 +570,15 @@ func runGoGetModule(invoke func(...string) (*bytes.Buffer, error), addRequire bo
 	return err
 }
 
+func addModuleRequire(invoke func(...string) (*bytes.Buffer, error), args []string) error {
+	// Using go get to create a new dependency results in an
+	// `// indirect` comment we may not want. The only way to avoid it
+	// is to add the require as direct first. Then we can use go get to
+	// update go.sum and tidy up.
+	_, err := invoke(append([]string{"mod", "edit", "-require"}, args...)...)
+	return err
+}
+
 func (s *Server) getUpgrades(ctx context.Context, snapshot source.Snapshot, uri span.URI, modules []string) (map[string]string, error) {
 	stdout, err := snapshot.RunGoCommandDirect(ctx, source.Normal|source.AllowNetwork, &gocommand.Invocation{
 		Verb:       "list",