modfile: add SetRequireSeparateIndirect
The new method is a variant of SetRequire, but adds new indirect
dependencies only in indirect-only blocks, and does not add new direct
dependencies to existing indirect-only blocks.
For golang/go#45965
Change-Id: I6730b586396658e710e4bf2afcf64fb2c827203f
Reviewed-on: https://go-review.googlesource.com/c/mod/+/325971
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/modfile/rule.go b/modfile/rule.go
index b9b8cce..78f83fa 100644
--- a/modfile/rule.go
+++ b/modfile/rule.go
@@ -954,6 +954,175 @@
f.SortBlocks()
}
+// SetRequireSeparateIndirect updates the requirements of f to contain the given
+// requirements. Comment contents (except for 'indirect' markings) are retained
+// from the first existing requirement for each module path, and block structure
+// is maintained as long as the indirect markings match.
+//
+// Any requirements on paths not already present in the file are added. Direct
+// requirements are added to the last block containing *any* other direct
+// requirement. Indirect requirements are added to the last block containing
+// *only* other indirect requirements. If no suitable block exists, a new one is
+// added, with the last block containing a direct dependency (if any)
+// immediately before the first block containing only indirect dependencies.
+//
+// The Syntax field is ignored for requirements in the given blocks.
+func (f *File) SetRequireSeparateIndirect(req []*Require) {
+ type modKey struct {
+ path string
+ indirect bool
+ }
+ need := make(map[modKey]string)
+ for _, r := range req {
+ need[modKey{r.Mod.Path, r.Indirect}] = r.Mod.Version
+ }
+
+ comments := make(map[string]Comments)
+ for _, r := range f.Require {
+ v, ok := need[modKey{r.Mod.Path, r.Indirect}]
+ if !ok {
+ if _, ok := need[modKey{r.Mod.Path, !r.Indirect}]; ok {
+ if _, dup := comments[r.Mod.Path]; !dup {
+ comments[r.Mod.Path] = r.Syntax.Comments
+ }
+ }
+ r.markRemoved()
+ continue
+ }
+ r.setVersion(v)
+ delete(need, modKey{r.Mod.Path, r.Indirect})
+ }
+
+ var (
+ lastDirectOrMixedBlock Expr
+ firstIndirectOnlyBlock Expr
+ lastIndirectOnlyBlock Expr
+ )
+ for _, stmt := range f.Syntax.Stmt {
+ switch stmt := stmt.(type) {
+ case *Line:
+ if len(stmt.Token) == 0 || stmt.Token[0] != "require" {
+ continue
+ }
+ if isIndirect(stmt) {
+ lastIndirectOnlyBlock = stmt
+ } else {
+ lastDirectOrMixedBlock = stmt
+ }
+ case *LineBlock:
+ if len(stmt.Token) == 0 || stmt.Token[0] != "require" {
+ continue
+ }
+ indirectOnly := true
+ for _, line := range stmt.Line {
+ if len(line.Token) == 0 {
+ continue
+ }
+ if !isIndirect(line) {
+ indirectOnly = false
+ break
+ }
+ }
+ if indirectOnly {
+ lastIndirectOnlyBlock = stmt
+ if firstIndirectOnlyBlock == nil {
+ firstIndirectOnlyBlock = stmt
+ }
+ } else {
+ lastDirectOrMixedBlock = stmt
+ }
+ }
+ }
+
+ isOrContainsStmt := func(stmt Expr, target Expr) bool {
+ if stmt == target {
+ return true
+ }
+ if stmt, ok := stmt.(*LineBlock); ok {
+ if target, ok := target.(*Line); ok {
+ for _, line := range stmt.Line {
+ if line == target {
+ return true
+ }
+ }
+ }
+ }
+ return false
+ }
+
+ addRequire := func(path, vers string, indirect bool, comments Comments) {
+ var line *Line
+ if indirect {
+ if lastIndirectOnlyBlock != nil {
+ line = f.Syntax.addLine(lastIndirectOnlyBlock, "require", path, vers)
+ } else {
+ // Add a new require block after the last direct-only or mixed "require"
+ // block (if any).
+ //
+ // (f.Syntax.addLine would add the line to an existing "require" block if
+ // present, but here the existing "require" blocks are all direct-only, so
+ // we know we need to add a new block instead.)
+ line = &Line{Token: []string{"require", path, vers}}
+ lastIndirectOnlyBlock = line
+ firstIndirectOnlyBlock = line // only block implies first block
+ if lastDirectOrMixedBlock == nil {
+ f.Syntax.Stmt = append(f.Syntax.Stmt, line)
+ } else {
+ for i, stmt := range f.Syntax.Stmt {
+ if isOrContainsStmt(stmt, lastDirectOrMixedBlock) {
+ f.Syntax.Stmt = append(f.Syntax.Stmt, nil) // increase size
+ copy(f.Syntax.Stmt[i+2:], f.Syntax.Stmt[i+1:]) // shuffle elements up
+ f.Syntax.Stmt[i+1] = line
+ break
+ }
+ }
+ }
+ }
+ } else {
+ if lastDirectOrMixedBlock != nil {
+ line = f.Syntax.addLine(lastDirectOrMixedBlock, "require", path, vers)
+ } else {
+ // Add a new require block before the first indirect block (if any).
+ //
+ // That way if the file initially contains only indirect lines,
+ // the direct lines still appear before it: we preserve existing
+ // structure, but only to the extent that that structure already
+ // reflects the direct/indirect split.
+ line = &Line{Token: []string{"require", path, vers}}
+ lastDirectOrMixedBlock = line
+ if firstIndirectOnlyBlock == nil {
+ f.Syntax.Stmt = append(f.Syntax.Stmt, line)
+ } else {
+ for i, stmt := range f.Syntax.Stmt {
+ if isOrContainsStmt(stmt, firstIndirectOnlyBlock) {
+ f.Syntax.Stmt = append(f.Syntax.Stmt, nil) // increase size
+ copy(f.Syntax.Stmt[i+1:], f.Syntax.Stmt[i:]) // shuffle elements up
+ f.Syntax.Stmt[i] = line
+ break
+ }
+ }
+ }
+ }
+ }
+
+ line.Comments.Before = commentsAdd(line.Comments.Before, comments.Before)
+ line.Comments.Suffix = commentsAdd(line.Comments.Suffix, comments.Suffix)
+
+ r := &Require{
+ Mod: module.Version{Path: path, Version: vers},
+ Indirect: indirect,
+ Syntax: line,
+ }
+ r.setIndirect(indirect)
+ f.Require = append(f.Require, r)
+ }
+
+ for k, vers := range need {
+ addRequire(k.path, vers, k.indirect, comments[k.path])
+ }
+ f.SortBlocks()
+}
+
func (f *File) DropRequire(path string) error {
for _, r := range f.Require {
if r.Mod.Path == path {
diff --git a/modfile/rule_test.go b/modfile/rule_test.go
index 7c6f0e8..68d74c5 100644
--- a/modfile/rule_test.go
+++ b/modfile/rule_test.go
@@ -254,6 +254,302 @@
},
}
+var setRequireSeparateIndirectTests = []struct {
+ desc string
+ in string
+ mods []require
+ out string
+}{
+ {
+ `https://golang.org/issue/45932`,
+ `module m
+ require (
+ x.y/a v1.2.3 //indirect
+ x.y/b v1.2.3
+ x.y/c v1.2.3
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", false},
+ {"x.y/b", "v1.2.3", false},
+ {"x.y/c", "v1.2.3", false},
+ },
+ `module m
+ require (
+ x.y/a v1.2.3
+ x.y/b v1.2.3
+ x.y/c v1.2.3
+ )
+ `,
+ },
+ {
+ `existing`,
+ `module m
+ require (
+ x.y/b v1.2.3
+
+ x.y/a v1.2.3
+ x.y/d v1.2.3
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", false},
+ {"x.y/b", "v1.2.3", false},
+ {"x.y/c", "v1.2.3", false},
+ },
+ `module m
+ require (
+ x.y/a v1.2.3
+ x.y/b v1.2.3
+ x.y/c v1.2.3
+ )
+ `,
+ },
+ {
+ `existing_indirect`,
+ `module m
+ require (
+ x.y/a v1.2.3
+ x.y/b v1.2.3 //
+ x.y/c v1.2.3 //c
+ x.y/d v1.2.3 // c
+ x.y/e v1.2.3 // indirect
+ x.y/f v1.2.3 //indirect
+ x.y/g v1.2.3 // indirect
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", true},
+ {"x.y/b", "v1.2.3", true},
+ {"x.y/c", "v1.2.3", true},
+ {"x.y/d", "v1.2.3", true},
+ {"x.y/e", "v1.2.3", true},
+ {"x.y/f", "v1.2.3", true},
+ {"x.y/g", "v1.2.3", true},
+ },
+ `module m
+ require (
+ x.y/a v1.2.3 // indirect
+ x.y/b v1.2.3 // indirect
+ x.y/c v1.2.3 // indirect; c
+ x.y/d v1.2.3 // indirect; c
+ x.y/e v1.2.3 // indirect
+ x.y/f v1.2.3 //indirect
+ x.y/g v1.2.3 // indirect
+ )
+ `,
+ },
+ {
+ `existing_multi`,
+ `module m
+ require x.y/a v1.2.3
+ require x.y/b v1.2.3 // demoted to indirect
+ require x.y/c v1.0.0 // not v1.2.3!
+ require x.y/d v1.2.3 // comment kept
+ require x.y/e v1.2.3 // comment kept
+ require x.y/f v1.2.3 // indirect; promoted to direct
+ // promoted to direct
+ require x.y/g v1.2.3 // indirect
+ require x.y/i v1.2.3 // indirect
+ require x.y/j v1.2.3 // indirect
+ `,
+ []require{
+ {"x.y/h", "v1.2.3", false}, // out of alphabetical order
+ {"x.y/i", "v1.2.3", true},
+ {"x.y/j", "v1.2.3", true},
+ {"x.y/a", "v1.2.3", false},
+ {"x.y/b", "v1.2.3", true},
+ {"x.y/c", "v1.2.3", false},
+ {"x.y/d", "v1.2.3", false},
+ {"x.y/e", "v1.2.3", true},
+ {"x.y/f", "v1.2.3", false},
+ {"x.y/g", "v1.2.3", false},
+ },
+ `module m
+ require x.y/a v1.2.3
+
+ require x.y/c v1.2.3 // not v1.2.3!
+
+ require (
+ x.y/d v1.2.3 // comment kept
+ x.y/f v1.2.3 // promoted to direct
+ // promoted to direct
+ x.y/g v1.2.3
+ x.y/h v1.2.3
+ )
+
+ require x.y/i v1.2.3 // indirect
+
+ require (
+ x.y/b v1.2.3 // indirect; demoted to indirect
+ x.y/e v1.2.3 // indirect; comment kept
+ x.y/j v1.2.3 // indirect
+ )
+ `,
+ },
+ {
+ `existing_duplicate`,
+ `module m
+ require (
+ x.y/a v1.0.0 // zero
+ x.y/a v1.1.0 // one
+ x.y/a v1.2.3 // two
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", true},
+ },
+ `module m
+ require x.y/a v1.2.3 // indirect; zero
+ `,
+ },
+ {
+ `existing_duplicate_multi`,
+ `module m
+ require x.y/a v1.0.0 // zero
+ require x.y/a v1.1.0 // one
+ require x.y/a v1.2.3 // two
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", true},
+ },
+ `module m
+ require x.y/a v1.2.3 // indirect; zero
+ `,
+ },
+ {
+ `existing_duplicate_mix_indirect`,
+ `module m
+ require (
+ x.y/a v1.0.0 // zero
+ x.y/a v1.1.0 // indirect; one
+ x.y/a v1.2.3 // indirect; two
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", true},
+ },
+ `module m
+ require x.y/a v1.2.3 // indirect; one
+ `,
+ },
+ {
+ `existing_duplicate_mix_direct`,
+ `module m
+ require (
+ x.y/a v1.0.0 // indirect; zero
+ x.y/a v1.1.0 // one
+ x.y/a v1.2.3 // two
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", false},
+ },
+ `module m
+ require x.y/a v1.2.3 // one
+ `,
+ },
+ {
+ `add_indirect_after_last_direct`,
+ `module m
+ require (
+ x.y/a v1.0.0 // comment a preserved
+ x.y/d v1.0.0 // comment d preserved
+ )
+ require (
+ x.y/b v1.0.0 // comment b preserved
+ x.y/e v1.0.0 // comment e preserved
+ )
+ go 1.17
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", false},
+ {"x.y/b", "v1.2.3", false},
+ {"x.y/c", "v1.2.3", true},
+ {"x.y/d", "v1.2.3", false},
+ {"x.y/e", "v1.2.3", false},
+ {"x.y/f", "v1.2.3", true},
+ },
+ `module m
+ require (
+ x.y/a v1.2.3 // comment a preserved
+ x.y/d v1.2.3 // comment d preserved
+ )
+ require (
+ x.y/b v1.2.3 // comment b preserved
+ x.y/e v1.2.3 // comment e preserved
+ )
+ require (
+ x.y/c v1.2.3 // indirect
+ x.y/f v1.2.3 // indirect
+ )
+ go 1.17
+ `,
+ },
+ {
+ `add_direct_before_first_indirect`,
+ `module m
+ require (
+ x.y/b v1.0.0 // indirect; comment b preserved
+ x.y/e v1.0.0 // indirect; comment d preserved
+ )
+ require (
+ x.y/c v1.0.0 // indirect; comment c preserved
+ x.y/f v1.0.0 // indirect; comment e preserved
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", false},
+ {"x.y/b", "v1.2.3", true},
+ {"x.y/c", "v1.2.3", true},
+ {"x.y/d", "v1.2.3", false},
+ {"x.y/e", "v1.2.3", true},
+ {"x.y/f", "v1.2.3", true},
+ },
+ `module m
+ require (
+ x.y/a v1.2.3
+ x.y/d v1.2.3
+ )
+ require (
+ x.y/b v1.2.3 // indirect; comment b preserved
+ x.y/e v1.2.3 // indirect; comment d preserved
+ )
+ require (
+ x.y/c v1.2.3 // indirect; comment c preserved
+ x.y/f v1.2.3 // indirect; comment e preserved
+ )
+ `,
+ },
+ {
+ `add_indirect_after_mixed`,
+ `module m
+ require (
+ x.y/a v1.0.0
+ x.y/b v1.0.0 // indirect
+ )
+ `,
+ []require{
+ {"x.y/a", "v1.2.3", false},
+ {"x.y/b", "v1.2.3", true}, // should remain in the existing mixed block
+ {"x.y/c", "v1.2.3", true}, // should be added in an indirect-only block
+ {"x.y/d", "v1.2.3", false}, // should appear in the existing mixed block
+ {"x.y/e", "v1.2.3", true}, // should be added in the same indirect-only block
+ },
+ `module m
+ require (
+ x.y/a v1.2.3
+ x.y/b v1.2.3 // indirect
+ x.y/d v1.2.3
+ )
+ require (
+ x.y/c v1.2.3 // indirect
+ x.y/e v1.2.3 // indirect
+ )
+ `,
+ },
+}
+
var addGoTests = []struct {
desc string
in string
@@ -1012,6 +1308,33 @@
}
}
+func TestSetRequireSeparateIndirect(t *testing.T) {
+ for _, tt := range setRequireSeparateIndirectTests {
+ t.Run(tt.desc, func(t *testing.T) {
+ var mods []*Require
+ for _, mod := range tt.mods {
+ mods = append(mods, &Require{
+ Mod: module.Version{
+ Path: mod.path,
+ Version: mod.vers,
+ },
+ Indirect: mod.indirect,
+ })
+ }
+
+ f := testEdit(t, tt.in, tt.out, true, func(f *File) error {
+ f.SetRequireSeparateIndirect(mods)
+ f.Cleanup()
+ return nil
+ })
+
+ if len(f.Require) != len(mods) {
+ t.Errorf("after Cleanup, len(Require) = %v; want %v", len(f.Require), len(mods))
+ }
+ })
+ }
+}
+
func TestAddGo(t *testing.T) {
for _, tt := range addGoTests {
t.Run(tt.desc, func(t *testing.T) {
