[release-branch.go1.10] cmd/cover: don't crash on non-gofmt'ed input

Without the change to cover.go, the new test fails with

panic: overlapping edits: [4946,4950)->"", [4947,4947)->"thisNameMustBeVeryLongToCauseOverflowOfCounterIncrementStatementOntoNextLineForTest.Count[112]++;"

The original code inserts "else{", deletes "else", and then positions
a new block just after the "}" that must come before the "else".
That works on gofmt'ed code, but fails if the code looks like "}else".
When there is no space between the "{" and the "else", the new block
is inserted into a location that we are deleting, leading to the
"overlapping edits" mentioned above.

This CL fixes this case by not deleting the "else" but just using the
one that is already there. That requires adjust the block offset to
come after the "{" that we insert.

Fixes #23927

Change-Id: I40ef592490878765bbce6550ddb439e43ac525b2
Reviewed-on: https://go-review.googlesource.com/98935
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-on: https://go-review.googlesource.com/102786
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

2 files changed, 21 insertions, 4 deletions

diff --git a/src/cmd/cover/cover.go b/src/cmd/cover/cover.go
index 500027ee0d..f496f4cff6 100644
--- a/src/cmd/cover/cover.go
+++ b/src/cmd/cover/cover.go
@@ -238,23 +238,28 @@ func (f *File) Visit(node ast.Node) ast.Visitor {
 		//		if y {
 		//		}
 		//	}
-		f.edit.Insert(f.offset(n.Body.End()), "else{")
 		elseOffset := f.findText(n.Body.End(), "else")
 		if elseOffset < 0 {
 			panic("lost else")
 		}
-		f.edit.Delete(elseOffset, elseOffset+4)
+		f.edit.Insert(elseOffset+4, "{")
 		f.edit.Insert(f.offset(n.Else.End()), "}")
+
+		// We just created a block, now walk it.
+		// Adjust the position of the new block to start after
+		// the "else". That will cause it to follow the "{"
+		// we inserted above.
+		pos := f.fset.File(n.Body.End()).Pos(elseOffset + 4)
 		switch stmt := n.Else.(type) {
 		case *ast.IfStmt:
 			block := &ast.BlockStmt{
-				Lbrace: n.Body.End(), // Start at end of the "if" block so the covered part looks like it starts at the "else".
+				Lbrace: pos,
 				List:   []ast.Stmt{stmt},
 				Rbrace: stmt.End(),
 			}
 			n.Else = block
 		case *ast.BlockStmt:
-			stmt.Lbrace = n.Body.End() // Start at end of the "if" block so the covered part looks like it starts at the "else".
+			stmt.Lbrace = pos
 		default:
 			panic("unexpected node type in if")
 		}
diff --git a/src/cmd/cover/cover_test.go b/src/cmd/cover/cover_test.go
index 79ddf4f465..f20fbb4b71 100644
--- a/src/cmd/cover/cover_test.go
+++ b/src/cmd/cover/cover_test.go
@@ -59,6 +59,17 @@ func TestCover(t *testing.T) {
 	for i, line := range lines {
 		lines[i] = bytes.Replace(line, []byte("LINE"), []byte(fmt.Sprint(i+1)), -1)
 	}
+
+	// Add a function that is not gofmt'ed. This used to cause a crash.
+	// We don't put it in test.go because then we would have to gofmt it.
+	// Issue 23927.
+	lines = append(lines, []byte("func unFormatted() {"),
+		[]byte("\tif true {"),
+		[]byte("\t}else{"),
+		[]byte("\t}"),
+		[]byte("}"))
+	lines = append(lines, []byte("func unFormatted2(b bool) {if b{}else{}}"))
+
 	if err := ioutil.WriteFile(coverInput, bytes.Join(lines, []byte("\n")), 0666); err != nil {
 		t.Fatal(err)
 	}
@@ -246,6 +257,7 @@ func TestCoverFunc(t *testing.T) {
 }
 
 func run(c *exec.Cmd, t *testing.T) {
+	t.Helper()
 	c.Stdout = os.Stdout
 	c.Stderr = os.Stderr
 	err := c.Run()