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>

diff --git a/src/cmd/go/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go
index d73d1146f0f3ed961400c4ca7325bf9f2372e998..2e4d6388cfe790b4f4337a04febe9eef125dda99 100644 (file)
--- 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 849e8c7ca1f8e1c8023edefa44f572853bd26a0d..de757ecd27bc19c8a7c2cf38afa90dd6fc25a3e0 100644 (file)
--- 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 4273da0317679a3dd9c44a23a3ba86c0c15312fa..f03bdd8d038a436f6eee89e9acfbaa5aa6a604f7 100644 (file)
--- 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 a01ef62d550f2cb92f190a5e50ce06f9719e3799..dc0fc3c4d0b8774f5bdee7ad3c5fa96efc59a235 100644 (file)
--- 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 80454871741cdb77cbb2a8f48c8e162356e61e03..b5c813b3e2881840b09edab0004e8772d5928d17 100644 (file)
--- 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)
        }
 }