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 <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

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
 }

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)