From d06704a3c2c3bb668926c8d20b9d4855b7131148 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 12 Mar 2019 19:10:43 -0700 Subject: [PATCH] cmd/cover: don't assume duplicate positions are in order Fixes #30746 Change-Id: I63f2d82f14eeaab6b14e956e21ddeec56fee025b Reviewed-on: https://go-review.googlesource.com/c/go/+/167257 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Rob Pike --- src/cmd/cover/cover.go | 52 +++++++++++++------- src/cmd/cover/cover_test.go | 94 +++++++++++++++++++++++++++++++++---- 2 files changed, 120 insertions(+), 26 deletions(-) diff --git a/src/cmd/cover/cover.go b/src/cmd/cover/cover.go index 0348849578..2394e57977 100644 --- a/src/cmd/cover/cover.go +++ b/src/cmd/cover/cover.go @@ -646,20 +646,11 @@ func (f *File) addVariables(w io.Writer) { // - 32-bit starting line number // - 32-bit ending line number // - (16 bit ending column number << 16) | (16-bit starting column number). - var lastStart, lastEnd token.Position for i, block := range f.blocks { start := f.fset.Position(block.startByte) end := f.fset.Position(block.endByte) - // It is possible for positions to repeat when there is a - // line directive that does not specify column information - // and the input has not been passed through gofmt. - // See issue #27350 and TestHtmlUnformatted. - if samePos(start, lastStart) && samePos(end, lastEnd) { - end.Column++ - } - lastStart = start - lastEnd = end + start, end = dedup(start, end) fmt.Fprintf(w, "\t\t%d, %d, %#x, // [%d]\n", start.Line, end.Line, (end.Column&0xFFFF)<<16|(start.Column&0xFFFF), i) } @@ -710,10 +701,39 @@ func isValidIdentifier(ident string) bool { return true } -// samePos returns whether two positions have the same file/line/column. -// We don't use p1 == p2 because token.Position also has an Offset field, -// and when the input uses //line directives two Positions can have different -// Offset values while having the same file/line/dolumn. -func samePos(p1, p2 token.Position) bool { - return p1.Filename == p2.Filename && p1.Line == p2.Line && p1.Column == p2.Column +// It is possible for positions to repeat when there is a line +// directive that does not specify column information and the input +// has not been passed through gofmt. +// See issues #27530 and #30746. +// Tests are TestHtmlUnformatted and TestLineDup. +// We use a map to avoid duplicates. + +// pos2 is a pair of token.Position values, used as a map key type. +type pos2 struct { + p1, p2 token.Position +} + +// seenPos2 tracks whether we have seen a token.Position pair. +var seenPos2 = make(map[pos2]bool) + +// dedup takes a token.Position pair and returns a pair that does not +// duplicate any existing pair. The returned pair will have the Offset +// fields cleared. +func dedup(p1, p2 token.Position) (r1, r2 token.Position) { + key := pos2{ + p1: p1, + p2: p2, + } + + // We want to ignore the Offset fields in the map, + // since cover uses only file/line/column. + key.p1.Offset = 0 + key.p2.Offset = 0 + + for seenPos2[key] { + key.p2.Column++ + } + seenPos2[key] = true + + return key.p1, key.p2 } diff --git a/src/cmd/cover/cover_test.go b/src/cmd/cover/cover_test.go index cf8f3d2384..a53660f744 100644 --- a/src/cmd/cover/cover_test.go +++ b/src/cmd/cover/cover_test.go @@ -40,16 +40,20 @@ var ( htmlGolden = filepath.Join(testdata, "html", "html.golden") // Temporary files. - tmpTestMain string - coverInput string - coverOutput string - htmlProfile string - htmlHTML string - htmlUDir string - htmlU string - htmlUTest string - htmlUProfile string - htmlUHTML string + tmpTestMain string + coverInput string + coverOutput string + htmlProfile string + htmlHTML string + htmlUDir string + htmlU string + htmlUTest string + htmlUProfile string + htmlUHTML string + lineDupDir string + lineDupGo string + lineDupTestGo string + lineDupProfile string ) var ( @@ -96,6 +100,10 @@ func TestMain(m *testing.M) { htmlUTest = filepath.Join(htmlUDir, "htmlunformatted_test.go") htmlUProfile = filepath.Join(htmlUDir, "htmlunformatted.cov") htmlUHTML = filepath.Join(htmlUDir, "htmlunformatted.html") + lineDupDir = filepath.Join(dir, "linedup") + lineDupGo = filepath.Join(lineDupDir, "linedup.go") + lineDupTestGo = filepath.Join(lineDupDir, "linedup_test.go") + lineDupProfile = filepath.Join(lineDupDir, "linedup.out") status := m.Run() @@ -484,6 +492,72 @@ lab: run(cmd, t) } +// lineDupContents becomes linedup.go in TestFuncWithDuplicateLines. +const lineDupContents = ` +package linedup + +var G int + +func LineDup(c int) { + for i := 0; i < c; i++ { +//line ld.go:100 + if i % 2 == 0 { + G++ + } + if i % 3 == 0 { + G++; G++ + } +//line ld.go:100 + if i % 4 == 0 { + G++; G++; G++ + } + if i % 5 == 0 { + G++; G++; G++; G++ + } + } +} +` + +// lineDupTestContents becomes linedup_test.go in TestFuncWithDuplicateLines. +const lineDupTestContents = ` +package linedup + +import "testing" + +func TestLineDup(t *testing.T) { + LineDup(100) +} +` + +// Test -func with duplicate //line directives with different numbers +// of statements. +func TestFuncWithDuplicateLines(t *testing.T) { + t.Parallel() + testenv.MustHaveGoRun(t) + buildCover(t) + + if err := os.Mkdir(lineDupDir, 0777); err != nil { + t.Fatal(err) + } + + if err := ioutil.WriteFile(lineDupGo, []byte(lineDupContents), 0444); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(lineDupTestGo, []byte(lineDupTestContents), 0444); err != nil { + t.Fatal(err) + } + + // go test -cover -covermode count -coverprofile TMPDIR/linedup.out + cmd := exec.Command(testenv.GoToolPath(t), "test", toolexecArg, "-cover", "-covermode", "count", "-coverprofile", lineDupProfile) + cmd.Dir = lineDupDir + run(cmd, t) + + // testcover -func=TMPDIR/linedup.out + cmd = exec.Command(testcover, "-func", lineDupProfile) + cmd.Dir = testTempDir + run(cmd, t) +} + func run(c *exec.Cmd, t *testing.T) { t.Helper() t.Log("running", c.Args) -- 2.48.1