cmd/go: reduce redundancy in direct mode lookup error messages

get.RepoRootForImportPath now returns errors that satisfy
load.ImportPathError in cases where the import path appears in the
messages. (The import path probably should appear in all errors from
this function, but this CL does not change these errors).

Changed modfetch.notExistError to be a wrapper (with an Unwrap method)
instead of a string. This means errors.As works with notFoundError and
ImportPathError.

ImportMissingError no longer prints the package path if it wraps an
ImportPathError.

TestMissingImportErrorRepetition no longer counts the package path
within a URL (like https://...?go-get=1).

Fixes #35986

Change-Id: I38f795191c46d04b542c553e705f23822260c790
Reviewed-on: https://go-review.googlesource.com/c/go/+/210338
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

5 files changed, 34 insertions, 12 deletions

diff --git a/src/cmd/go/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go
index d73d1146f0..2e4d6388cf 100644
--- a/src/cmd/go/internal/get/vcs.go
+++ b/src/cmd/go/internal/get/vcs.go
@@ -21,6 +21,7 @@ import (
 
 	"cmd/go/internal/base"
 	"cmd/go/internal/cfg"
+	"cmd/go/internal/load"
 	"cmd/go/internal/web"
 )
 
@@ -661,7 +662,7 @@ func RepoRootForImportPath(importPath string, mod ModuleMode, security web.Secur
 	if err == errUnknownSite {
 		rr, err = repoRootForImportDynamic(importPath, mod, security)
 		if err != nil {
-			err = fmt.Errorf("unrecognized import path %q: %v", importPath, err)
+			err = load.ImportErrorf(importPath, "unrecognized import path %q: %v", importPath, err)
 		}
 	}
 	if err != nil {
@@ -676,7 +677,7 @@ func RepoRootForImportPath(importPath string, mod ModuleMode, security web.Secur
 	if err == nil && strings.Contains(importPath, "...") && strings.Contains(rr.Root, "...") {
 		// Do not allow wildcards in the repo root.
 		rr = nil
-		err = fmt.Errorf("cannot expand ... in %q", importPath)
+		err = load.ImportErrorf(importPath, "cannot expand ... in %q", importPath)
 	}
 	return rr, err
 }
@@ -700,7 +701,7 @@ func repoRootFromVCSPaths(importPath string, security web.SecurityMode, vcsPaths
 		m := srv.regexp.FindStringSubmatch(importPath)
 		if m == nil {
 			if srv.prefix != "" {
-				return nil, fmt.Errorf("invalid %s import path %q", srv.prefix, importPath)
+				return nil, load.ImportErrorf(importPath, "invalid %s import path %q", srv.prefix, importPath)
 			}
 			continue
 		}
diff --git a/src/cmd/go/internal/modfetch/coderepo.go b/src/cmd/go/internal/modfetch/coderepo.go
index 849e8c7ca1..de757ecd27 100644
--- a/src/cmd/go/internal/modfetch/coderepo.go
+++ b/src/cmd/go/internal/modfetch/coderepo.go
@@ -359,7 +359,7 @@ func (r *codeRepo) convert(info *codehost.RevInfo, statVers string) (*RevInfo, e
 				Path: r.modPath,
 				Err: &module.InvalidVersionError{
 					Version: info2.Version,
-					Err:     notExistError(err.Error()),
+					Err:     notExistError{err: err},
 				},
 			}
 		}
diff --git a/src/cmd/go/internal/modfetch/repo.go b/src/cmd/go/internal/modfetch/repo.go
index 4273da0317..f03bdd8d03 100644
--- a/src/cmd/go/internal/modfetch/repo.go
+++ b/src/cmd/go/internal/modfetch/repo.go
@@ -250,9 +250,9 @@ func (lookupDisabledError) Error() string {
 var errLookupDisabled error = lookupDisabledError{}
 
 var (
-	errProxyOff       = notExistError("module lookup disabled by GOPROXY=off")
-	errNoproxy  error = notExistError("disabled by GOPRIVATE/GONOPROXY")
-	errUseProxy error = notExistError("path does not match GOPRIVATE/GONOPROXY")
+	errProxyOff       = notExistErrorf("module lookup disabled by GOPROXY=off")
+	errNoproxy  error = notExistErrorf("disabled by GOPRIVATE/GONOPROXY")
+	errUseProxy error = notExistErrorf("path does not match GOPRIVATE/GONOPROXY")
 )
 
 func lookupDirect(path string) (Repo, error) {
@@ -264,7 +264,7 @@ func lookupDirect(path string) (Repo, error) {
 	rr, err := get.RepoRootForImportPath(path, get.PreferMod, security)
 	if err != nil {
 		// We don't know where to find code for a module with this path.
-		return nil, notExistError(err.Error())
+		return nil, notExistError{err: err}
 	}
 
 	if rr.VCS == "mod" {
@@ -408,11 +408,22 @@ func (l *loggingRepo) Zip(dst io.Writer, version string) error {
 }
 
 // A notExistError is like os.ErrNotExist, but with a custom message
-type notExistError string
+type notExistError struct {
+	err error
+}
+
+func notExistErrorf(format string, args ...interface{}) error {
+	return notExistError{fmt.Errorf(format, args...)}
+}
 
 func (e notExistError) Error() string {
-	return string(e)
+	return e.err.Error()
 }
+
 func (notExistError) Is(target error) bool {
 	return target == os.ErrNotExist
 }
+
+func (e notExistError) Unwrap() error {
+	return e.err
+}
diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go
index a01ef62d55..dc0fc3c4d0 100644
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -43,6 +43,9 @@ func (e *ImportMissingError) Error() string {
 		if str.HasPathPrefix(e.Path, "cmd") {
 			return fmt.Sprintf("package %s is not in GOROOT (%s)", e.Path, filepath.Join(cfg.GOROOT, "src", e.Path))
 		}
+		if i := load.ImportPathError(nil); errors.As(e.QueryErr, &i) {
+			return fmt.Sprintf("cannot find module: %v", e.QueryErr)
+		}
 		if e.QueryErr != nil {
 			return fmt.Sprintf("cannot find module providing package %s: %v", e.Path, e.QueryErr)
 		}
diff --git a/src/go/build/build_test.go b/src/go/build/build_test.go
index 8045487174..b5c813b3e2 100644
--- a/src/go/build/build_test.go
+++ b/src/go/build/build_test.go
@@ -515,14 +515,21 @@ func TestMissingImportErrorRepetition(t *testing.T) {
 	os.Setenv("GO111MODULE", "on")
 	defer os.Setenv("GOPROXY", os.Getenv("GOPROXY"))
 	os.Setenv("GOPROXY", "off")
+	defer os.Setenv("GONOPROXY", os.Getenv("GONOPROXY"))
+	os.Setenv("GONOPROXY", "none")
 
 	ctxt := Default
 	ctxt.WorkingDir = tmp
 
 	pkgPath := "example.com/hello"
-	if _, err = ctxt.Import(pkgPath, tmp, FindOnly); err == nil {
+	_, err = ctxt.Import(pkgPath, tmp, FindOnly)
+	if err == nil {
 		t.Fatal("unexpected success")
-	} else if n := strings.Count(err.Error(), pkgPath); n != 1 {
+	}
+	// Don't count the package path with a URL like https://...?go-get=1.
+	// See golang.org/issue/35986.
+	errStr := strings.ReplaceAll(err.Error(), "://"+pkgPath+"?go-get=1", "://...?go-get=1")
+	if n := strings.Count(errStr, pkgPath); n != 1 {
 		t.Fatalf("package path %q appears in error %d times; should appear once\nerror: %v", pkgPath, n, err)
 	}
 }