Skip to content

Commit

Permalink
making the cyclic check resistant to reused slices and maps
Browse files Browse the repository at this point in the history
  • Loading branch information
yazgazan committed Jan 17, 2018
1 parent a80ff5b commit 2f2416d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
9 changes: 7 additions & 2 deletions diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,13 @@ type visited struct {
}

func (v *visited) add(lhs, rhs reflect.Value) error {
if canAddr(lhs) {
if canAddr(lhs) && !isEmptyMapOrSlice(lhs) {
if inPointers(v.lhs, lhs) {
return ErrCyclic
}
v.lhs = append(v.lhs, lhs.Pointer())
}
if canAddr(rhs) {
if canAddr(rhs) && !isEmptyMapOrSlice(lhs) {
if inPointers(v.rhs, rhs) {
return ErrCyclic
}
Expand All @@ -174,6 +174,11 @@ func (v *visited) add(lhs, rhs reflect.Value) error {
return nil
}

func isEmptyMapOrSlice(v reflect.Value) bool {
// we don't want to include empty slices and maps in our cyclic check, since these are not problematic
return (v.Kind() == reflect.Slice || v.Kind() == reflect.Map) && v.Len() == 0
}

func inPointers(pointers []uintptr, val reflect.Value) bool {
for _, lhs := range pointers {
if lhs == val.Pointer() {
Expand Down
20 changes: 20 additions & 0 deletions diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,22 @@ func TestCircular(t *testing.T) {
},
},
}
emptySlice := map[int]interface{}{
0: []interface{}{},
}
emptySlice[1] = emptySlice[0]
emptySliceNotRepeating := map[int]interface{}{
0: []interface{}{},
1: []interface{}{},
}
emptyMap := map[int]interface{}{
0: map[int]interface{}{},
}
emptyMap[1] = emptyMap[0]
emptyMapNotRepeating := map[int]interface{}{
0: map[int]interface{}{},
1: map[int]interface{}{},
}

for _, test := range []struct {
lhs interface{}
Expand All @@ -567,6 +583,10 @@ func TestCircular(t *testing.T) {
{lhs: first, rhs: notCyclic, wantError: true},
{lhs: notCyclic, rhs: first, wantError: true},
{lhs: notCyclic, rhs: notCyclic},
{lhs: emptySlice, rhs: emptySliceNotRepeating},
{lhs: emptySliceNotRepeating, rhs: emptySlice},
{lhs: emptyMap, rhs: emptyMapNotRepeating},
{lhs: emptyMapNotRepeating, rhs: emptyMap},
} {
d, err := Diff(test.lhs, test.rhs)

Expand Down

0 comments on commit 2f2416d

Please # to comment.