Skip to content

Commit 9b465a5

Browse files
committed
Move IsPointer logic to FieldTypes struct.
Rather than deciding which generated fields are represented as pointers in different places in the code, this patch adds an IsPointer() method to TypeFields to consolidate that logic.
1 parent 0f87c07 commit 9b465a5

3 files changed

Lines changed: 95 additions & 30 deletions

File tree

internal/generate/types.go

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,45 @@ func (f TypeFields) StructTag() string {
9898
return fmt.Sprintf("`json:\"%s\" yaml:\"%s\"`", tagValue, tagValue)
9999
}
100100

101+
// IsPointer returns whether this field should be a pointer type.
102+
// This consolidates the field-level pointer decision logic:
103+
// - Required + nullable fields (Omicron API pattern) need pointers
104+
// - Types in the nullable() exception list need pointers
105+
//
106+
// Note: Primitive type pointer logic (int, bool, time) is handled in
107+
// schemaValueToGoType() because those types can appear in nested contexts
108+
// (map values, array items) that don't go through TypeFields.
109+
func (f TypeFields) IsPointer() bool {
110+
if f.Schema == nil {
111+
return false
112+
}
113+
114+
v := f.Schema.Value
115+
116+
// Required + nullable fields should be pointers (Omicron API pattern):
117+
// they can be set to a null value, but they must not be omitted.
118+
// The SDK presents these fields as optional and serializes them to
119+
// `null` if not provided.
120+
if f.Required && v.Nullable {
121+
return true
122+
}
123+
124+
// Check hardcoded nullable exceptions (upstream API workarounds)
125+
if slices.Contains(nullable(), f.Type) {
126+
return true
127+
}
128+
129+
return false
130+
}
131+
132+
// GoType returns the Go type for this field, with pointer prefix if needed.
133+
func (f TypeFields) GoType() string {
134+
if f.IsPointer() && !strings.HasPrefix(f.Type, "*") {
135+
return "*" + f.Type
136+
}
137+
return f.Type
138+
}
139+
101140
// EnumTemplate holds the information for enum types
102141
type EnumTemplate struct {
103142
Description string
@@ -388,7 +427,7 @@ func writeTypes(f *os.File, typeCollection []TypeTemplate, typeValidationCollect
388427
if desc := ft.Description(); desc != "" {
389428
fmt.Fprintf(f, "\t%s\n", splitDocString(desc))
390429
}
391-
fmt.Fprintf(f, "\t%s %s %s\n", ft.Name, ft.Type, ft.StructTag())
430+
fmt.Fprintf(f, "\t%s %s %s\n", ft.Name, ft.GoType(), ft.StructTag())
392431
}
393432
fmt.Fprint(f, "}\n")
394433
}
@@ -569,29 +608,15 @@ func createTypeObject(schema *openapi3.Schema, name, typeName, description strin
569608
}
570609
}
571610

572-
// Omicron includes fields that are both required and nullable:
573-
// they can be set to a null value, but they must not be
574-
// omitted. The sdk should present these fields to the user as
575-
// optional, and serialize them to `null` if not provided.
576611
isRequired := slices.Contains(required, k)
577-
isRequiredNullable := v.Value.Nullable && isRequired
578-
if slices.Contains(nullable(), typeName) || isRequiredNullable {
579-
// We may have already decided to use a pointer. For
580-
// example, convertToValidGoType always uses pointers
581-
// for ints and bools. Prefix the type with "*", unless
582-
// we've already made it a pointer upstream.
583-
if !strings.HasPrefix(typeName, "*") {
584-
typeName = fmt.Sprintf("*%s", typeName)
585-
}
586-
}
587-
588612
field := TypeFields{
589613
Name: strcase.ToCamel(k),
590614
Type: typeName,
591615
MarshalKey: k,
592616
Schema: v,
593617
Required: isRequired,
594618
}
619+
// Note: pointer prefix is applied by TypeFields.GoType() based on IsPointer()
595620

596621
fields = append(fields, field)
597622

internal/generate/types_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,56 @@ func TestTypeFields_StructTag(t *testing.T) {
188188
})
189189
}
190190

191+
func TestTypeFields_IsPointer(t *testing.T) {
192+
tests := []struct {
193+
name string
194+
field TypeFields
195+
expected bool
196+
}{
197+
{
198+
name: "nil schema",
199+
field: TypeFields{Name: "Body", Schema: nil},
200+
expected: false,
201+
},
202+
{
203+
name: "nullable required",
204+
field: TypeFields{
205+
Name: "Config",
206+
Type: "SomeConfig",
207+
Schema: &openapi3.SchemaRef{Value: &openapi3.Schema{Type: &openapi3.Types{"object"}, Nullable: true}},
208+
Required: true,
209+
},
210+
expected: true,
211+
},
212+
{
213+
name: "nullable not required",
214+
field: TypeFields{
215+
Name: "Config",
216+
Type: "SomeConfig",
217+
Schema: &openapi3.SchemaRef{Value: &openapi3.Schema{Type: &openapi3.Types{"object"}, Nullable: true}},
218+
Required: false,
219+
},
220+
expected: false,
221+
},
222+
{
223+
name: "not nullable required",
224+
field: TypeFields{
225+
Name: "Config",
226+
Type: "SomeConfig",
227+
Schema: &openapi3.SchemaRef{Value: &openapi3.Schema{Type: &openapi3.Types{"object"}, Nullable: false}},
228+
Required: true,
229+
},
230+
expected: false,
231+
},
232+
}
233+
234+
for _, tt := range tests {
235+
t.Run(tt.name, func(t *testing.T) {
236+
assert.Equal(t, tt.expected, tt.field.IsPointer())
237+
})
238+
}
239+
}
240+
191241
func Test_createTypeObject(t *testing.T) {
192242
typesSpec := openapi3.Schema{
193243
Required: []string{"type"},

internal/generate/utils.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,13 @@ func isNullableArray(v *openapi3.SchemaRef) bool {
5959

6060
// formatStringType converts a string schema to a valid Go type.
6161
func formatStringType(t *openapi3.Schema) string {
62-
var format string
6362
switch t.Format {
64-
case "date-time":
65-
format = "*time.Time"
66-
case "date":
67-
format = "*time.Time"
68-
case "time":
69-
format = "*time.Time"
63+
case "date-time", "date", "time":
64+
// Time types need pointers for JSON marshaling
65+
return "*time.Time"
7066
default:
71-
format = "string"
67+
return "string"
7268
}
73-
74-
return format
7569
}
7670

7771
// toLowerFirstLetter returns the given string with the first letter converted to lower case.
@@ -201,10 +195,6 @@ func schemaValueToGoType(schemaValue *openapi3.Schema, property string) string {
201195
return fmt.Sprintf("[]%v", schemaType)
202196
}
203197

204-
if schemaValue.Type.Is("object") {
205-
return "object"
206-
}
207-
208198
fmt.Printf("[WARN] TODO: handle type %q for %q, marking as any for now\n", schemaValue.Type, property)
209199
return "any"
210200
}

0 commit comments

Comments
 (0)