Skip to content

Commit a9e2cea

Browse files
Handle double and long shape types in setSDKForScalar for list members (#683)
## Fix: handle `double` and `long` shape types in `setSDKForScalar` for list members ### Issue The QuickSight controller analysis resource fails to compile with errors like: ``` pkg/resource/analysis/sdk.go:6223:30: cannot use f2f6elemf1f0f1iter (variable of type *float64) as float64 value in assignment ``` This affects all `[]float64` list fields in the CRD-to-SDK conversion path, producing 10+ compile errors across the Analysis resource's `sdk.go`. ### Root Cause `setSDKForScalar` in `pkg/generate/code/set_sdk.go` has an `intOrFloat` map that handles narrowing conversions for `"integer"` (int64→int32) and `"float"` (float64→float32) shape types. These types need range checks and casts because the CRD uses 64-bit types while the SDK uses 32-bit types. However, `"double"` (float64) and `"long"` (int64) shape types were not handled. When these appear as list members: - CRD type: `[]*float64` — loop iterator is `*float64` - SDK type: `[]float64` — target element is `float64` The code fell through to the default `else` branch which strips the `*` dereference via `strings.TrimPrefix(setTo, "*")`, generating `elem = iter` instead of `elem = *iter`. ### Fix Added an explicit branch for `"double"` and `"long"` types in `setSDKForScalar`. Unlike `"integer"`/`"float"`, these are already native 64-bit Go types, so no range check or narrowing cast is needed — just correct pointer dereference in list context. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent f0b0805 commit a9e2cea

5 files changed

Lines changed: 22720 additions & 0 deletions

File tree

pkg/generate/code/set_sdk.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1765,6 +1765,16 @@ func setSDKForScalar(
17651765
}
17661766
out += fmt.Sprintf("%stemp := int32(%s)\n", indent, setTo)
17671767
out += fmt.Sprintf("%s%s = &temp\n", indent, targetVarPath)
1768+
} else if shape.Type == "double" || shape.Type == "long" {
1769+
targetVarPath := targetVarName
1770+
if targetFieldName != "" {
1771+
targetVarPath += "." + targetFieldName
1772+
}
1773+
if isListMember {
1774+
out += fmt.Sprintf("%s%s = %s\n", indent, targetVarPath, setTo)
1775+
} else {
1776+
out += fmt.Sprintf("%s%s = %s\n", indent, targetVarPath, strings.TrimPrefix(setTo, "*"))
1777+
}
17681778
} else {
17691779

17701780
targetVarPath := targetVarName

pkg/generate/code/set_sdk_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6656,3 +6656,55 @@ func TestSetSDK_QuickSight_DataSet_Update(t *testing.T) {
66566656
// should also use value-type access
66576657
assert.NotContains(got, "= &f6elemf0f0f0iter.Time")
66586658
}
6659+
6660+
// TestSetSDK_QuickSight_Analysis_Create tests that the SetSDK code generation
6661+
// for the QuickSight Analysis resource correctly handles "double" type list
6662+
// members. The Analysis Definition contains DecimalParameterDeclaration with
6663+
// DefaultValues.StaticValues which is []float64 in the SDK but []*float64 in
6664+
// the CRD. The generated code must dereference the CRD pointer element (*iter)
6665+
// when assigning to the SDK value element.
6666+
func TestSetSDK_QuickSight_Analysis_Create(t *testing.T) {
6667+
assert := assert.New(t)
6668+
require := require.New(t)
6669+
6670+
g := testutil.NewModelForService(t, "quicksight")
6671+
6672+
crd := testutil.GetCRDByName(t, g, "Analysis")
6673+
require.NotNil(crd)
6674+
assert.NotNil(crd.Ops.Create)
6675+
6676+
got, err := code.SetSDK(crd.Config(), crd, model.OpTypeCreate, "r.ko", "res", 1)
6677+
require.NoError(err)
6678+
6679+
// --- Double list members (DecimalParameterDeclaration.DefaultValues.StaticValues) ---
6680+
// The StaticValues field is []float64 in the SDK (value type) but
6681+
// []*float64 in the CRD (pointer type). When iterating over the CRD
6682+
// slice, the loop variable is *float64 and must be dereferenced to
6683+
// assign to the SDK's float64 element variable.
6684+
assert.Contains(got, "[]float64{}")
6685+
// The dereference must be present: elem = *iter
6686+
assert.Contains(got, "= *f2f6elemf1f0f1iter")
6687+
// Must NOT have the broken non-dereferenced assignment
6688+
assert.NotContains(got, "= f2f6elemf1f0f1iter\n")
6689+
}
6690+
6691+
// TestSetSDK_QuickSight_Analysis_Update tests that the same double list
6692+
// dereference fix applies to the Update operation.
6693+
func TestSetSDK_QuickSight_Analysis_Update(t *testing.T) {
6694+
assert := assert.New(t)
6695+
require := require.New(t)
6696+
6697+
g := testutil.NewModelForService(t, "quicksight")
6698+
6699+
crd := testutil.GetCRDByName(t, g, "Analysis")
6700+
require.NotNil(crd)
6701+
assert.NotNil(crd.Ops.Update)
6702+
6703+
got, err := code.SetSDK(crd.Config(), crd, model.OpTypeUpdate, "r.ko", "res", 1)
6704+
require.NoError(err)
6705+
6706+
// Same pattern as Create: double list elements must be dereferenced
6707+
assert.Contains(got, "[]float64{}")
6708+
assert.Contains(got, "= *f2f6elemf1f0f1iter")
6709+
assert.NotContains(got, "= f2f6elemf1f0f1iter\n")
6710+
}

pkg/generate/code/set_sdk_whitebox_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,70 @@ func TestSetSDKForScalar(t *testing.T) {
115115
res.Temperature = &temperatureCopy
116116
`,
117117
},
118+
{
119+
// "double" maps to float64 natively — no range check or cast
120+
// needed. In non-list context, the CRD source is *float64 and
121+
// the SDK target is *float64, so we strip the dereference.
122+
name: "double scalar non-list",
123+
targetFieldName: "Value",
124+
targetVarName: "res",
125+
targetVarType: "structure",
126+
sourceFieldPath: "Value",
127+
sourceVarName: "ko.Spec.Value",
128+
isListMember: false,
129+
shapeRef: &awssdkmodel.ShapeRef{
130+
Shape: &awssdkmodel.Shape{
131+
Type: "double",
132+
},
133+
OriginalMemberName: "Value",
134+
},
135+
indentLevel: 1,
136+
expected: "\tres.Value = ko.Spec.Value\n",
137+
},
138+
{
139+
// In a list context, the CRD source element is *float64 but
140+
// the SDK target element is float64 (value type). The generated
141+
// code must dereference with *.
142+
name: "double scalar list member",
143+
targetFieldName: "",
144+
targetVarName: "f0elem",
145+
targetVarType: "",
146+
sourceFieldPath: "Values",
147+
sourceVarName: "f0iter",
148+
isListMember: true,
149+
shapeRef: &awssdkmodel.ShapeRef{
150+
Shape: &awssdkmodel.Shape{
151+
Type: "double",
152+
ShapeName: "Double",
153+
},
154+
OriginalMemberName: "Values",
155+
OrigShapeName: "Double",
156+
},
157+
indentLevel: 1,
158+
expected: "\tf0elem = *f0iter\n",
159+
},
160+
{
161+
// "long" maps to int64 natively — no range check or cast needed.
162+
// In a list context, the CRD source element is *int64 but the
163+
// SDK target element is int64 (value type). Must dereference.
164+
name: "long scalar list member",
165+
targetFieldName: "",
166+
targetVarName: "f0elem",
167+
targetVarType: "",
168+
sourceFieldPath: "Values",
169+
sourceVarName: "f0iter",
170+
isListMember: true,
171+
shapeRef: &awssdkmodel.ShapeRef{
172+
Shape: &awssdkmodel.Shape{
173+
Type: "long",
174+
ShapeName: "Long",
175+
},
176+
OriginalMemberName: "Values",
177+
OrigShapeName: "Long",
178+
},
179+
indentLevel: 1,
180+
expected: "\tf0elem = *f0iter\n",
181+
},
118182
}
119183

120184
for _, tc := range testCases {

0 commit comments

Comments
 (0)