cmp: apply cleanupSurroundingIdentical's prepend/append after the loop instead of in defer

cmp/report_slices.go

@@ -552,6 +552,13 @@ func coalesceInterveningIdentical(groups []diffStats, windowSize int) []diffStat
 //	]
 func cleanupSurroundingIdentical(groups []diffStats, eq func(i, j int) bool) []diffStats {
 	var ix, iy int // indexes into sequence x and y
+
+	// Identical spans detected on the first or last group have no adjacent
+	// group to be folded into, so a new group must be prepended or appended.
+	// That is done after the loop since mutating the slice mid-loop would
+	// invalidate the group indexes used by the remaining iterations.
+	var numPrepend, numAppend int
+
 	for i, ds := range groups {
 		// Handle equal group.
 		if ds.NumDiff() == 0 {
@@ -579,9 +586,7 @@ func cleanupSurroundingIdentical(groups []diffStats, eq func(i, j int) bool) []d
 				} else {
 					// No preceding group exists, so prepend a new group,
 					// but do so after we finish iterating over all groups.
-					defer func() {
-						groups = append([]diffStats{{Name: groups[0].Name, NumIdentical: numLeadingIdentical}}, groups...)
-					}()
+					numPrepend = numLeadingIdentical
 				}
 				// Increment indexes since the preceding group would have handled this.
 				ix += numLeadingIdentical
@@ -595,9 +600,7 @@ func cleanupSurroundingIdentical(groups []diffStats, eq func(i, j int) bool) []d
 				} else {
 					// No succeeding group exists, so append a new group,
 					// but do so after we finish iterating over all groups.
-					defer func() {
-						groups = append(groups, diffStats{Name: groups[len(groups)-1].Name, NumIdentical: numTrailingIdentical})
-					}()
+					numAppend = numTrailingIdentical
 				}
 				// Do not increment indexes since the succeeding group will handle this.
 			}
@@ -610,5 +613,11 @@ func cleanupSurroundingIdentical(groups []diffStats, eq func(i, j int) bool) []d
 		ix += nx
 		iy += ny
 	}
+	if numPrepend > 0 {
+		groups = append([]diffStats{{Name: groups[0].Name, NumIdentical: numPrepend}}, groups...)
+	}
+	if numAppend > 0 {
+		groups = append(groups, diffStats{Name: groups[len(groups)-1].Name, NumIdentical: numAppend})
+	}
 	return groups
 }

cmp/report_slices_test.go

@@ -0,0 +1,82 @@
+// Copyright 2019, The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package cmp
+
+import (
+	"reflect"
+	"testing"
+)
+
+// TestCleanupSurroundingIdenticalEdgeGroups verifies that identical spans
+// detected at the very start or very end of the group list are retained.
+//
+// When the first (or last) group is an unequal group that has leading (or
+// trailing) identical elements, there is no adjacent group to fold the span
+// into, so a new group must be prepended (or appended). That mutation must
+// happen after the iteration completes, since inserting into the slice
+// mid-loop would invalidate the group indexes. Previously the code used a
+// deferred closure to mutate the local slice variable, but the function has
+// an unnamed result, so the return value was already assigned before the
+// deferred closures ran. The mutation was therefore silently discarded and
+// those identical elements were dropped from the report.
+func TestCleanupSurroundingIdenticalEdgeGroups(t *testing.T) {
+	tests := []struct {
+		name   string
+		groups []diffStats
+		eq     func(i, j int) bool
+		want   []diffStats
+	}{
+		{
+			name:   "LeadingIdenticalWithNoPrecedingGroup",
+			groups: []diffStats{{Name: "byte", NumRemoved: 2, NumInserted: 1}},
+			// Leading span: eq(0,0) is true. Trailing: eq(1,0) is false.
+			eq:   func(i, j int) bool { return i == 0 },
+			want: []diffStats{{Name: "byte", NumIdentical: 1}, {Name: "byte", NumRemoved: 1}},
+		},
+		{
+			name:   "TrailingIdenticalWithNoSucceedingGroup",
+			groups: []diffStats{{Name: "byte", NumRemoved: 1, NumInserted: 2}},
+			// Leading: eq(0,0) is false. Trailing span: eq(0,1) is true.
+			eq:   func(i, j int) bool { return j == 1 },
+			want: []diffStats{{Name: "byte", NumInserted: 1}, {Name: "byte", NumIdentical: 1}},
+		},
+		{
+			// x span is [A C B], y span is [A B]: one leading identical and
+			// one trailing identical element, with no adjacent group for either.
+			name:   "BothEdgesOfASingleUnequalGroup",
+			groups: []diffStats{{Name: "byte", NumRemoved: 3, NumInserted: 2}},
+			eq:     func(i, j int) bool { return (i == 0 && j == 0) || (i == 2 && j == 1) },
+			want: []diffStats{
+				{Name: "byte", NumIdentical: 1},
+				{Name: "byte", NumRemoved: 1},
+				{Name: "byte", NumIdentical: 1},
+			},
+		},
+		{
+			// A middle group with a leading identical span still folds into its
+			// preceding equal group; behaviour here must not change.
+			name: "MiddleGroupKeepsFoldingIntoNeighbours",
+			groups: []diffStats{
+				{Name: "byte", NumIdentical: 4},
+				{Name: "byte", NumRemoved: 2, NumInserted: 1},
+				{Name: "byte", NumIdentical: 3},
+			},
+			eq: func(i, j int) bool { return i == 4 && j == 4 },
+			want: []diffStats{
+				{Name: "byte", NumIdentical: 5},
+				{Name: "byte", NumRemoved: 1},
+				{Name: "byte", NumIdentical: 3},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := cleanupSurroundingIdentical(tt.groups, tt.eq)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("cleanupSurroundingIdentical(%v, eq) = %v, want %v", tt.groups, got, tt.want)
+			}
+		})
+	}
+}