cmd/go/internal/modload: return a module-only result from QueryPattern

This allows a single QueryPattern call to resolve a path that could be
either a package or a module. It is important to be able to make a
single QueryPattern call — rather than a QueryPattern followed by a
Query for the specific module path — to provide appropriate fallback
behavior: if the proxy returns package results but does not contain a
module result, we don't want to fall back to the next proxy to look
for the (probably-nonexistent) module.

For #37438

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

Author: Bryan C. Mills

src/cmd/go/internal/modget/get.go

@@ -912,22 +912,15 @@ func getQuery(ctx context.Context, path, vers string, prevM module.Version, forc
 	// If it turns out to only exist as a module, we can detect the resulting
 	// PackageNotInModuleError and avoid a second round-trip through (potentially)
 	// all of the configured proxies.
-	results, err := modload.QueryPattern(ctx, path, vers, modload.Selected, allowed)
+	results, modOnly, err := modload.QueryPattern(ctx, path, vers, modload.Selected, allowed)
 	if err != nil {
-		// If the path doesn't contain a wildcard, check whether it was actually a
-		// module path instead. If so, return that.
-		if !strings.Contains(path, "...") {
-			var modErr *modload.PackageNotInModuleError
-			if errors.As(err, &modErr) && modErr.Mod.Path == path {
-				if modErr.Mod.Version != vers {
-					logOncef("go: %s %s => %s", path, vers, modErr.Mod.Version)
-				}
-				return modErr.Mod, nil
-			}
-		}
-
 		return module.Version{}, err
 	}
+	if len(results) == 0 {
+		// The path doesn't contain a wildcard, but was actually a
+		// module path instead. Return that.
+		return modOnly.Mod, nil
+	}
 
 	m := results[0].Mod
 	if m.Path != path {

src/cmd/go/internal/modload/import.go

@@ -383,7 +383,7 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
 	// and return m, dir, ImpportMissingError.
 	fmt.Fprintf(os.Stderr, "go: finding module for package %s\n", path)
 
-	candidates, err := QueryPattern(ctx, path, "latest", Selected, CheckAllowed)
+	candidates, err := QueryPackages(ctx, path, "latest", Selected, CheckAllowed)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
 			// Return "cannot find module providing package […]" instead of whatever

src/cmd/go/internal/modload/query.go

@@ -504,20 +504,39 @@ type QueryResult struct {
 	Packages []string
 }
 
+// QueryPackages is like QueryPattern, but requires that the pattern match at
+// least one package and omits the non-package result (if any).
+func QueryPackages(ctx context.Context, pattern, query string, current func(string) string, allowed AllowedFunc) ([]QueryResult, error) {
+	pkgMods, modOnly, err := QueryPattern(ctx, pattern, query, current, allowed)
+
+	if len(pkgMods) == 0 && err == nil {
+		return nil, &PackageNotInModuleError{
+			Mod:         modOnly.Mod,
+			Replacement: Replacement(modOnly.Mod),
+			Query:       query,
+			Pattern:     pattern,
+		}
+	}
+
+	return pkgMods, err
+}
+
 // QueryPattern looks up the module(s) containing at least one package matching
 // the given pattern at the given version. The results are sorted by module path
-// length in descending order.
+// length in descending order. If any proxy provides a non-empty set of candidate
+// modules, no further proxies are tried.
 //
-// QueryPattern queries modules with package paths up to the first "..."
-// in the pattern. For the pattern "example.com/a/b.../c", QueryPattern would
-// consider prefixes of "example.com/a". If multiple modules have versions
-// that match the query and packages that match the pattern, QueryPattern
-// picks the one with the longest module path.
+// For wildcard patterns, QueryPattern looks in modules with package paths up to
+// the first "..." in the pattern. For the pattern "example.com/a/b.../c",
+// QueryPattern would consider prefixes of "example.com/a".
 //
 // If any matching package is in the main module, QueryPattern considers only
 // the main module and only the version "latest", without checking for other
 // possible modules.
-func QueryPattern(ctx context.Context, pattern, query string, current func(string) string, allowed AllowedFunc) ([]QueryResult, error) {
+//
+// QueryPattern always returns at least one QueryResult (which may be only
+// modOnly) or a non-nil error.
+func QueryPattern(ctx context.Context, pattern, query string, current func(string) string, allowed AllowedFunc) (pkgMods []QueryResult, modOnly *QueryResult, err error) {
 	ctx, span := trace.StartSpan(ctx, "modload.QueryPattern "+pattern+" "+query)
 	defer span.Done()
 
@@ -531,6 +550,7 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 	}
 
 	var match func(mod module.Version, root string, isLocal bool) *search.Match
+	matchPattern := search.MatchPattern(pattern)
 
 	if i := strings.Index(pattern, "..."); i >= 0 {
 		base = pathpkg.Dir(pattern[:i+3])
@@ -558,20 +578,29 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 	if HasModRoot() {
 		m := match(Target, modRoot, true)
 		if len(m.Pkgs) > 0 {
-			if query != "latest" {
-				return nil, fmt.Errorf("can't query specific version for package %s in the main module (%s)", pattern, Target.Path)
+			if query != "latest" && query != "upgrade" && query != "patch" {
+				return nil, nil, fmt.Errorf("can't query version %q for package %s in the main module (%s)", query, pattern, Target.Path)
 			}
 			if err := allowed(ctx, Target); err != nil {
-				return nil, fmt.Errorf("internal error: package %s is in the main module (%s), but version is not allowed: %w", pattern, Target.Path, err)
+				return nil, nil, fmt.Errorf("internal error: package %s is in the main module (%s), but version is not allowed: %w", pattern, Target.Path, err)
 			}
 			return []QueryResult{{
 				Mod:      Target,
 				Rev:      &modfetch.RevInfo{Version: Target.Version},
 				Packages: m.Pkgs,
-			}}, nil
+			}}, nil, nil
 		}
 		if err := firstError(m); err != nil {
-			return nil, err
+			return nil, nil, err
+		}
+
+		if query != "latest" && query != "upgrade" && query != "patch" && matchPattern(Target.Path) {
+			if err := allowed(ctx, Target); err == nil {
+				modOnly = &QueryResult{
+					Mod: Target,
+					Rev: &modfetch.RevInfo{Version: Target.Version},
+				}
+			}
 		}
 	}
 
@@ -580,14 +609,17 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 		candidateModules = modulePrefixesExcludingTarget(base)
 	)
 	if len(candidateModules) == 0 {
-		return nil, &PackageNotInModuleError{
-			Mod:     Target,
-			Query:   query,
-			Pattern: pattern,
+		if modOnly == nil {
+			return nil, nil, &PackageNotInModuleError{
+				Mod:     Target,
+				Query:   query,
+				Pattern: pattern,
+			}
 		}
+		return nil, modOnly, nil
 	}
 
-	err := modfetch.TryProxies(func(proxy string) error {
+	err = modfetch.TryProxies(func(proxy string) error {
 		queryModule := func(ctx context.Context, path string) (r QueryResult, err error) {
 			ctx, span := trace.StartSpan(ctx, "modload.QueryPattern.queryModule ["+proxy+"] "+path)
 			defer span.Done()
@@ -606,7 +638,7 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 			}
 			m := match(r.Mod, root, isLocal)
 			r.Packages = m.Pkgs
-			if len(r.Packages) == 0 {
+			if len(r.Packages) == 0 && !matchPattern(path) {
 				if err := firstError(m); err != nil {
 					return r, err
 				}
@@ -620,12 +652,19 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
 			return r, nil
 		}
 
-		var err error
-		results, err = queryPrefixModules(ctx, candidateModules, queryModule)
+		allResults, err := queryPrefixModules(ctx, candidateModules, queryModule)
+		results = allResults[:0]
+		for _, r := range allResults {
+			if len(r.Packages) == 0 {
+				modOnly = &r
+			} else {
+				results = append(results, r)
+			}
+		}
 		return err
 	})
 
-	return results, err
+	return results[:len(results):len(results)], modOnly, err
 }
 
 // modulePrefixesExcludingTarget returns all prefixes of path that may plausibly
@@ -651,11 +690,6 @@ func modulePrefixesExcludingTarget(path string) []string {
 	return prefixes
 }
 
-type prefixResult struct {
-	QueryResult
-	err error
-}
-
 func queryPrefixModules(ctx context.Context, candidateModules []string, queryModule func(ctx context.Context, path string) (QueryResult, error)) (found []QueryResult, err error) {
 	ctx, span := trace.StartSpan(ctx, "modload.queryPrefixModules")
 	defer span.Done()

src/cmd/go/internal/work/build.go

@@ -741,7 +741,7 @@ func installOutsideModule(ctx context.Context, args []string) {
 		// Don't check for retractions if a specific revision is requested.
 		allowed = nil
 	}
-	qrs, err := modload.QueryPattern(ctx, patterns[0], version, modload.Selected, allowed)
+	qrs, err := modload.QueryPackages(ctx, patterns[0], version, modload.Selected, allowed)
 	if err != nil {
 		base.Fatalf("go install %s: %v", args[0], err)
 	}

src/cmd/go/testdata/script/mod_get_downgrade_missing.txt

@@ -3,14 +3,24 @@ cp go.mod go.mod.orig
 # getting a specific version of a module along with a pattern
 # not yet present in that module should report the version mismatch
 # rather than a "matched no packages" warning.
+
 ! go get example.net/pkgadded@v1.1.0 example.net/pkgadded/subpkg/...
 stderr '^go get: conflicting versions for module example\.net/pkgadded: v1\.1\.0 and v1\.2\.0$'
 ! stderr 'matched no packages'
 cmp go.mod.orig go.mod
 
-! go get example.net/pkgadded/...@v1.0.0
-stderr '^go get example\.net/pkgadded/\.\.\.@v1\.0\.0: module example\.net/pkgadded@v1\.0\.0 found, but does not contain packages matching example\.net/pkgadded/\.\.\.$'
-cmp go.mod.orig go.mod
+
+# A wildcard pattern should match a package in a module with that path.
+
+go get example.net/pkgadded/...@v1.0.0
+go list -m all
+stdout '^example.net/pkgadded v1.0.0'
+cp go.mod.orig go.mod
+
+
+# If we need to resolve a transitive dependency of a package,
+# and another argument constrains away the version that provides that
+# package, then 'go get' should fail with a useful error message.
 
 ! go get example.net/pkgadded@v1.0.0 .
 stderr -count=1 '^go: found example.net/pkgadded/subpkg in example.net/pkgadded v1\.2\.0$'  # TODO: We shouldn't even try v1.2.0.