Add implicit filter to Transformers (#29)

Declaring a transformer "func(T) T" where T is a concrete type
is a common transformation. However, this is currently problematic
as the transformation now infinitely applies to itself recursively.

In order to allow this form of transformation, add a simple
(but subtle) filter to Transformers where a Transformer can only
apply if that specific Transformer does not already exist within
the tail of the current Path since the last non-Transform step.

This rule does not prevent more advance usages of Transformers
where the user *does* want the Transformer to apply recursively
to previously transformed output, since those situations are
almost always followed by some path step that is *not* a
transformation (e.g., pointer indirect, slice indexing, etc).

3 files changed, 19 insertions, 18 deletions

diff --git a/cmp/compare_test.go b/cmp/compare_test.go
index 6a69ad3..dc247ca 100644
--- a/cmp/compare_test.go
+++ b/cmp/compare_test.go
@@ -1633,15 +1633,11 @@ func (gs germSorter) Swap(i, j int)      { gs[i], gs[j] = gs[j], gs[i] }
 func project2Tests() []test {
 	const label = "Project2"
 
-	sortGerms := cmp.FilterValues(func(x, y []*pb.Germ) bool {
-		ok1 := sort.IsSorted(germSorter(x))
-		ok2 := sort.IsSorted(germSorter(y))
-		return !ok1 || !ok2
-	}, cmp.Transformer("Sort", func(in []*pb.Germ) []*pb.Germ {
+	sortGerms := cmp.Transformer("Sort", func(in []*pb.Germ) []*pb.Germ {
 		out := append([]*pb.Germ(nil), in...) // Make copy
 		sort.Sort(germSorter(out))
 		return out
-	}))
+	})
 
 	equalDish := cmp.Comparer(func(x, y *ts.Dish) bool {
 		if x == nil || y == nil {
@@ -1739,7 +1735,7 @@ func project2Tests() []test {
 {teststructs.GermBatch}.DirtyGerms[17]:
 	-: <non-existent>
 	+: []*testprotos.Germ{s"germ1"}
-{teststructs.GermBatch}.DirtyGerms[18][2->?]:
+Sort({teststructs.GermBatch}.DirtyGerms[18])[2->?]:
 	-: s"germ4"
 	+: <non-existent>
 {teststructs.GermBatch}.DishMap[1]:
diff --git a/cmp/example_test.go b/cmp/example_test.go
index 8729db3..67ead5b 100644
--- a/cmp/example_test.go
+++ b/cmp/example_test.go
@@ -258,16 +258,11 @@ func ExampleOption_equalEmpty() {
 // This example is for demonstrative purposes; use cmpopts.SortSlices instead.
 func ExampleOption_sortedSlice() {
 	// This Transformer sorts a []int.
-	// Since the transformer transforms []int into []int, there is problem where
-	// this is recursively applied forever. To prevent this, use a FilterValues
-	// to first check for the condition upon which the transformer ought to apply.
-	trans := cmp.FilterValues(func(x, y []int) bool {
-		return !sort.IntsAreSorted(x) || !sort.IntsAreSorted(y)
-	}, cmp.Transformer("Sort", func(in []int) []int {
+	trans := cmp.Transformer("Sort", func(in []int) []int {
 		out := append([]int(nil), in...) // Copy input to avoid mutating it
 		sort.Ints(out)
 		return out
-	}))
+	})
 
 	x := struct{ Ints []int }{[]int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}}
 	y := struct{ Ints []int }{[]int{2, 8, 0, 9, 6, 1, 4, 7, 3, 5}}
diff --git a/cmp/options.go b/cmp/options.go
index a4e159a..c19fbc7 100644
--- a/cmp/options.go
+++ b/cmp/options.go
@@ -215,9 +215,12 @@ func (invalid) apply(s *state, _, _ reflect.Value) bool {
 // The transformer f must be a function "func(T) R" that converts values of
 // type T to those of type R and is implicitly filtered to input values
 // assignable to T. The transformer must not mutate T in any way.
-// If T and R are the same type, an additional filter must be applied to
-// act as the base case to prevent an infinite recursion applying the same
-// transform to itself (see the SortedSlice example).
+//
+// To help prevent some cases of infinite recursive cycles applying the
+// same transform to the output of itself (e.g., in the case where the
+// input and output types are the same), an implicit filter is added such that
+// a transformer is applicable only if that exact transformer is not already
+// in the tail of the Path since the last non-Transform step.
 //
 // The name is a user provided label that is used as the Transform.Name in the
 // transformation PathStep. If empty, an arbitrary name is used.
@@ -248,7 +251,14 @@ type transformer struct {
 
 func (tr *transformer) isFiltered() bool { return tr.typ != nil }
 
-func (tr *transformer) filter(_ *state, _, _ reflect.Value, t reflect.Type) applicableOption {
+func (tr *transformer) filter(s *state, _, _ reflect.Value, t reflect.Type) applicableOption {
+	for i := len(s.curPath) - 1; i >= 0; i-- {
+		if t, ok := s.curPath[i].(*transform); !ok {
+			break // Hit most recent non-Transform step
+		} else if tr == t.trans {
+			return nil // Cannot directly use same Transform
+		}
+	}
 	if tr.typ == nil || t.AssignableTo(tr.typ) {
 		return tr
 	}