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>

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
 		}

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},
 				},
 			}
 		}

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
+}

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)
 		}

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)
 	}
 }