Lazily report errors in makeStructFields

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:]

fields_test.go

@@ -199,18 +199,35 @@ 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"),
 		in: struct {
 			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"),