From 6192b9875128c5f53a69b959d5a1abf0f10ae93f Mon Sep 17 00:00:00 2001
From: Jay Conrod <jayconrod@google.com>
Date: Thu, 7 Jan 2021 11:14:06 -0500
Subject: [PATCH] cmd/go: make hints in error messages more consistent

* All commands the user can run to fix the problem now appear alone on
  a separate line after a tab.
* Removed -d from 'go get' commands.
* Replaced 'go mod tidy' with 'go mod download $modpath' when a
  package might be provided by a module missing a sum.
* Errors about 'path@version' syntax are more explicit.

Fixes #29415
Fixes #42087
Fixes #43430
Fixes #43523

Change-Id: I4427c2c4506a727a2c727d652fd2d506bb134d3b
Reviewed-on: https://go-review.googlesource.com/c/go/+/282121
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
---
 src/cmd/go/go_test.go                              |  4 ++--
 src/cmd/go/internal/get/get.go                     |  2 +-
 src/cmd/go/internal/load/pkg.go                    |  6 +-----
 src/cmd/go/internal/modget/get.go                  |  2 +-
 src/cmd/go/internal/modload/import.go              | 14 ++++++++------
 src/cmd/go/internal/modload/init.go                |  2 +-
 src/cmd/go/internal/modload/load.go                |  2 +-
 src/cmd/go/internal/modload/vendor.go              |  2 +-
 src/cmd/go/internal/test/testflag.go               |  2 +-
 .../testdata/script/mod_get_promote_implicit.txt   | 10 +++++++---
 src/cmd/go/testdata/script/mod_get_retract.txt     |  2 +-
 src/cmd/go/testdata/script/mod_invalid_path.txt    |  2 +-
 src/cmd/go/testdata/script/mod_sum_ambiguous.txt   |  8 ++++++--
 src/cmd/go/testdata/script/mod_sum_readonly.txt    |  2 +-
 src/cmd/go/testdata/script/mod_vendor_auto.txt     |  6 +++---
 src/cmd/go/testdata/script/mod_versions.txt        |  6 +++---
 src/cmd/go/testdata/script/test_flag.txt           |  6 +++---
 17 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index c472620db29..3cd3454d5a0 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -2655,12 +2655,12 @@ func TestBadCommandLines(t *testing.T) {
 	tg.tempFile("src/@x/x.go", "package x\n")
 	tg.setenv("GOPATH", tg.path("."))
 	tg.runFail("build", "@x")
-	tg.grepStderr("invalid input directory name \"@x\"|cannot use path@version syntax", "did not reject @x directory")
+	tg.grepStderr("invalid input directory name \"@x\"|can only use path@version syntax with 'go get' and 'go install' in module-aware mode", "did not reject @x directory")
 
 	tg.tempFile("src/@x/y/y.go", "package y\n")
 	tg.setenv("GOPATH", tg.path("."))
 	tg.runFail("build", "@x/y")
-	tg.grepStderr("invalid import path \"@x/y\"|cannot use path@version syntax", "did not reject @x/y import path")
+	tg.grepStderr("invalid import path \"@x/y\"|can only use path@version syntax with 'go get' and 'go install' in module-aware mode", "did not reject @x/y import path")
 
 	tg.tempFile("src/-x/x.go", "package x\n")
 	tg.setenv("GOPATH", tg.path("."))
diff --git a/src/cmd/go/internal/get/get.go b/src/cmd/go/internal/get/get.go
index 94a42c4f73a..38ff3823f22 100644
--- a/src/cmd/go/internal/get/get.go
+++ b/src/cmd/go/internal/get/get.go
@@ -202,7 +202,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
 func downloadPaths(patterns []string) []string {
 	for _, arg := range patterns {
 		if strings.Contains(arg, "@") {
-			base.Fatalf("go: cannot use path@version syntax in GOPATH mode")
+			base.Fatalf("go: can only use path@version syntax with 'go get' and 'go install' in module-aware mode")
 			continue
 		}
 
diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index 855f9698a2c..cffc8fcefa5 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -769,11 +769,7 @@ func loadPackageData(path, parentPath, parentDir, parentRoot string, parentIsStd
 	}
 
 	if strings.Contains(path, "@") {
-		if cfg.ModulesEnabled {
-			return nil, false, errors.New("can only use path@version syntax with 'go get'")
-		} else {
-			return nil, false, errors.New("cannot use path@version syntax in GOPATH mode")
-		}
+		return nil, false, errors.New("can only use path@version syntax with 'go get' and 'go install' in module-aware mode")
 	}
 
 	// Determine canonical package path and directory.
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index 8463ec4e9c7..0770b601c0a 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -1558,7 +1558,7 @@ func (r *resolver) checkPackagesAndRetractions(ctx context.Context, pkgPatterns
 		}
 	}
 	if retractPath != "" {
-		fmt.Fprintf(os.Stderr, "go: run 'go get %s@latest' to switch to the latest unretracted version\n", retractPath)
+		fmt.Fprintf(os.Stderr, "go: to switch to the latest unretracted version, run:\n\tgo get %s@latest", retractPath)
 	}
 }
 
diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go
index 055878c528a..9925d5b9054 100644
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -134,6 +134,7 @@ func (e *AmbiguousImportError) Error() string {
 // for its .zip file.
 type ImportMissingSumError struct {
 	importPath   string
+	modPaths     []string
 	found, inAll bool
 }
 
@@ -145,7 +146,7 @@ func (e *ImportMissingSumError) Error() string {
 		message = fmt.Sprintf("missing go.sum entry for module providing package %s", e.importPath)
 	}
 	if e.inAll {
-		return message + "; to add it:\n\tgo mod tidy"
+		return message + fmt.Sprintf("; to add it:\n\tgo mod download %s", strings.Join(e.modPaths, " "))
 	}
 	return message
 }
@@ -238,7 +239,7 @@ func importFromBuildList(ctx context.Context, path string, buildList []module.Ve
 	// Check each module on the build list.
 	var dirs []string
 	var mods []module.Version
-	haveSumErr := false
+	var sumErrModPaths []string
 	for _, m := range buildList {
 		if !maybeInModule(path, m.Path) {
 			// Avoid possibly downloading irrelevant modules.
@@ -251,8 +252,9 @@ func importFromBuildList(ctx context.Context, path string, buildList []module.Ve
 				// We are missing a sum needed to fetch a module in the build list.
 				// We can't verify that the package is unique, and we may not find
 				// the package at all. Keep checking other modules to decide which
-				// error to report.
-				haveSumErr = true
+				// error to report. Multiple sums may be missing if we need to look in
+				// multiple nested modules to resolve the import; we'll report them all.
+				sumErrModPaths = append(sumErrModPaths, m.Path)
 				continue
 			}
 			// Report fetch error.
@@ -273,8 +275,8 @@ func importFromBuildList(ctx context.Context, path string, buildList []module.Ve
 	if len(mods) > 1 {
 		return module.Version{}, "", &AmbiguousImportError{importPath: path, Dirs: dirs, Modules: mods}
 	}
-	if haveSumErr {
-		return module.Version{}, "", &ImportMissingSumError{importPath: path, found: len(mods) > 0}
+	if len(sumErrModPaths) > 0 {
+		return module.Version{}, "", &ImportMissingSumError{importPath: path, modPaths: sumErrModPaths, found: len(mods) > 0}
 	}
 	if len(mods) == 1 {
 		return mods[0], dirs[0], nil
diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go
index 1a51c58bf29..bc8d17e0a5f 100644
--- a/src/cmd/go/internal/modload/init.go
+++ b/src/cmd/go/internal/modload/init.go
@@ -380,7 +380,7 @@ func LoadModFile(ctx context.Context) {
 
 	if f.Module == nil {
 		// No module declaration. Must add module path.
-		base.Fatalf("go: no module declaration in go.mod.\n\tRun 'go mod edit -module=example.com/mod' to specify the module path.")
+		base.Fatalf("go: no module declaration in go.mod. To specify the module path:\n\tgo mod edit -module=example.com/mod")
 	}
 
 	if err := checkModulePathLax(f.Module.Mod.Path); err != nil {
diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index ae5b8ef6abd..cd36da6a87d 100644
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -868,7 +868,7 @@ func loadFromRoots(params loaderParams) *loader {
 						// base.Errorf. Ideally, 'go list' should not fail because of this,
 						// but today, LoadPackages calls WriteGoMod unconditionally, which
 						// would fail with a less clear message.
-						base.Errorf("go: %[1]s: package %[2]s imported from implicitly required module; try 'go get -d %[1]s' to add missing requirements", pkg.path, dep.path)
+						base.Errorf("go: %[1]s: package %[2]s imported from implicitly required module; to add missing requirements, run:\n\tgo get %[2]s@%[3]s", pkg.path, dep.path, dep.mod.Version)
 					}
 					ld.direct[dep.mod.Path] = true
 				}
diff --git a/src/cmd/go/internal/modload/vendor.go b/src/cmd/go/internal/modload/vendor.go
index 80d49053c6c..d8fd91f1fea 100644
--- a/src/cmd/go/internal/modload/vendor.go
+++ b/src/cmd/go/internal/modload/vendor.go
@@ -214,6 +214,6 @@ func checkVendorConsistency() {
 	}
 
 	if vendErrors.Len() > 0 {
-		base.Fatalf("go: inconsistent vendoring in %s:%s\n\nrun 'go mod vendor' to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory", modRoot, vendErrors)
+		base.Fatalf("go: inconsistent vendoring in %s:%s\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor", modRoot, vendErrors)
 	}
 }
diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go
index d2671ff5a79..10e6604da5f 100644
--- a/src/cmd/go/internal/test/testflag.go
+++ b/src/cmd/go/internal/test/testflag.go
@@ -325,7 +325,7 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 		if !testC {
 			buildFlag = "-i"
 		}
-		fmt.Fprintf(os.Stderr, "flag %s is not a 'go test' flag (unknown flags cannot be used with %s)\n", firstUnknownFlag, buildFlag)
+		fmt.Fprintf(os.Stderr, "go test: unknown flag %s cannot be used with %s\n", firstUnknownFlag, buildFlag)
 		exitWithUsage()
 	}
 
diff --git a/src/cmd/go/testdata/script/mod_get_promote_implicit.txt b/src/cmd/go/testdata/script/mod_get_promote_implicit.txt
index c64e0c0f706..10ca6594e4c 100644
--- a/src/cmd/go/testdata/script/mod_get_promote_implicit.txt
+++ b/src/cmd/go/testdata/script/mod_get_promote_implicit.txt
@@ -6,10 +6,12 @@ cp go.mod.orig go.mod
 go list -m indirect-with-pkg
 stdout '^indirect-with-pkg v1.0.0 => ./indirect-with-pkg$'
 ! go list ./use-indirect
-stderr '^go: m/use-indirect: package indirect-with-pkg imported from implicitly required module; try ''go get -d m/use-indirect'' to add missing requirements$'
+stderr '^go: m/use-indirect: package indirect-with-pkg imported from implicitly required module; to add missing requirements, run:\n\tgo get indirect-with-pkg@v1.0.0$'
 
-# We can promote the implicit requirement by getting the importing package,
-# as hinted.
+# We can promote the implicit requirement by getting the importing package.
+# NOTE: the hint recommends getting the imported package (tested below) since
+# it's more obvious and doesn't require -d. However, that adds an '// indirect'
+# comment on the requirement.
 go get -d m/use-indirect
 cmp go.mod go.mod.use
 cp go.mod.orig go.mod
@@ -17,6 +19,8 @@ cp go.mod.orig go.mod
 # We can also promote implicit requirements using 'go get' on them, or their
 # packages. This gives us "// indirect" requirements, since 'go get' doesn't
 # know they're needed by the main module. See #43131 for the rationale.
+# The hint above recommends this because it's more obvious usage and doesn't
+# require the -d flag.
 go get -d indirect-with-pkg indirect-without-pkg
 cmp go.mod go.mod.indirect
 
diff --git a/src/cmd/go/testdata/script/mod_get_retract.txt b/src/cmd/go/testdata/script/mod_get_retract.txt
index 6e328eb5929..fe0ac886295 100644
--- a/src/cmd/go/testdata/script/mod_get_retract.txt
+++ b/src/cmd/go/testdata/script/mod_get_retract.txt
@@ -11,7 +11,7 @@ cp go.mod.orig go.mod
 go mod edit -require example.com/retract/self/prev@v1.9.0
 go get -d example.com/retract/self/prev
 stderr '^go: warning: example.com/retract/self/prev@v1.9.0: retracted by module author: self$'
-stderr '^go: run ''go get example.com/retract/self/prev@latest'' to switch to the latest unretracted version$'
+stderr '^go: to switch to the latest unretracted version, run:\n\tgo get example.com/retract/self/prev@latest$'
 go list -m example.com/retract/self/prev
 stdout '^example.com/retract/self/prev v1.9.0$'
 
diff --git a/src/cmd/go/testdata/script/mod_invalid_path.txt b/src/cmd/go/testdata/script/mod_invalid_path.txt
index 05a51335717..667828839fd 100644
--- a/src/cmd/go/testdata/script/mod_invalid_path.txt
+++ b/src/cmd/go/testdata/script/mod_invalid_path.txt
@@ -3,7 +3,7 @@
 # Test that go list fails on a go.mod with no module declaration.
 cd $WORK/gopath/src/mod
 ! go list .
-stderr '^go: no module declaration in go.mod.\n\tRun ''go mod edit -module=example.com/mod'' to specify the module path.$'
+stderr '^go: no module declaration in go.mod. To specify the module path:\n\tgo mod edit -module=example.com/mod$'
 
 # Test that go mod init in GOPATH doesn't add a module declaration
 # with a path that can't possibly be a module path, because
diff --git a/src/cmd/go/testdata/script/mod_sum_ambiguous.txt b/src/cmd/go/testdata/script/mod_sum_ambiguous.txt
index 209367181d4..5344dc00297 100644
--- a/src/cmd/go/testdata/script/mod_sum_ambiguous.txt
+++ b/src/cmd/go/testdata/script/mod_sum_ambiguous.txt
@@ -25,13 +25,17 @@ cp go.sum.a-only go.sum
 ! go list example.com/ambiguous/a/b
 stderr '^missing go.sum entry needed to verify package example.com/ambiguous/a/b is provided by exactly one module$'
 ! go list -deps .
-stderr '^use.go:3:8: missing go.sum entry needed to verify package example.com/ambiguous/a/b is provided by exactly one module; to add it:\n\tgo mod tidy$'
+stderr '^use.go:3:8: missing go.sum entry needed to verify package example.com/ambiguous/a/b is provided by exactly one module; to add it:\n\tgo mod download example.com/ambiguous/a/b$'
 
 cp go.sum.b-only go.sum
 ! go list example.com/ambiguous/a/b
 stderr '^missing go.sum entry for module providing package example.com/ambiguous/a/b$'
 ! go list -deps .
-stderr '^use.go:3:8: missing go.sum entry for module providing package example.com/ambiguous/a/b; to add it:\n\tgo mod tidy$'
+stderr '^use.go:3:8: missing go.sum entry for module providing package example.com/ambiguous/a/b; to add it:\n\tgo mod download example.com/ambiguous/a$'
+
+cp go.sum.buildlist-only go.sum
+! go list -deps .
+stderr '^use.go:3:8: missing go.sum entry for module providing package example.com/ambiguous/a/b; to add it:\n\tgo mod download example.com/ambiguous/a example.com/ambiguous/a/b$'
 
 -- go.mod --
 module m
diff --git a/src/cmd/go/testdata/script/mod_sum_readonly.txt b/src/cmd/go/testdata/script/mod_sum_readonly.txt
index 866f4c1ae45..00b4d7b5d2e 100644
--- a/src/cmd/go/testdata/script/mod_sum_readonly.txt
+++ b/src/cmd/go/testdata/script/mod_sum_readonly.txt
@@ -47,7 +47,7 @@ stderr '^missing go.sum entry for module providing package rsc.io/quote$'
 # a package that imports it without that error.
 go list -e -deps -f '{{.ImportPath}}{{with .Error}} {{.Err}}{{end}}' .
 stdout '^m$'
-stdout '^rsc.io/quote missing go.sum entry for module providing package rsc.io/quote; to add it:\n\tgo mod tidy$'
+stdout '^rsc.io/quote missing go.sum entry for module providing package rsc.io/quote; to add it:\n\tgo mod download rsc.io/quote$'
 ! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.zip
 
 # go.sum should not have been written.
diff --git a/src/cmd/go/testdata/script/mod_vendor_auto.txt b/src/cmd/go/testdata/script/mod_vendor_auto.txt
index 1b362eda0bd..b0ea907206a 100644
--- a/src/cmd/go/testdata/script/mod_vendor_auto.txt
+++ b/src/cmd/go/testdata/script/mod_vendor_auto.txt
@@ -66,7 +66,7 @@ stderr '^go: inconsistent vendoring in '$WORK[/\\]auto':$'
 stderr '^\texample.com/printversion@v1.0.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt'
 stderr '^\texample.com/unused: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
 stderr '^\texample.com/version@v1.2.0: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
-stderr '\n\nrun .go mod vendor. to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory$'
+stderr '^\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor$'
 
 # Module-specific subcommands should continue to load the full module graph.
 go mod graph
@@ -135,7 +135,7 @@ stderr '^go: inconsistent vendoring in '$WORK[/\\]auto':$'
 stderr '^\texample.com/printversion@v1.0.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt'
 stderr '^\texample.com/unused: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
 stderr '^\texample.com/version@v1.2.0: is replaced in go.mod, but not marked as replaced in vendor/modules.txt'
-stderr '\n\nrun .go mod vendor. to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory$'
+stderr '^\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor$'
 
 # If -mod=vendor is set, limited consistency checks should apply even when
 # the go version is 1.13 or earlier.
@@ -151,7 +151,7 @@ cp $WORK/modules-bad-1.13.txt vendor/modules.txt
 ! go list -mod=vendor -f {{.Dir}} -tags tools all
 stderr '^go: inconsistent vendoring in '$WORK[/\\]auto':$'
 stderr '^\texample.com/printversion@v1.0.0: is explicitly required in go.mod, but vendor/modules.txt indicates example.com/printversion@v1.1.0$'
-stderr '\n\nrun .go mod vendor. to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory$'
+stderr '^\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor$'
 
 # If the go version is still 1.13, 'go mod vendor' should write a
 # matching vendor/modules.txt containing the corrected 1.13 data.
diff --git a/src/cmd/go/testdata/script/mod_versions.txt b/src/cmd/go/testdata/script/mod_versions.txt
index fd5e5c589d2..9e6322bae11 100644
--- a/src/cmd/go/testdata/script/mod_versions.txt
+++ b/src/cmd/go/testdata/script/mod_versions.txt
@@ -1,14 +1,14 @@
 # Test rejection of pkg@version in GOPATH mode.
 env GO111MODULE=off
 ! go get rsc.io/quote@v1.5.1
-stderr 'cannot use path@version syntax in GOPATH mode'
+stderr '^go: can only use path@version syntax with ''go get'' and ''go install'' in module-aware mode$'
 ! go build rsc.io/quote@v1.5.1
-stderr 'cannot use path@version syntax in GOPATH mode'
+stderr '^package rsc.io/quote@v1.5.1: can only use path@version syntax with ''go get'' and ''go install'' in module-aware mode$'
 
 env GO111MODULE=on
 cd x
 ! go build rsc.io/quote@v1.5.1
-stderr 'can only use path@version syntax with ''go get'''
+stderr '^package rsc.io/quote@v1.5.1: can only use path@version syntax with ''go get'' and ''go install'' in module-aware mode$'
 
 -- x/go.mod --
 module x
diff --git a/src/cmd/go/testdata/script/test_flag.txt b/src/cmd/go/testdata/script/test_flag.txt
index ec88d38cbe8..0142b3f308f 100644
--- a/src/cmd/go/testdata/script/test_flag.txt
+++ b/src/cmd/go/testdata/script/test_flag.txt
@@ -9,13 +9,13 @@ go test -count=1 -custom -args -v=7
 # However, it should be an error to use custom flags when -i or -c are used,
 # since we know for sure that no test binary will run at all.
 ! go test -i -custom
-stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -i\)$'
+stderr '^go test: unknown flag -custom cannot be used with -i$'
 ! go test -c -custom
-stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -c\)$'
+stderr '^go test: unknown flag -custom cannot be used with -c$'
 
 # The same should apply even if -c or -i come after a custom flag.
 ! go test -custom -c
-stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -c\)$'
+stderr '^go test: unknown flag -custom cannot be used with -c$'
 
 -- go.mod --
 module m