cmd/go: make 'go get' match patterns against packages, not modules

This is a follow-up to CL 174099, fixing an important TODO.
The 'go help modget' documentation will be clarified in anotehr CL,
pending further discussion.

When invoked without -m, 'go get' will no longer match arguments
containing "..." against module paths. If a module's path matches a
pattern but no packages within the module match the pattern, the
module should not be upgraded. For example, if
golang.org/x/tools/playground and golang.org/x/tools are separate
modules, and only golang.org/x/tools is in the build list,
'go get golang.org/x/tools/playground/...' should add
golang.org/x/tools/playground to the build list and leave
golang.org/x/tools alone.

Updates #26902

Change-Id: I2bd18c7950db1aa7bd8527210c1baf2c7d174375
Reviewed-on: https://go-review.googlesource.com/c/go/+/176578
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

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

@@ -336,37 +336,32 @@ func runGet(cmd *base.Command, args []string) {
 			}
 
 		case strings.Contains(path, "..."):
-			// Find modules in the build list matching the pattern, if any.
-			match := search.MatchPattern(path)
-			matched := false
-			for _, m := range modload.BuildList() {
-				// TODO: If we have matching packages already in the build list and we
-				// know which module(s) they are in, then we should not upgrade the
-				// modules that do *not* contain those packages, even if the module path
-				// is a prefix of the pattern.
-				//
-				// For example, if we have modules golang.org/x/tools and
-				// golang.org/x/tools/playground, and all of the packages matching
-				// golang.org/x/tools/playground... are in the
-				// golang.org/x/tools/playground module, then we should not *also* try
-				// to upgrade golang.org/x/tools if the user says 'go get
-				// golang.org/x/tools/playground...@latest'.
-				if match(m.Path) || str.HasPathPrefix(path, m.Path) {
-					queries = append(queries, &query{querySpec: querySpec{path: m.Path, vers: vers, prevM: m, forceModulePath: true}, arg: arg})
-					matched = true
+			// If we're using -m, look up modules in the build list that match
+			// the pattern. Report an error if no modules match.
+			if *getM {
+				match := search.MatchPattern(path)
+				matched := false
+				for _, m := range modload.BuildList() {
+					if match(m.Path) || str.HasPathPrefix(path, m.Path) {
+						queries = append(queries, &query{querySpec: querySpec{path: m.Path, vers: vers, prevM: m, forceModulePath: true}, arg: arg})
+						matched = true
+					}
+				}
+				if !matched {
+					base.Errorf("go get %s: pattern matches no modules in build list", arg)
+					continue
 				}
-			}
-			// If matched, we're done.
-			// If we're using -m, report an error.
-			// Otherwise, look up a module containing packages that match the pattern.
-			if matched {
 				break
 			}
-			if *getM {
-				base.Errorf("go get %s: pattern matches no modules in build list", arg)
-				continue
-			}
-			queries = append(queries, &query{querySpec: querySpec{path: path, vers: vers}, arg: arg})
+
+			// If we're not using -m, wait until we load packages to look up modules.
+			// We don't know yet whether any modules in the build list provide
+			// packages matching the pattern. For example, suppose
+			// golang.org/x/tools and golang.org/x/tools/playground are separate
+			// modules, and only golang.org/x/tools is in the build list. If the
+			// user runs 'go get golang.org/x/tools/playground/...', we should
+			// add a requirement for golang.org/x/tools/playground. We should not
+			// upgrade golang.org/x/tools.
 
 		case path == "all":
 			// This is the package pattern "all" not the module pattern "all",
@@ -463,8 +458,16 @@ func runGet(cmd *base.Command, args []string) {
 	var matches []*search.Match
 	var install []string
 	for {
-		var queries []*query
 		var seenPkgs map[string]bool
+		seenQuery := make(map[querySpec]bool)
+		var queries []*query
+		addQuery := func(q *query) {
+			if !seenQuery[q.querySpec] {
+				seenQuery[q.querySpec] = true
+				queries = append(queries, q)
+			}
+		}
+
 		if len(pkgPatterns) > 0 {
 			// Don't load packages if pkgPatterns is empty. Both
 			// modload.ImportPathsQuiet and ModulePackages convert an empty list
@@ -474,16 +477,21 @@ func runGet(cmd *base.Command, args []string) {
 			} else {
 				matches = modload.ImportPathsQuiet(pkgPatterns)
 			}
-			seenQuery := make(map[querySpec]bool)
 			seenPkgs = make(map[string]bool)
 			install = make([]string, 0, len(pkgPatterns))
 			for i, match := range matches {
-				if len(match.Pkgs) == 0 {
-					// We'll print a warning at the end of the outer loop to avoid
-					// repeating warnings on multiple iterations.
+				arg := pkgGets[i]
+
+				if !*getM && len(match.Pkgs) == 0 {
+					// If the pattern did not match any packages, look up a new module.
+					// If the pattern doesn't match anything on the last iteration,
+					// we'll print a warning after the outer loop.
+					if !search.IsRelativePath(arg.path) && !match.Literal && arg.path != "all" {
+						addQuery(&query{querySpec: querySpec{path: arg.path, vers: arg.vers}, arg: arg.raw})
+					}
 					continue
 				}
-				arg := pkgGets[i]
+
 				install = append(install, arg.path)
 				allStd := true
 				for _, pkg := range match.Pkgs {
@@ -501,11 +509,7 @@ func runGet(cmd *base.Command, args []string) {
 						continue
 					}
 					allStd = false
-					spec := querySpec{path: m.Path, vers: arg.vers}
-					if !seenQuery[spec] {
-						seenQuery[spec] = true
-						queries = append(queries, &query{querySpec: querySpec{path: m.Path, vers: arg.vers, forceModulePath: true, prevM: m}, arg: arg.raw})
-					}
+					addQuery(&query{querySpec: querySpec{path: m.Path, vers: arg.vers, forceModulePath: true, prevM: m}, arg: arg.raw})
 				}
 				if allStd {
 					if *getM {

src/cmd/go/testdata/mod/example.com_nest_sub_v1.0.0.txt

+Written by hand.
+Test case for nested modules without an explicit relationship.
+This is nested below the top-level module.
+
+-- .mod --
+module example.com/nest/sub
+-- .info --
+{"Version": "v1.0.0"}
+-- go.mod --
+module example.com/nest/sub
+-- y/y.go --
+package y

src/cmd/go/testdata/mod/example.com_nest_v1.0.0.txt

+Written by hand.
+Test case for nested modules without an explicit relationship.
+This is the top-level module.
+
+-- .mod --
+module example.com/nest
+-- .info --
+{"Version": "v1.0.0"}
+-- go.mod --
+module example.com/nest
+-- sub/x/x.go --
+package x

src/cmd/go/testdata/mod/example.com_nest_v1.1.0.txt

+Written by hand.
+Test case for nested modules without an explicit relationship.
+This is the top-level module.
+
+-- .mod --
+module example.com/nest
+-- .info --
+{"Version": "v1.1.0"}
+-- go.mod --
+module example.com/nest
+-- sub/x/x.go --
+package x

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

@@ -16,9 +16,10 @@ stderr '^go get rsc.io@v0.1.0: can.t get a specific version of the main module$'
 ! go get -d rsc.io/x@v0.1.0
 stderr '^go get rsc.io/x@v0.1.0: can.t query specific version for package rsc.io/x in the main module \(rsc.io\)$'
 
-# TODO: upgrading a package pattern not contained in the main module should not
+# Upgrading a package pattern not contained in the main module should not
 # attempt to upgrade the main module.
-! go get rsc.io/quote/...@v1.5.1
+go get rsc.io/quote/...@v1.5.1
+grep 'rsc.io/quote v1.5.1' go.mod
 
 -- go.mod.orig --
 module rsc.io

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

@@ -22,6 +22,14 @@ stderr 'go get rsc.io/quote/x...: module rsc.io/quote@latest \(v1.5.2\) found, b
 stderr 'go get rsc.io/quote/x/...: module rsc.io/quote@latest \(v1.5.2\) found, but does not contain packages matching rsc.io/quote/x/...'
 ! grep 'require rsc.io/quote' go.mod
 
+# If a pattern matches no packages within a module, the module should not
+# be upgraded, even if the module path matches the pattern.
+cp go.mod.orig go.mod
+go mod edit -require example.com/nest@v1.0.0
+go get example.com/nest/sub/y...
+grep 'example.com/nest/sub v1.0.0' go.mod
+grep 'example.com/nest v1.0.0' go.mod
+
 -- go.mod.orig --
 module m