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 <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

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 = `
+-- [email protected]/go.mod --
+module example.com
+
+go 1.12
+
+require random.org v1.2.3
+-- [email protected]/blah/blah.go --
+package blah
+
+import "random.org/bye"
+
+func SaySomething() {
+	bye.Goodbye()
+}
+-- [email protected]/go.mod --
+module random.org
+
+go 1.12
+-- [email protected]/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,18 +562,23 @@ 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
 		}
 	}
 	_, err := invoke(append([]string{"get", "-d"}, args...)...)
 	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",