cmd/go: avoid accidental downgrades in 'go get' with latest and patch

Currently, 'go get -u' and 'go get -u=patch' avoid accidentally
downgrading modules by preventing upgrades in two cases:

1) If the current version is a prerelease that is semantically later
   than the "latest" or "patch" version.
2) If the current version is a pseudoversion that is chronologically
   newer than the "latest" or "patch" version.

With this change, 'go get m@latest' and 'go get m@patch' prevent
downgrades using the same checks.

Also: 'go get m@patch' now works if m is a module path but not a
package path (i.e., there is no package in the module root directory).

Fixes #30634
Fixes #32537

Change-Id: I916630c385b5f3ba7c13e0d65ba08f73a1a67829
Reviewed-on: https://go-review.googlesource.com/c/go/+/180337
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/alldocs.go

@@ -586,11 +586,12 @@
 // depending on it as needed.
 //
 // The version suffix @latest explicitly requests the latest minor release of the
-// given path.
-//
-// The suffix @patch requests the latest patch release: if the path is already in
-// the build list, the selected version will have the same minor version.
-// If the path is not already in the build list, @patch is equivalent to @latest.
+// given path. The suffix @patch requests the latest patch release: if the path
+// is already in the build list, the selected version will have the same minor
+// version. If the path is not already in the build list, @patch is equivalent
+// to @latest. Neither @latest nor @patch will cause 'go get' to downgrade a module
+// in the build list if it is required at a newer pre-release version that is
+// newer than the latest released version.
 //
 // Although get defaults to using the latest version of the module containing
 // a named package, it does not use the latest version of that module's

src/cmd/go/internal/modload/build.go

@@ -79,7 +79,7 @@ func addUpdate(m *modinfo.ModulePublic) {
 		return
 	}
 
-	if info, err := Query(m.Path, "latest", Allowed); err == nil && semver.Compare(info.Version, m.Version) > 0 {
+	if info, err := Query(m.Path, "latest", m.Version, Allowed); err == nil && semver.Compare(info.Version, m.Version) > 0 {
 		m.Update = &modinfo.ModulePublic{
 			Path:    m.Path,
 			Version: info.Version,
@@ -127,7 +127,7 @@ func moduleInfo(m module.Version, fromBuildList bool) *modinfo.ModulePublic {
 	// complete fills in the extra fields in m.
 	complete := func(m *modinfo.ModulePublic) {
 		if m.Version != "" {
-			if q, err := Query(m.Path, m.Version, nil); err != nil {
+			if q, err := Query(m.Path, m.Version, "", nil); err != nil {
 				m.Error = &modinfo.ModuleError{Err: err.Error()}
 			} else {
 				m.Version = q.Version

src/cmd/go/internal/modload/init.go

@@ -739,7 +739,7 @@ func fixVersion(path, vers string) (string, error) {
 		return vers, nil
 	}
 
-	info, err := Query(path, vers, nil)
+	info, err := Query(path, vers, "", nil)
 	if err != nil {
 		return "", err
 	}

src/cmd/go/internal/modload/list.go

@@ -55,18 +55,28 @@ func listModules(args []string, listVersions bool) []*modinfo.ModulePublic {
 			base.Fatalf("go: cannot use relative path %s to specify module", arg)
 		}
 		if i := strings.Index(arg, "@"); i >= 0 {
-			info, err := Query(arg[:i], arg[i+1:], nil)
+			path := arg[:i]
+			vers := arg[i+1:]
+			var current string
+			for _, m := range buildList {
+				if m.Path == path {
+					current = m.Version
+					break
+				}
+			}
+
+			info, err := Query(path, vers, current, nil)
 			if err != nil {
 				mods = append(mods, &modinfo.ModulePublic{
-					Path:    arg[:i],
-					Version: arg[i+1:],
+					Path:    path,
+					Version: vers,
 					Error: &modinfo.ModuleError{
 						Err: err.Error(),
 					},
 				})
 				continue
 			}
-			mods = append(mods, moduleInfo(module.Version{Path: arg[:i], Version: info.Version}, false))
+			mods = append(mods, moduleInfo(module.Version{Path: path, Version: info.Version}, false))
 			continue
 		}
 
@@ -101,7 +111,7 @@ func listModules(args []string, listVersions bool) []*modinfo.ModulePublic {
 					// Don't make the user provide an explicit '@latest' when they're
 					// explicitly asking what the available versions are.
 					// Instead, resolve the module, even if it isn't an existing dependency.
-					info, err := Query(arg, "latest", nil)
+					info, err := Query(arg, "latest", "", nil)
 					if err == nil {
 						mods = append(mods, moduleInfo(module.Version{Path: arg, Version: info.Version}, false))
 					} else {

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

@@ -24,9 +24,13 @@ import (
 // The module must be a complete module path.
 // The version must take one of the following forms:
 //
-//	- the literal string "latest", denoting the latest available, allowed tagged version,
-//	  with non-prereleases preferred over prereleases.
-//	  If there are no tagged versions in the repo, latest returns the most recent commit.
+// - the literal string "latest", denoting the latest available, allowed
+//   tagged version, with non-prereleases preferred over prereleases.
+//   If there are no tagged versions in the repo, latest returns the most
+//   recent commit.
+// - the literal string "patch", denoting the latest available tagged version
+//   with the same major and minor number as current. If current is "",
+//   "patch" is equivalent to "latest".
 // - v1, denoting the latest available tagged version v1.x.x.
 // - v1.2, denoting the latest available tagged version v1.2.x.
 // - v1.2.3, a semantic version string denoting that tagged version.
@@ -35,20 +39,30 @@ import (
 //   with non-prereleases preferred over prereleases.
 // - a repository commit identifier or tag, denoting that commit.
 //
-// If the allowed function is non-nil, Query excludes any versions for which allowed returns false.
+// current is optional, denoting the current version of the module.
+// If query is "latest" or "patch", current will be returned if it is a newer
+// semantic version or if it is a chronologically later pseudoversion. This
+// prevents accidental downgrades from newer prerelease or development
+// versions.
+//
+// If the allowed function is non-nil, Query excludes any versions for which
+// allowed returns false.
 //
 // If path is the path of the main module and the query is "latest",
 // Query returns Target.Version as the version.
-func Query(path, query string, allowed func(module.Version) bool) (*modfetch.RevInfo, error) {
+func Query(path, query, current string, allowed func(module.Version) bool) (*modfetch.RevInfo, error) {
 	var info *modfetch.RevInfo
 	err := modfetch.TryProxies(func(proxy string) (err error) {
-		info, err = queryProxy(proxy, path, query, allowed)
+		info, err = queryProxy(proxy, path, query, current, allowed)
 		return err
 	})
 	return info, err
 }
 
-func queryProxy(proxy, path, query string, allowed func(module.Version) bool) (*modfetch.RevInfo, error) {
+func queryProxy(proxy, path, query, current string, allowed func(module.Version) bool) (*modfetch.RevInfo, error) {
+	if current != "" && !semver.IsValid(current) {
+		return nil, fmt.Errorf("invalid previous version %q", current)
+	}
 	if allowed == nil {
 		allowed = func(module.Version) bool { return true }
 	}
@@ -61,9 +75,22 @@ func queryProxy(proxy, path, query string, allowed func(module.Version) bool) (*
 	var ok func(module.Version) bool
 	var prefix string
 	var preferOlder bool
+	var mayUseLatest bool
 	switch {
 	case query == "latest":
 		ok = allowed
+		mayUseLatest = true
+
+	case query == "patch":
+		if current == "" {
+			ok = allowed
+			mayUseLatest = true
+		} else {
+			prefix = semver.MajorMinor(current)
+			ok = func(m module.Version) bool {
+				return matchSemverPrefix(prefix, m.Version) && allowed(m)
+			}
+		}
 
 	case strings.HasPrefix(query, "<="):
 		v := query[len("<="):]
@@ -166,41 +193,59 @@ func queryProxy(proxy, path, query string, allowed func(module.Version) bool) (*
 		return nil, err
 	}
 
+	lookup := func(v string) (*modfetch.RevInfo, error) {
+		rev, err := repo.Stat(v)
+		if err != nil {
+			return nil, err
+		}
+
+		// For "latest" and "patch", make sure we don't accidentally downgrade
+		// from a newer prerelease or from a chronologically newer pseudoversion.
+		if current != "" && (query == "latest" || query == "patch") {
+			currentTime, err := modfetch.PseudoVersionTime(current)
+			if semver.Compare(rev.Version, current) < 0 || (err == nil && rev.Time.Before(currentTime)) {
+				return repo.Stat(current)
+			}
+		}
+
+		return rev, nil
+	}
+
 	if preferOlder {
 		for _, v := range versions {
 			if semver.Prerelease(v) == "" && ok(module.Version{Path: path, Version: v}) {
-				return repo.Stat(v)
+				return lookup(v)
 			}
 		}
 		for _, v := range versions {
 			if semver.Prerelease(v) != "" && ok(module.Version{Path: path, Version: v}) {
-				return repo.Stat(v)
+				return lookup(v)
 			}
 		}
 	} else {
 		for i := len(versions) - 1; i >= 0; i-- {
 			v := versions[i]
 			if semver.Prerelease(v) == "" && ok(module.Version{Path: path, Version: v}) {
-				return repo.Stat(v)
+				return lookup(v)
 			}
 		}
 		for i := len(versions) - 1; i >= 0; i-- {
 			v := versions[i]
 			if semver.Prerelease(v) != "" && ok(module.Version{Path: path, Version: v}) {
-				return repo.Stat(v)
+				return lookup(v)
 			}
 		}
 	}
 
-	if query == "latest" {
+	if mayUseLatest {
 		// Special case for "latest": if no tags match, use latest commit in repo,
 		// provided it is not excluded.
-		if info, err := repo.Latest(); err == nil && allowed(module.Version{Path: path, Version: info.Version}) {
-			return info, nil
+		if latest, err := repo.Latest(); err == nil && allowed(module.Version{Path: path, Version: latest.Version}) {
+			return lookup(latest.Name)
 		}
 	}
 
-	return nil, &NoMatchingVersionError{query: query}
+	return nil, &NoMatchingVersionError{query: query, current: current}
 }
 
 // isSemverPrefix reports whether v is a semantic version prefix: v1 or v1.2 (not v1.2.3).
@@ -310,7 +355,7 @@ func QueryPattern(pattern, query string, allowed func(module.Version) bool) ([]Q
 	err := modfetch.TryProxies(func(proxy string) error {
 		queryModule := func(path string) (r QueryResult, err error) {
 			r.Mod.Path = path
-			r.Rev, err = queryProxy(proxy, path, query, allowed)
+			r.Rev, err = queryProxy(proxy, path, query, "", allowed)
 			if err != nil {
 				return r, err
 			}
@@ -445,11 +490,15 @@ func queryPrefixModules(candidateModules []string, queryModule func(path string)
 // code for the versions it knows about, and thus did not have the opportunity
 // to return a non-400 status code to suppress fallback.
 type NoMatchingVersionError struct {
-	query string
+	query, current string
 }
 
 func (e *NoMatchingVersionError) Error() string {
-	return fmt.Sprintf("no matching versions for query %q", e.query)
+	currentSuffix := ""
+	if (e.query == "latest" || e.query == "patch") && e.current != "" {
+		currentSuffix = fmt.Sprintf(" (current version is %s)", e.current)
+	}
+	return fmt.Sprintf("no matching versions for query %q", e.query) + currentSuffix
 }
 
 // A packageNotInModuleError indicates that QueryPattern found a candidate

src/cmd/go/internal/modload/query_test.go

@@ -52,6 +52,7 @@ var (
 var queryTests = []struct {
 	path    string
 	query   string
+	current string
 	allow   string
 	vers    string
 	err     string
@@ -108,7 +109,18 @@ var queryTests = []struct {
 	{path: queryRepo, query: "v1.9.10-pre2+wrongmetadata", err: `unknown revision v1.9.10-pre2+wrongmetadata`},
 	{path: queryRepo, query: "v1.9.10-pre2", err: `unknown revision v1.9.10-pre2`},
 	{path: queryRepo, query: "latest", vers: "v1.9.9"},
+	{path: queryRepo, query: "latest", current: "v1.9.10-pre1", vers: "v1.9.10-pre1"},
+	{path: queryRepo, query: "latest", current: "v1.9.10-pre2+metadata", vers: "v1.9.10-pre2.0.20190513201126-42abcb6df8ee"},
+	{path: queryRepo, query: "latest", current: "v0.0.0-20990101120000-5ba9a4ea6213", vers: "v0.0.0-20990101120000-5ba9a4ea6213"},
 	{path: queryRepo, query: "latest", allow: "NOMATCH", err: `no matching versions for query "latest"`},
+	{path: queryRepo, query: "latest", current: "v1.9.9", allow: "NOMATCH", err: `no matching versions for query "latest" (current version is v1.9.9)`},
+	{path: queryRepo, query: "latest", current: "v1.99.99", err: `unknown revision v1.99.99`},
+	{path: queryRepo, query: "patch", current: "", vers: "v1.9.9"},
+	{path: queryRepo, query: "patch", current: "v0.1.0", vers: "v0.1.2"},
+	{path: queryRepo, query: "patch", current: "v1.9.0", vers: "v1.9.9"},
+	{path: queryRepo, query: "patch", current: "v1.9.10-pre1", vers: "v1.9.10-pre1"},
+	{path: queryRepo, query: "patch", current: "v1.9.10-pre2+metadata", vers: "v1.9.10-pre2.0.20190513201126-42abcb6df8ee"},
+	{path: queryRepo, query: "patch", current: "v1.99.99", err: `no matching versions for query "patch" (current version is v1.99.99)`},
 	{path: queryRepo, query: ">v1.9.9", vers: "v1.9.10-pre1"},
 	{path: queryRepo, query: ">v1.10.0", err: `no matching versions for query ">v1.10.0"`},
 	{path: queryRepo, query: ">=v1.10.0", err: `no matching versions for query ">=v1.10.0"`},
@@ -147,8 +159,8 @@ func TestQuery(t *testing.T) {
 			ok, _ := path.Match(allow, m.Version)
 			return ok
 		}
-		t.Run(strings.ReplaceAll(tt.path, "/", "_")+"/"+tt.query+"/"+allow, func(t *testing.T) {
-			info, err := Query(tt.path, tt.query, allowed)
+		t.Run(strings.ReplaceAll(tt.path, "/", "_")+"/"+tt.query+"/"+tt.current+"/"+allow, func(t *testing.T) {
+			info, err := Query(tt.path, tt.query, tt.current, allowed)
 			if tt.err != "" {
 				if err != nil && err.Error() == tt.err {
 					return

src/cmd/go/internal/mvs/mvs.go

@@ -35,7 +35,7 @@ type Reqs interface {
 	// Max returns the maximum of v1 and v2 (it returns either v1 or v2).
 	//
 	// For all versions v, Max(v, "none") must be v,
-	// and for the tanget passed as the first argument to MVS functions,
+	// and for the target passed as the first argument to MVS functions,
 	// Max(target, v) must be target.
 	//
 	// Note that v1 < v2 can be written Max(v1, v2) != v1

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

+A module which has no root package.
+
+-- .mod --
+module example.com/noroot
+-- .info --
+{"Version":"v1.0.0"}
+-- pkg/pkg.go --
+package pkg

src/cmd/go/testdata/mod/example.com_noroot_v1.0.1.txt

+A module which has no root package.
+
+-- .mod --
+module example.com/noroot
+-- .info --
+{"Version":"v1.0.1"}
+-- pkg/pkg.go --
+package pkg

src/cmd/go/testdata/mod/example.com_pseudoupgrade_v0.0.0-20190429073000-30950c05d534.txt → src/cmd/go/testdata/mod/example.com_pseudoupgrade_v0.0.0-20190430073000-30950c05d534.txt

@@ -5,7 +5,7 @@ written by hand
 module example.com/pseudoupgrade
 
 -- .info --
-{"Version":"v0.0.0-20190429073000-30950c05d534","Name":"v0.0.0-20190429073000-30950c05d534","Short":"30950c05d534","Time":"2019-04-29T07:30:00Z"}
+{"Version":"v0.0.0-20190430073000-30950c05d534","Name":"v0.0.0-20190430073000-30950c05d534","Short":"30950c05d534","Time":"2019-04-30T07:30:00Z"}
 
 -- pseudoupgrade.go --
 package pseudoupgrade

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

@@ -14,6 +14,7 @@ go list -m all
 stdout '^github.com/rsc/legacytest v2\.0\.1-0\.\d{14}-7303f7796364\+incompatible$'
 
 # get should include incompatible tags in "latest" calculation.
+go mod edit -droprequire github.com/rsc/legacytest
 go get -d github.com/rsc/legacytest@latest
 go list
 go list -m all

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

@@ -3,17 +3,38 @@ env GO111MODULE=on
 # For this test module there are three versions:
 #   * v0.1.1-0.20190429073117-b5426c86b553
 #   * v0.1.0
-#   * v0.0.0-20190429073000-30950c05d534
+#   * v0.0.0-20190430073000-30950c05d534
 # Only v0.1.0 is tagged.
 #
-# The latest pseudo-version is semantically higher than the latest tag.
-# 'get -u' should not downgrade to the (lower) tagged version.
+# The v0.1.1 pseudo-version is semantically higher than the latest tag.
+# The v0.0.0 pseudo-version is chronologically newer.
 
+# 'get -u' should not downgrade to the (lower) tagged version.
 go get -d example.com/pseudoupgrade@b5426c8
-go get -u
+go get -d -u
 go list -m -u all
 stdout '^example.com/pseudoupgrade v0.1.1-0.20190429073117-b5426c86b553$'
 
+# 'get example.com/pseudoupgrade@latest' should not downgrade to
+# the (lower) tagged version.
+go get -d example.com/pseudoupgrade@latest
+go list -m all
+stdout '^example.com/pseudoupgrade v0.1.1-0.20190429073117-b5426c86b553$'
+
+# We should observe the same behavior with the newer pseudo-version.
+go get -d example.com/pseudoupgrade@v0.0.0-20190430073000-30950c05d534
+
+# 'get -u' should not downgrade to the chronologically older tagged version.
+go get -d -u
+go list -m -u all
+stdout '^example.com/pseudoupgrade v0.0.0-20190430073000-30950c05d534$'
+
+# 'get example.com/pseudoupgrade@latest' should not downgrade to the
+# chronologically older tagged version.
+go get -d example.com/pseudoupgrade@latest
+go list -m -u all
+stdout '^example.com/pseudoupgrade v0.0.0-20190430073000-30950c05d534$'
+
 -- go.mod --
 module x
 

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

@@ -3,9 +3,12 @@ env GO111MODULE=on
 # For this test module there are three versions:
 #   * v0.1.1-0.20190429073117-b5426c86b553
 #   * v0.1.0
-#   * v0.0.0-20190429073000-30950c05d534
+#   * v0.0.0-20190430073000-30950c05d534
 # Only v0.1.0 is tagged.
 #
+# The v0.1.1 pseudo-version is semantically higher than the latest tag.
+# The v0.0.0 pseudo-version is chronologically newer.
+
 # The latest pseudo-version is semantically higher than the latest tag.
 # 'list -u' should not suggest a lower version as an upgrade.
 
@@ -13,6 +16,10 @@ go get -d example.com/pseudoupgrade@b5426c8
 go list -m -u all
 stdout '^example.com/pseudoupgrade v0.1.1-0.20190429073117-b5426c86b553$'
 
+go get -d example.com/pseudoupgrade@v0.0.0-20190430073000-30950c05d534
+go list -m -u all
+stdout '^example.com/pseudoupgrade v0.0.0-20190430073000-30950c05d534$'
+
 -- go.mod --
 module x
 

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

@@ -10,7 +10,7 @@ go mod download example.com/badchain/b@v1.1.0
 go mod download example.com/badchain/c@v1.1.0
 
 # Try to update example.com/badchain/a (and its dependencies).
-! go get -d -u example.com/badchain/a
+! go get -d example.com/badchain/a
 cmp stderr update-a-expected
 cmp go.mod go.mod.orig
 

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

@@ -76,6 +76,13 @@ stderr 'cannot use pattern .* with explicit version'
 # However, standard-library packages without explicit versions are fine.
 go get -d -u=patch -d cmd/go
 
+# We can upgrade to a new version of a module with no root package.
+go get -d example.com/noroot@v1.0.0
+go list -m all
+stdout '^example.com/noroot v1.0.0$'
+go get -d example.com/noroot@patch
+go list -m all
+stdout '^example.com/noroot v1.0.1$'
 
 -- go.mod --
 module x