Skip to content

Commit

Permalink
Support named strings as inlined map keys (#83)
Browse files Browse the repository at this point in the history
It should be possible to support map[~string]T as an inlined fallback
so long as the string type does not implement marshal/unmarshal methods.

Co-authored-by: Nick Khyl <[email protected]>
  • Loading branch information
dsnet and nickkhyl authored Dec 21, 2024
1 parent 764075c commit 27d2cb3
Show file tree
Hide file tree
Showing 5 changed files with 211 additions and 14 deletions.
11 changes: 7 additions & 4 deletions arshal_inlined.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ func marshalInlinedFallbackAll(enc *jsontext.Encoder, va addressableValue, mo *j
}
return nil
} else {
m := v // must be a map[string]V
m := v // must be a map[~string]V
n := m.Len()
if n == 0 {
return nil
}
mk := newAddressableValue(stringType)
mk := newAddressableValue(m.Type().Key())
mv := newAddressableValue(m.Type().Elem())
marshalKey := func(mk addressableValue) error {
xe := export.Encoder(enc)
Expand Down Expand Up @@ -202,12 +202,15 @@ func unmarshalInlinedFallbackNext(dec *jsontext.Decoder, va addressableValue, uo
} else {
name := string(unquotedName) // TODO: Intern this?

m := v // must be a map[string]V
m := v // must be a map[~string]V
if m.IsNil() {
m.Set(reflect.MakeMap(m.Type()))
}
mk := reflect.ValueOf(name)
mv := newAddressableValue(v.Type().Elem()) // TODO: Cache across calls?
if mkt := m.Type().Key(); mkt != stringType {
mk = mk.Convert(mkt)
}
mv := newAddressableValue(m.Type().Elem()) // TODO: Cache across calls?
if v2 := m.MapIndex(mk); v2.IsValid() {
mv.Set(v2)
}
Expand Down
184 changes: 184 additions & 0 deletions arshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,14 @@ type (
structInlineMapStringInt struct {
X map[string]int `json:",inline"`
}
structInlineMapNamedStringInt struct {
X map[namedString]int `json:",inline"`
}
structInlineMapNamedStringAny struct {
A int `json:",omitzero"`
X map[namedString]any `json:",inline"`
B int `json:",omitzero"`
}
structNoCaseInlineTextValue struct {
AAA string `json:",omitempty,strictcase"`
AA_b string `json:",omitempty"`
Expand Down Expand Up @@ -2665,6 +2673,72 @@ func TestMarshal(t *testing.T) {
},
want: `{"one":"1","two":"2","zero":"0"}`,
canonicalize: true,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringInt"),
in: structInlineMapNamedStringInt{
X: map[namedString]int{"zero": 0, "one": 1, "two": 2},
},
want: `{"one":1,"two":2,"zero":0}`,
canonicalize: true,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringInt/Deterministic"),
opts: []Options{Deterministic(true)},
in: structInlineMapNamedStringInt{
X: map[namedString]int{"zero": 0, "one": 1, "two": 2},
},
want: `{"one":1,"two":2,"zero":0}`,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/Nil"),
in: structInlineMapNamedStringAny{X: nil},
want: `{}`,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/Empty"),
in: structInlineMapNamedStringAny{X: make(map[namedString]any)},
want: `{}`,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/NonEmptyN1"),
in: structInlineMapNamedStringAny{X: map[namedString]any{"fizz": nil}},
want: `{"fizz":null}`,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/NonEmptyN2"),
in: structInlineMapNamedStringAny{X: map[namedString]any{"fizz": time.Time{}, "buzz": math.Pi}},
want: `{"buzz":3.141592653589793,"fizz":"0001-01-01T00:00:00Z"}`,
canonicalize: true,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/NonEmptyWithOthers"),
in: structInlineMapNamedStringAny{
A: 1,
X: map[namedString]any{"fizz": nil},
B: 2,
},
// NOTE: Inlined fallback fields are always serialized last.
want: `{"A":1,"B":2,"fizz":null}`,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/RejectInvalidUTF8"),
opts: []Options{jsontext.AllowInvalidUTF8(false)},
in: structInlineMapNamedStringAny{X: map[namedString]any{"\xde\xad\xbe\xef": nil}},
want: `{`,
wantErr: EM(jsonwire.ErrInvalidUTF8).withPos(`{`, "").withType(0, T[namedString]()),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/AllowInvalidUTF8"),
opts: []Options{jsontext.AllowInvalidUTF8(true)},
in: structInlineMapNamedStringAny{X: map[namedString]any{"\xde\xad\xbe\xef": nil}},
want: `{"ޭ��":null}`,
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/InvalidValue"),
opts: []Options{jsontext.AllowInvalidUTF8(true)},
in: structInlineMapNamedStringAny{X: map[namedString]any{"name": make(chan string)}},
want: `{"name"`,
wantErr: EM(nil).withPos(`{"name":`, "/name").withType(0, T[chan string]()),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MarshalFuncV1"),
opts: []Options{
WithMarshalers(MarshalFuncV1(func(v float64) ([]byte, error) {
return []byte(fmt.Sprintf(`"%v"`, v)), nil
})),
},
in: structInlineMapNamedStringAny{X: map[namedString]any{"fizz": 3.14159}},
want: `{"fizz":"3.14159"}`,
}, {
name: jsontest.Name("Structs/InlinedFallback/DiscardUnknownMembers"),
opts: []Options{DiscardUnknownMembers(true)},
Expand Down Expand Up @@ -6781,6 +6855,116 @@ func TestUnmarshal(t *testing.T) {
want: addr(structInlineMapStringInt{
X: map[string]int{"zero": 0, "one": 1, "two": 2},
}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringInt"),
inBuf: `{"zero": 0, "one": 1, "two": 2}`,
inVal: new(structInlineMapNamedStringInt),
want: addr(structInlineMapNamedStringInt{
X: map[namedString]int{"zero": 0, "one": 1, "two": 2},
}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringInt/Null"),
inBuf: `{"zero": 0, "one": null, "two": 2}`,
inVal: new(structInlineMapNamedStringInt),
want: addr(structInlineMapNamedStringInt{
X: map[namedString]int{"zero": 0, "one": 0, "two": 2},
}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringInt/Invalid"),
inBuf: `{"zero": 0, "one": {}, "two": 2}`,
inVal: new(structInlineMapNamedStringInt),
want: addr(structInlineMapNamedStringInt{
X: map[namedString]int{"zero": 0, "one": 0},
}),
wantErr: EU(nil).withPos(`{"zero": 0, "one": `, "/one").withType('{', T[int]()),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringInt/StringifiedNumbers"),
opts: []Options{StringifyNumbers(true)},
inBuf: `{"zero": "0", "one": 1, "two": "2"}`,
inVal: new(structInlineMapNamedStringInt),
want: addr(structInlineMapNamedStringInt{
X: map[namedString]int{"zero": 0, "one": 0},
}),
wantErr: EU(nil).withPos(`{"zero": "0", "one": `, "/one").withType('0', T[int]()),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringInt/UnmarshalFuncV1"),
opts: []Options{
WithUnmarshalers(UnmarshalFuncV1(func(b []byte, v *int) error {
i, err := strconv.ParseInt(string(bytes.Trim(b, `"`)), 10, 64)
if err != nil {
return err
}
*v = int(i)
return nil
})),
},
inBuf: `{"zero": "0", "one": "1", "two": "2"}`,
inVal: new(structInlineMapNamedStringInt),
want: addr(structInlineMapNamedStringInt{
X: map[namedString]int{"zero": 0, "one": 1, "two": 2},
}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/Noop"),
inBuf: `{"A":1,"B":2}`,
inVal: new(structInlineMapNamedStringAny),
want: addr(structInlineMapNamedStringAny{A: 1, X: nil, B: 2}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MergeN1/Nil"),
inBuf: `{"A":1,"fizz":"buzz","B":2}`,
inVal: new(structInlineMapNamedStringAny),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": "buzz"}, B: 2}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MergeN1/Empty"),
inBuf: `{"A":1,"fizz":"buzz","B":2}`,
inVal: addr(structInlineMapNamedStringAny{X: map[namedString]any{}}),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": "buzz"}, B: 2}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MergeN1/ObjectN1"),
inBuf: `{"A":1,"fizz":{"charlie":"DELTA","echo":"foxtrot"},"B":2}`,
inVal: addr(structInlineMapNamedStringAny{X: map[namedString]any{"fizz": jsonObject{
"alpha": "bravo",
"charlie": "delta",
}}}),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": jsonObject{
"alpha": "bravo",
"charlie": "DELTA",
"echo": "foxtrot",
}}, B: 2}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MergeN2/ObjectN1"),
inBuf: `{"A":1,"fizz":"buzz","B":2,"foo": [ 1 , 2 , 3 ]}`,
inVal: addr(structInlineMapNamedStringAny{X: map[namedString]any{"fizz": "wuzz"}}),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": "buzz", "foo": jsonArray{1.0, 2.0, 3.0}}, B: 2}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MergeInvalidValue"),
inBuf: `{"A":1,"fizz":nil,"B":2}`,
inVal: new(structInlineMapNamedStringAny),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": nil}}),
wantErr: newInvalidCharacterError("i", "within literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MergeInvalidValue/Existing"),
inBuf: `{"A":1,"fizz":nil,"B":2}`,
inVal: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": true}}),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": true}}),
wantErr: newInvalidCharacterError("i", "within literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/CaseSensitive"),
inBuf: `{"A":1,"fizz":"buzz","B":2,"a":3}`,
inVal: new(structInlineMapNamedStringAny),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": "buzz", "a": 3.0}, B: 2}),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/RejectDuplicateNames"),
opts: []Options{jsontext.AllowDuplicateNames(false)},
inBuf: `{"A":1,"fizz":"buzz","B":2,"fizz":"buzz"}`,
inVal: new(structInlineMapNamedStringAny),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": "buzz"}, B: 2}),
wantErr: newDuplicateNameError("", []byte(`"fizz"`), len64(`{"A":1,"fizz":"buzz","B":2,`)),
}, {
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/AllowDuplicateNames"),
opts: []Options{jsontext.AllowDuplicateNames(true)},
inBuf: `{"A":1,"fizz":{"one":1,"two":-2},"B":2,"fizz":{"two":2,"three":3}}`,
inVal: new(structInlineMapNamedStringAny),
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": map[string]any{"one": 1.0, "two": 2.0, "three": 3.0}}, B: 2}),
}, {
name: jsontest.Name("Structs/InlinedFallback/RejectUnknownMembers"),
opts: []Options{RejectUnknownMembers(true)},
Expand Down
6 changes: 3 additions & 3 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@
// A Go embedded field is implicitly inlined unless an explicit JSON name
// is specified. The inlined field must be a Go struct
// (that does not implement any JSON methods), [jsontext.Value],
// map[string]T, or an unnamed pointer to such types. When marshaling,
// map[~string]T, or an unnamed pointer to such types. When marshaling,
// inlined fields from a pointer type are omitted if it is nil.
// Inlined fields of type [jsontext.Value] and map[string]T are called
// Inlined fields of type [jsontext.Value] and map[~string]T are called
// “inlined fallbacks” as they can represent all possible
// JSON object members not directly handled by the parent struct.
// Only one inlined fallback field may be specified in a struct,
Expand All @@ -119,7 +119,7 @@
// - unknown: The "unknown" option is a specialized variant
// of the inlined fallback to indicate that this Go struct field
// contains any number of unknown JSON object members. The field type must
// be a [jsontext.Value], map[string]T, or an unnamed pointer to such types.
// be a [jsontext.Value], map[~string]T, or an unnamed pointer to such types.
// If [DiscardUnknownMembers] is specified when marshaling,
// the contents of this field are ignored.
// If [RejectUnknownMembers] is specified when unmarshaling,
Expand Down
12 changes: 8 additions & 4 deletions fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func makeStructFields(root reflect.Type) (sf structFields, serr *SemanticError)
// Reject any types with custom serialization otherwise
// it becomes impossible to know what sub-fields to inline.
tf := indirectType(f.typ)
if which := implementsWhich(tf, allMethodTypes...); which != nil && tf != jsontextValueType {
if implementsWhich(tf, allMethodTypes...) != nil && tf != jsontextValueType {
serr = orErrorf(serr, t, "inlined Go struct field %s of type %s must not implement marshal or unmarshal methods", sf.Name, tf)
continue // invalid inlined field; treat as ignored
}
Expand All @@ -141,16 +141,20 @@ func makeStructFields(root reflect.Type) (sf structFields, serr *SemanticError)
seen[tf] = true
continue
} else if !sf.IsExported() {
err := fmt.Errorf("inlined Go struct field %s is not exported", sf.Name)
return structFields{}, &SemanticError{GoType: t, Err: err}
serr = orErrorf(serr, t, "inlined Go struct field %s is not exported", sf.Name)
continue // invalid inlined field; treat as ignored
}

// Handle an inlined field that serializes to/from any number of
// JSON object members back by a Go map or jsontext.Value.
switch {
case tf == jsontextValueType:
f.fncs = nil // specially handled in arshal_inlined.go
case tf.Kind() == reflect.Map && tf.Key() == stringType:
case tf.Kind() == reflect.Map && tf.Key().Kind() == reflect.String:
if implementsWhich(tf.Key(), allMethodTypes...) != nil {
serr = orErrorf(serr, t, "inlined map field %s of type %s must have a string key that does not implement marshal or unmarshal methods", sf.Name, tf)
continue // invalid inlined field; treat as ignored
}
f.fncs = lookupArshaler(tf.Elem())
default:
serr = orErrorf(serr, t, "inlined Go struct field %s of type %s must be a Go struct, Go map of string key, or jsontext.Value", sf.Name, tf)
Expand Down
12 changes: 9 additions & 3 deletions fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,17 @@ func TestMakeStructFields(t *testing.T) {
}{},
wantErr: errors.New(`inlined Go struct field A of type map[int]interface {} must be a Go struct, Go map of string key, or jsontext.Value`),
}, {
name: jsontest.Name("InlineUnsupported/MapNamedStringKey"),
name: jsontest.Name("InlineUnsupported/MapTextMarshalerStringKey"),
in: struct {
A map[namedString]any `json:",inline"`
A map[nocaseString]any `json:",inline"`
}{},
wantErr: errors.New(`inlined Go struct field A of type map[json.namedString]interface {} must be a Go struct, Go map of string key, or jsontext.Value`),
wantErr: errors.New(`inlined map field A of type map[json.nocaseString]interface {} must have a string key that does not implement marshal or unmarshal methods`),
}, {
name: jsontest.Name("InlineUnsupported/MapMarshalerV1StringKey"),
in: struct {
A map[stringMarshalEmpty]any `json:",inline"`
}{},
wantErr: errors.New(`inlined map field A of type map[json.stringMarshalEmpty]interface {} must have a string key that does not implement marshal or unmarshal methods`),
}, {
name: jsontest.Name("InlineUnsupported/DoublePointer"),
in: struct {
Expand Down

0 comments on commit 27d2cb3

Please sign in to comment.