Merge pull request #108 from liggitt/map-compare

Fix map comparison of nil values and missing keys
evanphx · Aug 8, 2020 · 162e562 · 162e562
2 parents 1d4c88e + 44b01e1
commit 162e562
Show file tree

Hide file tree

Showing 4 changed files with 146 additions and 2 deletions.
diff --git a/merge.go b/merge.go
@@ -311,7 +311,12 @@ func matchesValue(av, bv interface{}) bool {
 			return false
 		}
 		for key := range bt {
-			if !matchesValue(at[key], bt[key]) {
+			av, aOK := at[key]
+			bv, bOK := bt[key]
+			if aOK != bOK {
+				return false
+			}
+			if !matchesValue(av, bv) {
 				return false
 			}
 		}

diff --git a/merge_test.go b/merge_test.go
@@ -462,6 +462,73 @@ func createNestedMap(m map[string]interface{}, depth int, objectCount *int) {
 	}
 }
 
+func TestMatchesValue(t *testing.T) {
+	testcases := []struct {
+		name string
+		a    interface{}
+		b    interface{}
+		want bool
+	}{
+		{
+			name: "map empty",
+			a:    map[string]interface{}{},
+			b:    map[string]interface{}{},
+			want: true,
+		},
+		{
+			name: "map equal keys, equal non-nil value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"1": true},
+			want: true,
+		},
+		{
+			name: "map equal keys, equal nil value",
+			a:    map[string]interface{}{"1": nil},
+			b:    map[string]interface{}{"1": nil},
+			want: true,
+		},
+
+		{
+			name: "map different value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"1": false},
+			want: false,
+		},
+		{
+			name: "map different key, matching non-nil value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"2": true},
+			want: false,
+		},
+		{
+			name: "map different key, matching nil value",
+			a:    map[string]interface{}{"1": nil},
+			b:    map[string]interface{}{"2": nil},
+			want: false,
+		},
+		{
+			name: "map different key, first nil value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"2": nil},
+			want: false,
+		},
+		{
+			name: "map different key, second nil value",
+			a:    map[string]interface{}{"1": nil},
+			b:    map[string]interface{}{"2": true},
+			want: false,
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := matchesValue(tc.a, tc.b)
+			if got != tc.want {
+				t.Fatalf("want %v, got %v", tc.want, got)
+			}
+		})
+	}
+}
+
 func benchmarkMatchesValueWithDeeplyNestedFields(depth int, b *testing.B) {
 	a := map[string]interface{}{}
 	objCount := 1

diff --git a/v5/merge.go b/v5/merge.go
@@ -311,7 +311,12 @@ func matchesValue(av, bv interface{}) bool {
 			return false
 		}
 		for key := range bt {
-			if !matchesValue(at[key], bt[key]) {
+			av, aOK := at[key]
+			bv, bOK := bt[key]
+			if aOK != bOK {
+				return false
+			}
+			if !matchesValue(av, bv) {
 				return false
 			}
 		}

diff --git a/v5/merge_test.go b/v5/merge_test.go
@@ -462,6 +462,73 @@ func createNestedMap(m map[string]interface{}, depth int, objectCount *int) {
 	}
 }
 
+func TestMatchesValue(t *testing.T) {
+	testcases := []struct {
+		name string
+		a    interface{}
+		b    interface{}
+		want bool
+	}{
+		{
+			name: "map empty",
+			a:    map[string]interface{}{},
+			b:    map[string]interface{}{},
+			want: true,
+		},
+		{
+			name: "map equal keys, equal non-nil value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"1": true},
+			want: true,
+		},
+		{
+			name: "map equal keys, equal nil value",
+			a:    map[string]interface{}{"1": nil},
+			b:    map[string]interface{}{"1": nil},
+			want: true,
+		},
+
+		{
+			name: "map different value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"1": false},
+			want: false,
+		},
+		{
+			name: "map different key, matching non-nil value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"2": true},
+			want: false,
+		},
+		{
+			name: "map different key, matching nil value",
+			a:    map[string]interface{}{"1": nil},
+			b:    map[string]interface{}{"2": nil},
+			want: false,
+		},
+		{
+			name: "map different key, first nil value",
+			a:    map[string]interface{}{"1": true},
+			b:    map[string]interface{}{"2": nil},
+			want: false,
+		},
+		{
+			name: "map different key, second nil value",
+			a:    map[string]interface{}{"1": nil},
+			b:    map[string]interface{}{"2": true},
+			want: false,
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := matchesValue(tc.a, tc.b)
+			if got != tc.want {
+				t.Fatalf("want %v, got %v", tc.want, got)
+			}
+		})
+	}
+}
+
 func benchmarkMatchesValueWithDeeplyNestedFields(depth int, b *testing.B) {
 	a := map[string]interface{}{}
 	objCount := 1