diff --git a/fields.go b/fields.go index e6fb6d3..dcda302 100644 --- a/fields.go +++ b/fields.go @@ -51,7 +51,11 @@ type structField struct { var errNoExportedFields = errors.New("Go struct has no exported fields") -func makeStructFields(root reflect.Type) (structFields, *SemanticError) { +func makeStructFields(root reflect.Type) (sf structFields, serr *SemanticError) { + orErrorf := func(serr *SemanticError, t reflect.Type, f string, a ...any) *SemanticError { + return cmp.Or(serr, &SemanticError{GoType: t, Err: fmt.Errorf(f, a...)}) + } + // Setup a queue for a breath-first search. var queueIndex int type queueEntry struct { @@ -80,8 +84,9 @@ func makeStructFields(root reflect.Type) (structFields, *SemanticError) { hasAnyJSONTag = hasAnyJSONTag || hasTag options, ignored, err := parseFieldOptions(sf) if err != nil { - return structFields{}, &SemanticError{GoType: t, Err: err} - } else if ignored { + serr = cmp.Or(serr, &SemanticError{GoType: t, Err: err}) + } + if ignored { continue } hasAnyJSONField = true @@ -94,46 +99,45 @@ func makeStructFields(root reflect.Type) (structFields, *SemanticError) { fieldOptions: options, } if sf.Anonymous && !f.hasName { - f.inline = true // implied by use of Go embedding without an explicit name + if tf := indirectType(f.typ); tf.Kind() != reflect.Struct { + serr = orErrorf(serr, t, "embedded Go struct field %s of non-struct type %s must be explicitly marked as `inline` or given a JSON name", sf.Name, tf) + } else { + f.inline = true // implied by use of Go embedding without an explicit name + } } if f.inline || f.unknown { // Handle an inlined field that serializes to/from // zero or more JSON object members. if f.inline && f.unknown { - err := fmt.Errorf("Go struct field %s cannot have both `inline` and `unknown` specified", sf.Name) - return structFields{}, &SemanticError{GoType: t, Err: err} + serr = orErrorf(serr, t, "Go struct field %s cannot have both `inline` and `unknown` specified", sf.Name) + continue } switch f.fieldOptions { case fieldOptions{name: f.name, quotedName: f.quotedName, inline: true}: case fieldOptions{name: f.name, quotedName: f.quotedName, unknown: true}: default: - err := fmt.Errorf("Go struct field %s cannot have any options other than `inline` or `unknown` specified", sf.Name) - return structFields{}, &SemanticError{GoType: t, Err: err} + serr = orErrorf(serr, t, "Go struct field %s cannot have any options other than `inline` or `unknown` specified", sf.Name) + continue } - // Unwrap one level of pointer indirection similar to how Go - // only allows embedding either T or *T, but not **T. - tf := f.typ - if tf.Kind() == reflect.Pointer && tf.Name() == "" { - tf = tf.Elem() - } // 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, - jsonMarshalerV2Type, jsonMarshalerV1Type, textMarshalerType, + jsonMarshalerV2Type, jsonMarshalerV1Type, textAppenderType, textMarshalerType, jsonUnmarshalerV2Type, jsonUnmarshalerV1Type, textUnmarshalerType, ); which != nil && tf != jsontextValueType { - err := fmt.Errorf("inlined Go struct field %s of type %s must not implement JSON marshal or unmarshal methods", sf.Name, tf) - return structFields{}, &SemanticError{GoType: t, Err: err} + serr = orErrorf(serr, t, "inlined Go struct field %s of type %s must not implement marshal or unmarshal methods", sf.Name, tf) + continue } // Handle an inlined field that serializes to/from // a finite number of JSON object members backed by a Go struct. if tf.Kind() == reflect.Struct { if f.unknown { - err := fmt.Errorf("inlined Go struct field %s of type %s with `unknown` tag must be a Go map of string key or a jsontext.Value", sf.Name, tf) - return structFields{}, &SemanticError{GoType: t, Err: err} + serr = orErrorf(serr, t, "inlined Go struct field %s of type %s with `unknown` tag must be a Go map of string key or a jsontext.Value", sf.Name, tf) + continue } if qe.visitChildren { queue = append(queue, queueEntry{tf, f.index, !seen[tf]}) @@ -150,14 +154,13 @@ func makeStructFields(root reflect.Type) (structFields, *SemanticError) { case tf.Kind() == reflect.Map && tf.Key() == stringType: f.fncs = lookupArshaler(tf.Elem()) default: - err := fmt.Errorf("inlined Go struct field %s of type %s must be a Go struct, Go map of string key, or jsontext.Value", sf.Name, tf) - return structFields{}, &SemanticError{GoType: t, Err: err} + 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) + continue } // Reject multiple inlined fallback fields within the same struct. if inlinedFallbackIndex >= 0 { - err := fmt.Errorf("inlined Go struct fields %s and %s cannot both be a Go map or jsontext.Value", t.Field(inlinedFallbackIndex).Name, sf.Name) - return structFields{}, &SemanticError{GoType: t, Err: err} + serr = orErrorf(serr, t, "inlined Go struct fields %s and %s cannot both be a Go map or jsontext.Value", t.Field(inlinedFallbackIndex).Name, sf.Name) } inlinedFallbackIndex = i @@ -194,15 +197,9 @@ func makeStructFields(root reflect.Type) (structFields, *SemanticError) { f.isEmpty = func(va addressableValue) bool { return va.IsNil() } } - // Reject user-specified names with invalid UTF-8. - if !utf8.ValidString(f.name) { - err := fmt.Errorf("Go struct field %s has JSON object name %q with invalid UTF-8", sf.Name, f.name) - return structFields{}, &SemanticError{GoType: t, Err: err} - } // Reject multiple fields with same name within the same struct. if j, ok := namesIndex[f.name]; ok { - err := fmt.Errorf("Go struct fields %s and %s conflict over JSON object name %q", t.Field(j).Name, sf.Name, f.name) - return structFields{}, &SemanticError{GoType: t, Err: err} + serr = orErrorf(serr, t, "Go struct fields %s and %s conflict over JSON object name %q", t.Field(j).Name, sf.Name, f.name) } namesIndex[f.name] = i @@ -223,7 +220,7 @@ func makeStructFields(root reflect.Type) (structFields, *SemanticError) { // errors returned by errors.New would fail to serialize. isEmptyStruct := t.NumField() == 0 if !isEmptyStruct && !hasAnyJSONTag && !hasAnyJSONField { - return structFields{}, &SemanticError{GoType: t, Err: errNoExportedFields} + serr = cmp.Or(serr, &SemanticError{GoType: t, Err: errNoExportedFields}) } } @@ -293,8 +290,16 @@ func makeStructFields(root reflect.Type) (structFields, *SemanticError) { if n := len(inlinedFallbacks); n == 1 || (n > 1 && len(inlinedFallbacks[0].index) != len(inlinedFallbacks[1].index)) { fs.inlinedFallback = &inlinedFallbacks[0] // dominant inlined fallback field } + return fs, serr +} - return fs, nil +// indirectType unwraps one level of pointer indirection +// similar to how Go only allows embedding either T or *T, but not **T. +func indirectType(t reflect.Type) reflect.Type { + if t.Kind() == reflect.Pointer && t.Name() == "" { + t = t.Elem() + } + return t } // matchFoldedName matches a case-insensitive name depending on the options. @@ -350,19 +355,21 @@ func parseFieldOptions(sf reflect.StructField) (out fieldOptions, ignored bool, // Check whether this field is unexported. if !sf.IsExported() { + ignored = true // In contrast to v1, v2 no longer forwards exported fields from // embedded fields of unexported types since Go reflection does not // allow the same set of operations that are available in normal cases // of purely exported fields. // See https://go.dev/issue/21357 and https://go.dev/issue/24153. if sf.Anonymous { + ignored = indirectType(sf.Type).Kind() != reflect.Struct // for legacy v1 compatibility err = cmp.Or(err, fmt.Errorf("embedded Go struct field %s of an unexported type must be explicitly ignored with a `json:\"-\"` tag", sf.Type.Name())) } // Tag options specified on an unexported field suggests user error. if hasTag { err = cmp.Or(err, fmt.Errorf("unexported Go struct field %s cannot have non-ignored `json:%q` tag", sf.Name, tag)) } - return fieldOptions{}, true, err + return fieldOptions{}, ignored, err } // Determine the JSON member name for this Go field. A user-specified name @@ -384,6 +391,10 @@ func parseFieldOptions(sf reflect.StructField) (out fieldOptions, ignored bool, err = cmp.Or(err, fmt.Errorf("Go struct field %s has malformed `json` tag: %v", sf.Name, err2)) } } + if !utf8.ValidString(opt) { + err = firstError(err, fmt.Errorf("Go struct field %s has JSON object name %q with invalid UTF-8", sf.Name, opt)) + opt = string([]rune(opt)) // replace invalid UTF-8 with utf8.RuneError + } out.hasName = true out.name = opt tag = tag[n:] diff --git a/fields_test.go b/fields_test.go index 61e48e6..de3993d 100644 --- a/fields_test.go +++ b/fields_test.go @@ -199,11 +199,27 @@ func TestMakeStructFields(t *testing.T) { want: structFields{ inlinedFallback: &structField{id: 0, index: []int{2}, typ: T[map[string]jsontext.Value](), fieldOptions: fieldOptions{name: "X", quotedName: `"X"`, unknown: true}}, }, + }, { + name: jsontest.Name("InlinedFallback/InvalidImplicit"), + in: struct { + MapStringAny + }{}, + want: structFields{ + flattened: []structField{ + {id: 0, index: []int{0}, typ: reflect.TypeOf(MapStringAny(nil)), fieldOptions: fieldOptions{name: "MapStringAny", quotedName: `"MapStringAny"`}}, + }, + }, + wantErr: errors.New("embedded Go struct field MapStringAny of non-struct type json.MapStringAny must be explicitly marked as `inline` or given a JSON name"), }, { name: jsontest.Name("InvalidUTF8"), in: struct { Name string `json:"'\\xde\\xad\\xbe\\xef'"` }{}, + want: structFields{ + flattened: []structField{ + {id: 0, index: []int{0}, typ: stringType, fieldOptions: fieldOptions{hasName: true, name: "\u07ad\ufffd\ufffd", quotedName: "\"\u07ad\ufffd\ufffd\""}}, + }, + }, wantErr: errors.New(`Go struct field Name has JSON object name "ޭ\xbe\xef" with invalid UTF-8`), }, { name: jsontest.Name("DuplicateName"), @@ -211,6 +227,7 @@ func TestMakeStructFields(t *testing.T) { A string `json:"same"` B string `json:"same"` }{}, + want: structFields{flattened: []structField{}}, wantErr: errors.New(`Go struct fields A and B conflict over JSON object name "same"`), }, { name: jsontest.Name("BothInlineAndUnknown"), @@ -235,37 +252,37 @@ func TestMakeStructFields(t *testing.T) { in: struct { A struct{ encoding.TextMarshaler } `json:",inline"` }{}, - wantErr: errors.New(`inlined Go struct field A of type struct { encoding.TextMarshaler } must not implement JSON marshal or unmarshal methods`), + wantErr: errors.New(`inlined Go struct field A of type struct { encoding.TextMarshaler } must not implement marshal or unmarshal methods`), }, { name: jsontest.Name("UnknownJSONMarshalerV1"), in: struct { A struct{ MarshalerV1 } `json:",unknown"` }{}, - wantErr: errors.New(`inlined Go struct field A of type struct { json.MarshalerV1 } must not implement JSON marshal or unmarshal methods`), + wantErr: errors.New(`inlined Go struct field A of type struct { json.MarshalerV1 } must not implement marshal or unmarshal methods`), }, { name: jsontest.Name("InlineJSONMarshalerV2"), in: struct { A struct{ MarshalerV2 } `json:",inline"` }{}, - wantErr: errors.New(`inlined Go struct field A of type struct { json.MarshalerV2 } must not implement JSON marshal or unmarshal methods`), + wantErr: errors.New(`inlined Go struct field A of type struct { json.MarshalerV2 } must not implement marshal or unmarshal methods`), }, { name: jsontest.Name("UnknownTextUnmarshaler"), in: struct { A *struct{ encoding.TextUnmarshaler } `json:",unknown"` }{}, - wantErr: errors.New(`inlined Go struct field A of type struct { encoding.TextUnmarshaler } must not implement JSON marshal or unmarshal methods`), + wantErr: errors.New(`inlined Go struct field A of type struct { encoding.TextUnmarshaler } must not implement marshal or unmarshal methods`), }, { name: jsontest.Name("InlineJSONUnmarshalerV1"), in: struct { A *struct{ UnmarshalerV1 } `json:",inline"` }{}, - wantErr: errors.New(`inlined Go struct field A of type struct { json.UnmarshalerV1 } must not implement JSON marshal or unmarshal methods`), + wantErr: errors.New(`inlined Go struct field A of type struct { json.UnmarshalerV1 } must not implement marshal or unmarshal methods`), }, { name: jsontest.Name("UnknownJSONUnmarshalerV2"), in: struct { A struct{ UnmarshalerV2 } `json:",unknown"` }{}, - wantErr: errors.New(`inlined Go struct field A of type struct { json.UnmarshalerV2 } must not implement JSON marshal or unmarshal methods`), + wantErr: errors.New(`inlined Go struct field A of type struct { json.UnmarshalerV2 } must not implement marshal or unmarshal methods`), }, { name: jsontest.Name("UnknownStruct"), in: struct { @@ -302,10 +319,10 @@ func TestMakeStructFields(t *testing.T) { }, { name: jsontest.Name("DuplicateEmbedInline"), in: struct { - MapStringAny + A MapStringAny `json:",inline"` B jsontext.Value `json:",inline"` }{}, - wantErr: errors.New(`inlined Go struct fields MapStringAny and B cannot both be a Go map or jsontext.Value`), + wantErr: errors.New(`inlined Go struct fields A and B cannot both be a Go map or jsontext.Value`), }} for _, tt := range tests { @@ -341,17 +358,15 @@ func TestMakeStructFields(t *testing.T) { } // Reproduce maps in want. - if tt.wantErr == nil { - tt.want.byActualName = make(map[string]*structField) - for i := range tt.want.flattened { - f := &tt.want.flattened[i] - tt.want.byActualName[f.name] = f - } - tt.want.byFoldedName = make(map[string][]*structField) - for i, f := range tt.want.flattened { - foldedName := string(foldName([]byte(f.name))) - tt.want.byFoldedName[foldedName] = append(tt.want.byFoldedName[foldedName], &tt.want.flattened[i]) - } + tt.want.byActualName = make(map[string]*structField) + for i := range tt.want.flattened { + f := &tt.want.flattened[i] + tt.want.byActualName[f.name] = f + } + tt.want.byFoldedName = make(map[string][]*structField) + for i, f := range tt.want.flattened { + foldedName := string(foldName([]byte(f.name))) + tt.want.byFoldedName[foldedName] = append(tt.want.byFoldedName[foldedName], &tt.want.flattened[i]) } // Only compare underlying error to simplify test logic. @@ -411,7 +426,7 @@ func TestParseTagOptions(t *testing.T) { in: struct { unexported }{}, - wantIgnored: true, + wantIgnored: false, wantErr: errors.New("embedded Go struct field unexported of an unexported type must be explicitly ignored with a `json:\"-\"` tag"), }, { name: jsontest.Name("Ignored"),