-
-
Notifications
You must be signed in to change notification settings - Fork 957
Open
Description
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
- This is common (at least for me) from prior versions because slices already can store
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
magnetikonline
Metadata
Metadata
Assignees
Labels
No labels