Evaluate options even if values are invalid (#121)

Rather than checking up-front whether the values are invalid,
give tryOptions a chance to operate on them and potentially ignore
the values. This is useful since a value can only be invalid if it
is a missing slice element or map entry, and provides FilterPath
combined with Ignore the ability to ignore such cases.

Some complexity is added to compareSlice to look for ignored elements
first before applying diffing so that we can decouple the stability
of the diffing algorithm from the primary result.

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