Evaluate options even if values are invalid

cmp/compare.go

@@ -65,12 +65,12 @@ import (
 // option explicitly permits comparing the unexported field.
 //
 // Slices are equal if they are both nil or both non-nil, where recursively
-// calling Equal on all slice or array elements report equal.
+// calling Equal on all non-ignored slice or array elements report equal.
 // Empty non-nil slices and nil slices are not equal; to equate empty slices,
 // consider using cmpopts.EquateEmpty.
 //
 // Maps are equal if they are both nil or both non-nil, where recursively
-// calling Equal on all map entries report equal.
+// calling Equal on all non-ignored map entries report equal.
 // Map keys are equal according to the == operator.
 // To use custom comparisons for map keys, consider using cmpopts.SortMaps.
 // Empty non-nil maps and nil maps are not equal; to equate empty maps,
@@ -216,13 +216,6 @@ func (s *state) compareAny(step PathStep) {
 	t := step.Type()
 	vx, vy := step.Values()
 
-	// TODO: Removing this check allows us FilterPath to operate on missing
-	// slice elements and map entries.
-	if !vx.IsValid() || !vy.IsValid() {
-		s.report(vx.IsValid() == vy.IsValid(), 0)
-		return
-	}
-
 	// Rule 1: Check whether an option applies on this node in the value tree.
 	if s.tryOptions(t, vx, vy) {
 		return
@@ -444,14 +437,47 @@ func (s *state) compareSlice(t reflect.Type, vx, vy reflect.Value) {
 		return step
 	}
 
-	// Compute an edit-script for slices vx and vy.
-	edits := diff.Difference(vx.Len(), vy.Len(), func(ix, iy int) diff.Result {
-		return s.statelessCompare(withIndexes(ix, iy))
+	// Ignore options are able to ignore missing elements in a slice.
+	// However, detecting these reliably requires an optimal differencing
+	// algorithm, for which diff.Difference is not.
+	//
+	// Instead, we first iterate through both slices to detect which elements
+	// would be ignored if standing alone. The index of non-discarded elements
+	// are stored in a separate slice, which diffing is then performed on.
+	var indexesX, indexesY []int
+	var ignoredX, ignoredY []bool
+	for ix := 0; ix < vx.Len(); ix++ {
+		ignored := s.statelessCompare(withIndexes(ix, -1)).NumDiff == 0
+		if !ignored {
+			indexesX = append(indexesX, ix)
+		}
+		ignoredX = append(ignoredX, ignored)
+	}
+	for iy := 0; iy < vy.Len(); iy++ {
+		ignored := s.statelessCompare(withIndexes(-1, iy)).NumDiff == 0
+		if !ignored {
+			indexesY = append(indexesY, iy)
+		}
+		ignoredY = append(ignoredY, ignored)
+	}
+
+	// Compute an edit-script for slices vx and vy (excluding ignored elements).
+	edits := diff.Difference(len(indexesX), len(indexesY), func(ix, iy int) diff.Result {
+		return s.statelessCompare(withIndexes(indexesX[ix], indexesY[iy]))
 	})
 
-	// Replay the edit-script.
+	// Replay the ignore-scripts and the edit-script.
 	var ix, iy int
-	for _, e := range edits {
+	for ix < vx.Len() || iy < vy.Len() {
+		var e diff.EditType
+		switch {
+		case ix < len(ignoredX) && ignoredX[ix]:
+			e = diff.UniqueX
+		case iy < len(ignoredY) && ignoredY[iy]:
+			e = diff.UniqueY
+		default:
+			e, edits = edits[0], edits[1:]
+		}
 		switch e {
 		case diff.UniqueX:
 			s.compareAny(withIndexes(ix, -1))

cmp/compare_test.go

@@ -32,11 +32,12 @@ var now = time.Now()
 func intPtr(n int) *int { return &n }
 
 type test struct {
-	label     string       // Test description
+	label     string       // Test name
 	x, y      interface{}  // Input values to compare
 	opts      []cmp.Option // Input options
 	wantDiff  string       // The exact difference string
 	wantPanic string       // Sub-string of an expected panic message
+	reason    string       // The reason for the expected outcome
 }
 
 func TestDiff(t *testing.T) {
@@ -67,16 +68,17 @@ func TestDiff(t *testing.T) {
 				}()
 				gotDiff = cmp.Diff(tt.x, tt.y, tt.opts...)
 			}()
+			// TODO: Require every test case to provide a reason.
 			if tt.wantPanic == "" {
 				if gotPanic != "" {
-					t.Fatalf("unexpected panic message: %s", gotPanic)
+					t.Fatalf("unexpected panic message: %s\nreason: %v", gotPanic, tt.reason)
 				}
 				if got, want := strings.TrimSpace(gotDiff), strings.TrimSpace(tt.wantDiff); got != want {
-					t.Fatalf("difference message:\ngot:\n%s\n\nwant:\n%s", got, want)
+					t.Fatalf("difference message:\ngot:\n%s\nwant:\n%s\nreason: %v", gotDiff, tt.wantDiff, tt.reason)
 				}
 			} else {
 				if !strings.Contains(gotPanic, tt.wantPanic) {
-					t.Fatalf("panic message:\ngot:  %s\nwant: %s", gotPanic, tt.wantPanic)
+					t.Fatalf("panic message:\ngot:  %s\nwant: %s\nreason: %v", gotPanic, tt.wantPanic, tt.reason)
 				}
 			}
 		})
@@ -549,6 +551,60 @@ root[1]["hr"]:
 		label: label,
 		x:     struct{ _ string }{},
 		y:     struct{ _ string }{},
+	}, {
+		label: label,
+		x: [2][]int{
+			{0, 0, 0, 1, 2, 3, 0, 0, 4, 5, 6, 7, 8, 0, 9, 0, 0},
+			{0, 1, 0, 0, 0, 20},
+		},
+		y: [2][]int{
+			{1, 2, 3, 0, 4, 5, 6, 7, 0, 8, 9, 0, 0, 0},
+			{0, 0, 1, 2, 0, 0, 0},
+		},
+		opts: []cmp.Option{
+			cmp.FilterPath(func(p cmp.Path) bool {
+				vx, vy := p.Last().Values()
+				if vx.IsValid() && vx.Kind() == reflect.Int && vx.Int() == 0 {
+					return true
+				}
+				if vy.IsValid() && vy.Kind() == reflect.Int && vy.Int() == 0 {
+					return true
+				}
+				return false
+			}, cmp.Ignore()),
+		},
+		wantDiff: `
+{[2][]int}[1][5->3]:
+	-: 20
+	+: 2`,
+		reason: "all zero slice elements are ignored (even if missing)",
+	}, {
+		label: label,
+		x: [2]map[string]int{
+			{"ignore1": 0, "ignore2": 0, "keep1": 1, "keep2": 2, "KEEP3": 3, "IGNORE3": 0},
+			{"keep1": 1, "ignore1": 0},
+		},
+		y: [2]map[string]int{
+			{"ignore1": 0, "ignore3": 0, "ignore4": 0, "keep1": 1, "keep2": 2, "KEEP3": 3},
+			{"keep1": 1, "keep2": 2, "ignore2": 0},
+		},
+		opts: []cmp.Option{
+			cmp.FilterPath(func(p cmp.Path) bool {
+				vx, vy := p.Last().Values()
+				if vx.IsValid() && vx.Kind() == reflect.Int && vx.Int() == 0 {
+					return true
+				}
+				if vy.IsValid() && vy.Kind() == reflect.Int && vy.Int() == 0 {
+					return true
+				}
+				return false
+			}, cmp.Ignore()),
+		},
+		wantDiff: `
+{[2]map[string]int}[1]["keep2"]:
+	-: <non-existent>
+	+: 2`,
+		reason: "all zero map entries are ignored (even if missing)",
 	}}
 }
 

cmp/options.go

@@ -106,6 +106,11 @@ func (opts Options) String() string {
 // FilterPath returns a new Option where opt is only evaluated if filter f
 // returns true for the current Path in the value tree.
 //
+// This filter is called even if a slice element or map entry is missing and
+// provides an opportunity to ignore such cases. The filter function must be
+// symmetric such that the filter result is identical regardless of whether the
+// missing value is from x or y.
+//
 // The option passed in may be an Ignore, Transformer, Comparer, Options, or
 // a previously filtered Option.
 func FilterPath(f func(Path) bool, opt Option) Option {

go.mod

@@ -1 +1,3 @@
 module github.com/google/go-cmp
+
+go 1.8