Drop flat structs for complex union types.

Drop the flat struct pattern for representing tagged unions with multiple value
types, and instead use interfaces with variant types.

Note that we also fix pre-existing bugs related to marshalling and
unmarshalling interface wrapper types. We now correctly serialize nil
interfaces to `null`, rather than `{"type": ""}`, and correspondingly unmarshal
null oneOf fields to `nil`.

Author: jmcarp

DESIGN.md

@@ -23,14 +23,12 @@ to different `serde` tagging strategies, and we handle each of them differently.
 
 ### Tagged union
 
-When a `oneOf` has:
+When a `oneOf` has exactly one discriminator property (a field with a single enum value per
+variant), we generate an **interface with variant types** pattern. This covers both
+`serde(tag = "type", content = "value")` unions (where variants carry data in a `value` field) and
+`serde(tag = "type")` unions (where variant fields are inlined at the top level).
 
-1. Exactly one discriminator property (a field with a single enum value per variant)
-2. Exactly one multi-type property (a field whose type varies across variants)
-
-We generate an **interface with variant wrapper types** pattern.
-
-**Example: `PrivateIpStack`**
+**Example: `PrivateIpStack`** (using serde's `tag` and `content`)
 
 In Rust, `PrivateIpStack` is defined as:
 
@@ -210,13 +208,7 @@ confusing to end users than consistent use of wrappers.
 Note: we can reconsider this choice if we're able to drop the use of `interface{}` types and
 pointers to primitives for variants, and if we're confident that those cases won't emerge again.
 
-### Discriminator with multiple value fields
-
-When a `oneOf` has a discriminator field and _multiple_ value fields, we use a flat struct that
-contains all properties from all variants. Properties that have different types across variants
-become `any`.
-
-**Example: `DiskSource`**
+**Example: `DiskSource`** (using serde's `tag` only)
 
 In Rust, `DiskSource` is defined as:
 
@@ -253,20 +245,65 @@ DiskSource:
         block_size: { $ref: "#/components/schemas/BlockSize" }
 ```
 
-This has a discriminator (`type`) but no multi-type property. Each variant has different fields
-(`block_size`, `snapshot_id`, `image_id`), not different types for the same field. So we generate a
-flat struct:
+This has a discriminator (`type`) and each variant has different fields. Each variant struct contains
+only its own fields (without the discriminator):
 
 ```go
+type diskSourceVariant interface {
+    isDiskSourceVariant()
+}
+
+type DiskSourceBlank struct {
+    BlockSize BlockSize `json:"block_size,omitempty"`
+}
+func (DiskSourceBlank) isDiskSourceVariant() {}
+
+type DiskSourceSnapshot struct {
+    SnapshotId string `json:"snapshot_id,omitempty"`
+}
+func (DiskSourceSnapshot) isDiskSourceVariant() {}
+
+type DiskSourceImage struct {
+    ImageId string `json:"image_id,omitempty"`
+}
+func (DiskSourceImage) isDiskSourceVariant() {}
+
+type DiskSourceImportingBlocks struct {
+    BlockSize BlockSize `json:"block_size,omitempty"`
+}
+func (DiskSourceImportingBlocks) isDiskSourceVariant() {}
+
 type DiskSource struct {
-    BlockSize  BlockSize      `json:"block_size,omitempty"`
-    Type       DiskSourceType `json:"type,omitempty"`
-    SnapshotId string         `json:"snapshot_id,omitempty"`
-    ImageId    string         `json:"image_id,omitempty"`
+    Value diskSourceVariant
 }
 ```
 
-If any property had different types across variants, it would become `any`.
+**Usage examples:**
+
+```go
+// Creating a disk from a snapshot
+params := oxide.DiskCreateParams{
+    Body: &oxide.DiskCreate{
+        Name: "my-disk",
+        DiskSource: oxide.DiskSource{
+            Value: &oxide.DiskSourceSnapshot{
+                SnapshotId: "snapshot-uuid",
+            },
+        },
+    },
+}
+```
+
+```go
+// Reading a disk source from the API
+disk, _ := client.DiskView(ctx, params)
+switch v := disk.DiskSource.Value.(type) {
+case *oxide.DiskSourceSnapshot:
+    fmt.Printf("From snapshot: %s\n", v.SnapshotId)
+case *oxide.DiskSourceImage:
+    fmt.Printf("From image: %s\n", v.ImageId)
+}
+```
 
 ### Untagged union
 

internal/generate/exceptions.go

@@ -22,10 +22,7 @@ func emptyTypes() []string {
 }
 
 func nullable() []string {
-	// TODO: This type has a nested required "Type" field, which hinders
-	// the usage of this type. Remove when this is fixed in the upstream API
 	return []string{
-		"InstanceDiskAttachment",
 		"LldpLinkConfig",
 		"TxEqConfig",
 		"TxEqConfig2",

internal/generate/templates/union_tagged.go.tpl

@@ -10,6 +10,9 @@ func (v {{.TypeName}}) {{.DiscriminatorMethod}}() {{.DiscriminatorType}} {
 }
 
 func (v *{{.TypeName}}) UnmarshalJSON(data []byte) error {
+	if string(data) == "null" {
+		return nil
+	}
 	type discriminator struct {
 		Type string `json:"{{.Discriminator}}"`
 	}
@@ -35,20 +38,21 @@ func (v *{{.TypeName}}) UnmarshalJSON(data []byte) error {
 }
 
 func (v {{.TypeName}}) MarshalJSON() ([]byte, error) {
+	if v.{{.ValueFieldName}} == nil {
+		return []byte("null"), nil
+	}
 	m := make(map[string]any)
 	m["{{.Discriminator}}"] = v.{{.DiscriminatorMethod}}()
-	if v.{{.ValueFieldName}} != nil {
-		valueBytes, err := json.Marshal(v.{{.ValueFieldName}})
-		if err != nil {
-			return nil, err
-		}
-		var valueMap map[string]any
-		if err := json.Unmarshal(valueBytes, &valueMap); err != nil {
-			return nil, err
-		}
-		for k, val := range valueMap {
-			m[k] = val
-		}
+	valueBytes, err := json.Marshal(v.{{.ValueFieldName}})
+	if err != nil {
+		return nil, err
+	}
+	var valueMap map[string]any
+	if err := json.Unmarshal(valueBytes, &valueMap); err != nil {
+		return nil, err
+	}
+	for k, val := range valueMap {
+		m[k] = val
 	}
 	return json.Marshal(m)
 }

internal/generate/test_utils/types_output

@@ -19,31 +19,109 @@ type DiskIdentifier struct {
 }
 
 
+// diskSourceVariant is implemented by DiskSource variants.
+type diskSourceVariant interface {
+	isDiskSourceVariant()
+}
+
+
 // DiskSourceType is the type definition for a DiskSourceType.
 type DiskSourceType string
 
-// DiskSourceSnapshot is create a disk from a disk snapshot
+// DiskSourceSnapshot is a variant of DiskSource.
 type DiskSourceSnapshot struct {
 	SnapshotId string `json:"snapshot_id,omitempty" yaml:"snapshot_id,omitempty"`
-	Type DiskSourceType `json:"type,omitempty" yaml:"type,omitempty"`
 }
 
+func (DiskSourceSnapshot) isDiskSourceVariant() {}
 
-// DiskSourceImage is create a disk from a project image
+
+// DiskSourceImage is a variant of DiskSource.
 type DiskSourceImage struct {
 	ImageId string `json:"image_id,omitempty" yaml:"image_id,omitempty"`
-	Type DiskSourceType `json:"type,omitempty" yaml:"type,omitempty"`
 }
 
+func (DiskSourceImage) isDiskSourceVariant() {}
+
 
 // DiskSource is the type definition for a DiskSource.
 type DiskSource struct {
-	// SnapshotId is the type definition for a SnapshotId.
-	SnapshotId string `json:"snapshot_id,omitempty" yaml:"snapshot_id,omitempty"`
-	// Type is the type definition for a Type.
-	Type DiskSourceType `json:"type,omitempty" yaml:"type,omitempty"`
-	// ImageId is the type definition for a ImageId.
-	ImageId string `json:"image_id,omitempty" yaml:"image_id,omitempty"`
+	Value diskSourceVariant 
+}
+
+func (v DiskSource) Type() DiskSourceType {
+	switch v.Value.(type) {
+	case DiskSourceSnapshot, *DiskSourceSnapshot:
+		return DiskSourceTypeSnapshot
+	case DiskSourceImage, *DiskSourceImage:
+		return DiskSourceTypeImage
+	default:
+		return ""
+	}
+}
+
+func (v *DiskSource) UnmarshalJSON(data []byte) error {
+	if string(data) == "null" {
+		return nil
+	}
+	type discriminator struct {
+		Type string `json:"type"`
+	}
+	var d discriminator
+	if err := json.Unmarshal(data, &d); err != nil {
+		return err
+	}
+
+	var value diskSourceVariant
+	switch d.Type {
+	case "snapshot":
+		value = &DiskSourceSnapshot{}
+	case "image":
+		value = &DiskSourceImage{}
+	default:
+		return fmt.Errorf("unknown variant %q, expected 'snapshot' or 'image'", d.Type)
+	}
+	if err := json.Unmarshal(data, value); err != nil {
+		return err
+	}
+	v.Value = value
+	return nil
+}
+
+func (v DiskSource) MarshalJSON() ([]byte, error) {
+	if v.Value == nil {
+		return []byte("null"), nil
+	}
+	m := make(map[string]any)
+	m["type"] = v.Type()
+	valueBytes, err := json.Marshal(v.Value)
+	if err != nil {
+		return nil, err
+	}
+	var valueMap map[string]any
+	if err := json.Unmarshal(valueBytes, &valueMap); err != nil {
+		return nil, err
+	}
+	for k, val := range valueMap {
+		m[k] = val
+	}
+	return json.Marshal(m)
+}
+
+
+
+// AsSnapshot attempts to convert the DiskSource to a DiskSourceSnapshot.
+// Returns the variant and true if the conversion succeeded, nil and false otherwise.
+func (v DiskSource) AsSnapshot() (*DiskSourceSnapshot, bool) {
+	val, ok := v.Value.(*DiskSourceSnapshot)
+	return val, ok
+}
+
+// AsImage attempts to convert the DiskSource to a DiskSourceImage.
+// Returns the variant and true if the conversion succeeded, nil and false otherwise.
+func (v DiskSource) AsImage() (*DiskSourceImage, bool) {
+	val, ok := v.Value.(*DiskSourceImage)
+	return val, ok
 }