The changes in #1981 causes nil optional array properties to be marshaled as null (and thus not valid according to the schema) in some cases due to the changes in pkg/codegen/templates/additional-properties.tmpl, pkg/codegen/templates/union-and-additional-properties.tmpl, and pkg/codegen/templates/union.tmpl.
What seems to be required to trigger this regression is:
- The array property is part of a union or other structure to trigger one of the above templates
- The array property is optional (not in the
required list)
- The array property has
x-go-type-skip-optional-pointer: true
- This is common (at least for me) from prior versions because slices already can store
nil, so an extra pointer layer is unnecessary
The change in the generated code looks something like this:
- if t.Foo != nil {
- object["foo"], err = json.Marshal(t.Foo)
- if err != nil {
- return nil, fmt.Errorf("error marshaling 'foo': %w", err)
- }
+ object["foo"], err = json.Marshal(t.Foo)
+ if err != nil {
+ return nil, fmt.Errorf("error marshaling 'foo': %w", err)
}
I consider this a regression because it causes it to emit a schema-invalid JSON document after parsing a valid one.
Thinking through the logic, I find:
- v2.4.0:
- Skip emitting a property if it is not required and is nil
- v2.5.0:
- Skip emitting a property if it has a pointer layer and is nil
- What I think it should do:
- Skip emitting a property if either:
- It has an optional pointer and is nil (implies it is not required)
- It is not required and is an inherently nillable type (slice, map) and is nil
The changes in #1981 causes nil optional array properties to be marshaled as
null(and thus not valid according to the schema) in some cases due to the changes inpkg/codegen/templates/additional-properties.tmpl,pkg/codegen/templates/union-and-additional-properties.tmpl, andpkg/codegen/templates/union.tmpl.What seems to be required to trigger this regression is:
requiredlist)x-go-type-skip-optional-pointer: truenil, so an extra pointer layer is unnecessaryThe change in the generated code looks something like this:
I consider this a regression because it causes it to emit a schema-invalid JSON document after parsing a valid one.
Thinking through the logic, I find: