From ee972315998be1520635f765a1ea230ad9833cbc Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 6 Mar 2018 07:57:19 -0800 Subject: [PATCH] [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 TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer Reviewed-on: https://go-review.googlesource.com/102786 Run-TryBot: Andrew Bonventre Reviewed-by: Ian Lance Taylor --- src/cmd/cover/cover.go | 13 +++++++++---- src/cmd/cover/cover_test.go | 12 ++++++++++++ 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() -- 2.48.1