modfile: in SetRequireSeparateIndirect, convert lines to blocks

When reading go.mod, SetRequireSeparateIndirect will insert new
requirements into the last uncommented direct-only or indirect-only
block OR line. If the last such statement is a line,
SetRequireSeparateIndirect converts it to a block before inserting new
requirements. Cleanup will convert it back to a line later if no
requirements are inserted.

For golang/go#47733

Change-Id: Id8ee3b0ce2d005488ddb3d9a5349115bd93938e7
Reviewed-on: https://go-review.googlesource.com/c/mod/+/348576
Trust: Jay Conrod <[email protected]>
Run-TryBot: Jay Conrod <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>

Author: Jay Conrod

modfile/rule.go

@@ -1087,9 +1087,8 @@ func (f *File) SetRequireSeparateIndirect(req []*Require) {
 		// We may insert new requirements into the last uncommented
 		// direct-only and indirect-only blocks. We may also move requirements
 		// to the opposite block if their indirect markings change.
-		lastDirectBlock, lastIndirectBlock *LineBlock
-		lastDirectBlockIndex               = -1
-		lastIndirectBlockIndex             = -1
+		lastDirectIndex   = -1
+		lastIndirectIndex = -1
 
 		// If there are no direct-only or indirect-only blocks, a new block may
 		// be inserted after the last require line or block.
@@ -1111,6 +1110,13 @@ func (f *File) SetRequireSeparateIndirect(req []*Require) {
 			}
 			lastRequireIndex = i
 			requireLineOrBlockCount++
+			if !hasComments(stmt.Comments) {
+				if isIndirect(stmt) {
+					lastIndirectIndex = i
+				} else {
+					lastDirectIndex = i
+				}
+			}
 
 		case *LineBlock:
 			if len(stmt.Token) == 0 || stmt.Token[0] != "require" {
@@ -1132,46 +1138,67 @@ func (f *File) SetRequireSeparateIndirect(req []*Require) {
 				}
 			}
 			if allDirect {
-				lastDirectBlock = stmt
-				lastDirectBlockIndex = i
+				lastDirectIndex = i
 			}
 			if allIndirect {
-				lastIndirectBlock = stmt
-				lastIndirectBlockIndex = i
+				lastIndirectIndex = i
 			}
 		}
 	}
 
 	oneFlatUncommentedBlock := requireLineOrBlockCount == 1 &&
 		!hasComments(*f.Syntax.Stmt[lastRequireIndex].Comment())
 
-	// Create direct and indirect blocks if needed. It's okay if they're empty.
-	// Cleanup will remove them and convert one-line blocks to lines.
-	if lastDirectBlock == nil {
-		lastDirectBlock = &LineBlock{Token: []string{"require"}}
-		i := len(f.Syntax.Stmt)
-		if lastIndirectBlockIndex >= 0 {
-			i = lastIndirectBlockIndex
-			lastIndirectBlockIndex++
-		} else if lastRequireIndex >= 0 {
-			i = lastRequireIndex + 1
-		}
+	// Create direct and indirect blocks if needed. Convert lines into blocks
+	// if needed. If we end up with an empty block or a one-line block,
+	// Cleanup will delete it or convert it to a line later.
+	insertBlock := func(i int) *LineBlock {
+		block := &LineBlock{Token: []string{"require"}}
 		f.Syntax.Stmt = append(f.Syntax.Stmt, nil)
 		copy(f.Syntax.Stmt[i+1:], f.Syntax.Stmt[i:])
-		f.Syntax.Stmt[i] = lastDirectBlock
-		lastDirectBlockIndex = i
+		f.Syntax.Stmt[i] = block
+		return block
 	}
 
-	if lastIndirectBlock == nil {
-		lastIndirectBlock = &LineBlock{Token: []string{"require"}}
-		i := len(f.Syntax.Stmt)
-		if lastDirectBlockIndex >= 0 {
-			i = lastDirectBlockIndex + 1
+	ensureBlock := func(i int) *LineBlock {
+		switch stmt := f.Syntax.Stmt[i].(type) {
+		case *LineBlock:
+			return stmt
+		case *Line:
+			block := &LineBlock{
+				Token: []string{"require"},
+				Line:  []*Line{stmt},
+			}
+			stmt.Token = stmt.Token[1:] // remove "require"
+			stmt.InBlock = true
+			f.Syntax.Stmt[i] = block
+			return block
+		default:
+			panic(fmt.Sprintf("unexpected statement: %v", stmt))
 		}
-		f.Syntax.Stmt = append(f.Syntax.Stmt, nil)
-		copy(f.Syntax.Stmt[i+1:], f.Syntax.Stmt[i:])
-		f.Syntax.Stmt[i] = lastIndirectBlock
-		lastIndirectBlockIndex = i
+	}
+
+	var lastDirectBlock *LineBlock
+	if lastDirectIndex < 0 {
+		if lastIndirectIndex >= 0 {
+			lastDirectIndex = lastIndirectIndex
+			lastIndirectIndex++
+		} else if lastRequireIndex >= 0 {
+			lastDirectIndex = lastRequireIndex + 1
+		} else {
+			lastDirectIndex = len(f.Syntax.Stmt)
+		}
+		lastDirectBlock = insertBlock(lastDirectIndex)
+	} else {
+		lastDirectBlock = ensureBlock(lastDirectIndex)
+	}
+
+	var lastIndirectBlock *LineBlock
+	if lastIndirectIndex < 0 {
+		lastIndirectIndex = lastDirectIndex + 1
+		lastIndirectBlock = insertBlock(lastIndirectIndex)
+	} else {
+		lastIndirectBlock = ensureBlock(lastIndirectIndex)
 	}
 
 	// Delete requirements we don't want anymore.

modfile/rule_test.go

@@ -339,6 +339,28 @@ var setRequireSeparateIndirectTests = []struct {
 		)
 		`,
 	},
+	{
+		`existing_line`,
+		`module m
+		require x.y/a v1.0.0
+		require x.y/c v1.0.0 // indirect
+		`,
+		[]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", true},
+		},
+		`module m
+		require (
+			x.y/a v1.2.3
+			x.y/b v1.2.3
+		)
+		require (
+			x.y/c v1.2.3 // indirect
+			x.y/d v1.2.3 // indirect
+		)`,
+	},
 	{
 		`existing_multi`,
 		`module m
@@ -366,7 +388,10 @@ var setRequireSeparateIndirectTests = []struct {
 			{"x.y/g", "v1.2.3", false},
 		},
 		`module m
-		require x.y/a v1.2.3
+		require (
+			x.y/a v1.2.3
+			x.y/h v1.2.3
+		)
 		require x.y/b v1.2.3 // indirect; demoted to indirect
 		require x.y/c v1.2.3 // not v1.2.3!
 		require x.y/d v1.2.3 // comment kept
@@ -376,7 +401,6 @@ var setRequireSeparateIndirectTests = []struct {
 		require x.y/g v1.2.3
 		require x.y/i v1.2.3 // indirect
 		require x.y/j v1.2.3 // indirect
-		require x.y/h v1.2.3
 		`,
 	},
 	{