refactor: use named field policies in codegen overrides

Author: dmacvicar

AGENTS.md

@@ -195,6 +195,8 @@ Terraform state reflects what the user cares about, not the entire API response.
 
 For generated XML -> model conversions, see `internal/codegen/README.md`, especially the "Preserve User Intent For Optional Nested Objects" section. That generator-level rule is authoritative for optional nested object readback behavior.
 
+For codegen overrides, prefer the policy layer in `internal/codegen/policy/field_policy.go`. Add exact field overrides there using named policy functions rather than adding new struct/field special cases in generator templates. Template branches should stay generic unless the behavior is truly structural and cannot be expressed as field policy.
+
 **Computed** (not Optional) - Always read from API
 - Examples: `id`, `key`, `allocation`, `physical`
 - `model.Key = types.StringValue(xml.Key)`

internal/codegen/README.md

@@ -100,19 +100,29 @@ Examples:
 
 ### Override strategy
 
-When the default rules are not enough, use explicit overrides keyed by full Terraform or XML path rather than helper functions like `isUserManagedFooStruct`.
+When the default rules are not enough, use explicit overrides keyed by exact field identity rather than helper functions like `isUserManagedFooStruct`.
+
+Prefer declaring overrides in the policy layer as a registry of named policy functions, for example:
+
+- `StoragePool.capacity` → `policyComputedReportedField`, `policyUseStateForUnknown`
+- `DomainCPU.mode` → `policyPreservePlannedValueOnReadbackOmit`
+
+This keeps policy declarative and reviewable. Avoid encoding overrides as ad hoc conditionals in converter templates.
 
 Good override targets:
 
 - `storage_pool.capacity`
 - `storage_pool.allocation`
 - `storage_volume.physical`
+- `DomainCPU.mode`
+- `DomainGraphicSpice.listen`
 
 Avoid:
 
 - Struct-name allowlists for one field
 - Field-name heuristics that ignore nesting scope
 - Mixing reflection logic with provider semantics in the same function
+- Adding new `if struct == ... && field == ...` branches in generator templates when a policy override can express the behavior
 
 This keeps the generator predictable, makes exceptions easy to audit, and prevents the parser from turning into a collection of special cases.
 

internal/codegen/policy/field_policy.go

@@ -2,6 +2,41 @@ package policy
 
 import "github.com/dmacvicar/terraform-provider-libvirt/v2/internal/codegen/generator"
 
+type fieldPolicy func(*generator.FieldIR)
+
+var fieldPolicies = map[string][]fieldPolicy{
+	"StoragePool.capacity": {
+		policyComputedReportedField,
+		policyUseStateForUnknown,
+		policyDisablePreserveUserIntent,
+	},
+	"StoragePool.allocation": {
+		policyComputedReportedField,
+		policyDisablePreserveUserIntent,
+	},
+	"StoragePool.available": {
+		policyComputedReportedField,
+		policyDisablePreserveUserIntent,
+	},
+	"StorageVolume.capacity": {
+		policyComputedReportedField,
+	},
+	"StorageVolume.allocation": {
+		policyComputedReportedField,
+		policyDisablePreserveUserIntent,
+	},
+	"StorageVolume.physical": {
+		policyComputedReportedField,
+		policyDisablePreserveUserIntent,
+	},
+	"DomainCPU.mode": {
+		policyPreservePlannedValueOnReadbackOmit,
+	},
+	"DomainGraphicSpice.listen": {
+		policyPreservePlannedValueOnReadbackOmit,
+	},
+}
+
 // ApplyFieldPolicies mutates the IR with Terraform-specific schema/conversion
 // semantics after structural reflection is complete.
 func ApplyFieldPolicies(structs []*generator.StructIR) {
@@ -18,8 +53,7 @@ func applyStructPolicies(s *generator.StructIR) {
 
 		applyTopLevelIdentityPolicy(s, field)
 		applyTopLevelImmutabilityPolicy(s, field)
-		applyReportedFieldPolicy(s, field)
-		applyReadbackPreservationPolicy(s, field)
+		applyExplicitFieldPolicies(s, field)
 	}
 }
 
@@ -58,50 +92,39 @@ func applyTopLevelImmutabilityPolicy(s *generator.StructIR, field *generator.Fie
 	}
 }
 
-func applyReportedFieldPolicy(s *generator.StructIR, field *generator.FieldIR) {
+func applyExplicitFieldPolicies(s *generator.StructIR, field *generator.FieldIR) {
 	if field.IsFlattenedUnit {
 		return
 	}
 
-	switch s.Name {
-	case "StoragePool":
-		switch field.TFName {
-		case "capacity":
-			field.IsComputed = true
-			field.IsOptional = false
-			field.IsRequired = false
-			field.PlanModifier = "UseStateForUnknown"
-			field.PreserveUserIntent = false
-		case "allocation", "available":
-			field.IsComputed = true
-			field.IsOptional = false
-			field.IsRequired = false
-			field.PreserveUserIntent = false
-		}
-	case "StorageVolume":
-		switch field.TFName {
-		case "capacity":
-			field.IsComputed = true
-			field.IsOptional = false
-			field.IsRequired = false
-		case "allocation", "physical":
-			field.IsComputed = true
-			field.IsOptional = false
-			field.IsRequired = false
-			field.PreserveUserIntent = false
-		}
-	}
+	key := fieldOverrideKey(s, field)
+	applyPolicies(field, fieldPolicies[key])
 }
 
-func applyReadbackPreservationPolicy(s *generator.StructIR, field *generator.FieldIR) {
-	switch s.Name {
-	case "DomainCPU":
-		if field.TFName == "mode" {
-			field.PreservePlannedValueOnReadbackOmit = true
-		}
-	case "DomainGraphicSpice":
-		if field.TFName == "listen" {
-			field.PreservePlannedValueOnReadbackOmit = true
-		}
+func fieldOverrideKey(s *generator.StructIR, field *generator.FieldIR) string {
+	return s.Name + "." + field.TFName
+}
+
+func applyPolicies(field *generator.FieldIR, policies []fieldPolicy) {
+	for _, policy := range policies {
+		policy(field)
 	}
 }
+
+func policyComputedReportedField(field *generator.FieldIR) {
+	field.IsComputed = true
+	field.IsOptional = false
+	field.IsRequired = false
+}
+
+func policyUseStateForUnknown(field *generator.FieldIR) {
+	field.PlanModifier = "UseStateForUnknown"
+}
+
+func policyDisablePreserveUserIntent(field *generator.FieldIR) {
+	field.PreserveUserIntent = false
+}
+
+func policyPreservePlannedValueOnReadbackOmit(field *generator.FieldIR) {
+	field.PreservePlannedValueOnReadbackOmit = true
+}

internal/codegen/policy/field_policy_test.go

@@ -76,3 +76,56 @@ func TestApplyFieldPoliciesMarksReadbackPreservationOverrides(t *testing.T) {
 		t.Fatal("expected DomainGraphicSpice.listen to preserve planned value on omitted readback")
 	}
 }
+
+func TestApplyFieldPoliciesMarksReportedFieldOverrides(t *testing.T) {
+	structs := []*generator.StructIR{
+		{
+			Name: "StoragePool",
+			Fields: []*generator.FieldIR{
+				{TFName: "capacity", IsOptional: true, IsRequired: true, PreserveUserIntent: true},
+				{TFName: "allocation", IsOptional: true, IsRequired: true, PreserveUserIntent: true},
+			},
+		},
+		{
+			Name: "StorageVolume",
+			Fields: []*generator.FieldIR{
+				{TFName: "capacity", IsOptional: true, IsRequired: true},
+				{TFName: "physical", IsOptional: true, IsRequired: true, PreserveUserIntent: true},
+			},
+		},
+	}
+
+	ApplyFieldPolicies(structs)
+
+	poolCapacity := structs[0].Fields[0]
+	if !poolCapacity.IsComputed || poolCapacity.IsOptional || poolCapacity.IsRequired {
+		t.Fatal("expected StoragePool.capacity to be computed-only after override")
+	}
+	if poolCapacity.PlanModifier != "UseStateForUnknown" {
+		t.Fatalf("expected StoragePool.capacity plan modifier UseStateForUnknown, got %q", poolCapacity.PlanModifier)
+	}
+	if poolCapacity.PreserveUserIntent {
+		t.Fatal("expected StoragePool.capacity to disable PreserveUserIntent")
+	}
+
+	poolAllocation := structs[0].Fields[1]
+	if !poolAllocation.IsComputed || poolAllocation.IsOptional || poolAllocation.IsRequired {
+		t.Fatal("expected StoragePool.allocation to be computed-only after override")
+	}
+	if poolAllocation.PreserveUserIntent {
+		t.Fatal("expected StoragePool.allocation to disable PreserveUserIntent")
+	}
+
+	volumeCapacity := structs[1].Fields[0]
+	if !volumeCapacity.IsComputed || volumeCapacity.IsOptional || volumeCapacity.IsRequired {
+		t.Fatal("expected StorageVolume.capacity to be computed-only after override")
+	}
+
+	volumePhysical := structs[1].Fields[1]
+	if !volumePhysical.IsComputed || volumePhysical.IsOptional || volumePhysical.IsRequired {
+		t.Fatal("expected StorageVolume.physical to be computed-only after override")
+	}
+	if volumePhysical.PreserveUserIntent {
+		t.Fatal("expected StorageVolume.physical to disable PreserveUserIntent")
+	}
+}