Type accretion for struct fields should use equal (#3029)

We used to use isSubtype but we should just create a more specific type
when we can.

Fixes #2993

go/marshal/encode_test.go

@@ -686,15 +686,20 @@ func TestEncodeOriginal(t *testing.T) {
 	assert.True(MustMarshal(s).Equals(orig.Set("foo", types.Number(43))))
 
 	// Field type of base are preserved
-	st := types.MakeStructType("S", []string{"foo"},
-		[]*types.Type{types.MakeUnionType(types.StringType, types.NumberType)})
+	st := types.MakeStructTypeFromFields("S", types.FieldMap{
+		"foo": types.MakeUnionType(types.StringType, types.NumberType),
+	})
 	orig = types.NewStructWithType(st, []types.Value{types.Number(42)})
 	err = Unmarshal(orig, &s)
 	assert.NoError(err)
 	s.Foo = 43
 	out := MustMarshal(s)
 	assert.True(out.Equals(orig.Set("foo", types.Number(43))))
-	assert.True(out.Type().Equals(st))
+
+	st2 := types.MakeStructTypeFromFields("S", types.FieldMap{
+		"foo": types.NumberType,
+	})
+	assert.True(out.Type().Equals(st2))
 
 	// It's OK to have an empty original field
 	s = S{

go/types/assert_type_test.go

@@ -156,10 +156,10 @@ func TestAssertConcreteTypeIsUnion(tt *testing.T) {
 			MakeStructTypeFromFields("", FieldMap{"foo": StringType}),
 			MakeStructTypeFromFields("", FieldMap{"bar": StringType}))))
 
-	assertInvalid(tt,
+	assert.False(tt, IsSubtype(
 		MakeStructTypeFromFields("", FieldMap{}),
 		MakeUnionType(MakeStructTypeFromFields("", FieldMap{"foo": StringType}),
-			NumberType))
+			NumberType)))
 
 	assert.True(tt, IsSubtype(
 		MakeUnionType(
@@ -169,13 +169,13 @@ func TestAssertConcreteTypeIsUnion(tt *testing.T) {
 			MakeStructTypeFromFields("", FieldMap{"foo": StringType, "bar": StringType}),
 			MakeStructTypeFromFields("", FieldMap{"bar": StringType}))))
 
-	assertInvalid(tt,
+	assert.False(tt, IsSubtype(
 		MakeUnionType(
 			MakeStructTypeFromFields("", FieldMap{"foo": StringType}),
 			MakeStructTypeFromFields("", FieldMap{"bar": StringType})),
 		MakeUnionType(
 			MakeStructTypeFromFields("", FieldMap{"foo": StringType, "bar": StringType}),
-			NumberType))
+			NumberType)))
 }
 
 func TestAssertTypeEmptyListUnion(tt *testing.T) {

go/types/struct.go

@@ -121,7 +121,7 @@ func (s Struct) Get(n string) Value {
 // struct field a new struct type is created.
 func (s Struct) Set(n string, v Value) Struct {
 	f, i := s.desc().findField(n)
-	if i == -1 || !IsSubtype(f.t, v.Type()) {
+	if i == -1 || !f.t.Equals(v.Type()) {
 		// New/change field
 		data := make(StructData, len(s.values)+1)
 		for i, f := range s.desc().fields {

go/types/struct_test.go

@@ -84,10 +84,13 @@ func TestGenericStructSet(t *testing.T) {
 	s5 := s.Set("x", Number(42))
 	assert.True(MakeStructType("S3", []string{"b", "o", "x"}, []*Type{BoolType, StringType, NumberType}).Equals(s5.Type()))
 
-	// Subtype
+	// Subtype is not equal.
 	s6 := NewStruct("", StructData{"l": NewList(Number(0), Number(1), Bool(false), Bool(true))})
 	s7 := s6.Set("l", NewList(Number(2), Number(3)))
-	assert.True(s6.Type().Equals(s7.Type()))
+	t7 := MakeStructTypeFromFields("", FieldMap{
+		"l": MakeListType(NumberType),
+	})
+	assert.True(t7.Equals(s7.Type()))
 }
 
 func TestGenericStructDelete(t *testing.T) {

js/noms/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@attic/noms",
   "license": "Apache-2.0",
-  "version": "65.6.2",
+  "version": "65.7.0",
   "description": "Noms JS SDK",
   "repository": "https://github.com/attic-labs/noms/tree/master/js/noms",
   "main": "dist/commonjs/noms.js",

js/noms/src/struct-test.js

@@ -97,10 +97,13 @@ suite('Struct', () => {
       x: numberType,
     })));
 
-    // Subtype
+    // Subtype is not equal
     const s9 = newStruct('', {l: new List([0, 1, false, true])});
     const s10 = new StructMirror(s9).set('l', new List([2, 3]));
-    assert.isTrue(equals(s9.type, s10.type));
+    const t10 = makeStructType('', {
+      l: makeListType(numberType),
+    });
+    assert.isTrue(equals(t10, s10.type));
   });
 
   test('struct delete', () => {

js/noms/src/struct.js

@@ -15,7 +15,6 @@ import {getTypeOfValue, makeStructType, findFieldIndex} from './type.js';
 import {invariant} from './assert.js';
 import {isPrimitive} from './primitives.js';
 import * as Bytes from './bytes.js';
-import {isSubtype} from './assert-type.js';
 import walk from './walk.js';
 import type {WalkCallback} from './walk.js';
 import type {ValueReader} from './value-store.js';
@@ -184,7 +183,7 @@ export class StructMirror<T: Struct> {
   set(name: string, value: Value): Struct {
     const fields = this.desc.fields;
     const i = findFieldIndex(name, fields);
-    if (i === -1 || !isSubtype(fields[i].type, getTypeOfValue(value))) {
+    if (i === -1 || !equals(fields[i].type, getTypeOfValue(value))) {
       // New/change field
       const data = Object.create(null);
       for (let i = 0; i < fields.length; i++) {