cmd/go: refactor modload.Import for better -mod=readonly errors

When -mod=readonly is set, Import will now allow imports from
replacements without explicit requirements. With -mod=mod, this would
add a new requirement but does not trigger a module lookup, so it's
determinisitic.

Before reporting an error for an unknown import with -mod=readonly,
check whether the import is valid. If there's a typo in the import,
that's more relevant.

For #40728

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

diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go
index c625184b8b831a4938d21e8131cde99df537d60a..10b1e7f4b84a270f054f42fed77f0218136e1969 100644 (file)
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -107,6 +107,25 @@ func (e *AmbiguousImportError) Error() string {
 
 var _ load.ImportPathError = &AmbiguousImportError{}
 
+type invalidImportError struct {
+       importPath string
+       err        error
+}
+
+func (e *invalidImportError) ImportPath() string {
+       return e.importPath
+}
+
+func (e *invalidImportError) Error() string {
+       return e.err.Error()
+}
+
+func (e *invalidImportError) Unwrap() error {
+       return e.err
+}
+
+var _ load.ImportPathError = &invalidImportError{}
+
 // importFromBuildList finds the module and directory in the build list
 // containing the package with the given import path. The answer must be unique:
 // importFromBuildList returns an error if multiple modules attempt to provide
@@ -207,17 +226,6 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di
 func queryImport(ctx context.Context, path string) (module.Version, error) {
        pathIsStd := search.IsStandardImportPath(path)
 
-       if cfg.BuildMod == "readonly" {
-               var queryErr error
-               if !pathIsStd {
-                       if cfg.BuildModReason == "" {
-                               queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
-                       } else {
-                               queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason)
-                       }
-               }
-               return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr}
-       }
        if modRoot == "" && !allowMissingModuleImports {
                return module.Version{}, &ImportMissingError{
                        Path:     path,
@@ -226,8 +234,9 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
        }
 
        // Not on build list.
-       // To avoid spurious remote fetches, next try the latest replacement for each module.
-       // (golang.org/issue/26241)
+       // To avoid spurious remote fetches, next try the latest replacement for each
+       // module (golang.org/issue/26241). This should give a useful message
+       // in -mod=readonly, and it will allow us to add a requirement with -mod=mod.
        if modFile != nil {
                latest := map[string]string{} // path -> version
                for _, r := range modFile.Replace {
@@ -288,6 +297,11 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
                }
        }
 
+       // Before any further lookup, check that the path is valid.
+       if err := module.CheckImportPath(path); err != nil {
+               return module.Version{}, &invalidImportError{importPath: path, err: err}
+       }
+
        if pathIsStd {
                // This package isn't in the standard library, isn't in any module already
                // in the build list, and isn't in any other module that the user has
@@ -299,6 +313,19 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
                return module.Version{}, &ImportMissingError{Path: path}
        }
 
+       if cfg.BuildMod == "readonly" {
+               var queryErr error
+               if cfg.BuildModExplicit {
+                       queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
+               } else if cfg.BuildModReason != "" {
+                       queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason)
+               }
+               return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr}
+       }
+
+       // Look up module containing the package, for addition to the build list.
+       // Goal is to determine the module, download it to dir,
+       // and return m, dir, ImpportMissingError.
        fmt.Fprintf(os.Stderr, "go: finding module for package %s\n", path)
 
        candidates, err := QueryPackage(ctx, path, "latest", CheckAllowed)

diff --git a/src/cmd/go/testdata/script/mod_build_info_err.txt b/src/cmd/go/testdata/script/mod_build_info_err.txt
index 87a099b219c2356f62aa1e6d88e479e398340365..a6853b5c8691c715e3143b986b2cde67f36716c6 100644 (file)
--- a/src/cmd/go/testdata/script/mod_build_info_err.txt
+++ b/src/cmd/go/testdata/script/mod_build_info_err.txt
@@ -2,7 +2,7 @@
 # Verifies golang.org/issue/34393.
 
 go list -e -deps -f '{{with .Error}}{{.Pos}}: {{.Err}}{{end}}' ./main
-stdout 'bad[/\\]bad.go:3:8: malformed module path "🐧.example.com/string": invalid char ''🐧'''
+stdout 'bad[/\\]bad.go:3:8: malformed import path "🐧.example.com/string": invalid char ''🐧'''
 
 -- go.mod --
 module m