Skip to content

Commit b76c182

Browse files
authored
fix(cfn2ts): doesn't handle property types with the same type as a primitive type (#25460)
We currently do not account for cases where a property type can have a complex type that matches one of our primitive types. For example, [AWS::IoT::TopicRule.LocationAction](https://github.com/aws/aws-cdk/blob/a0ad49df7b2536d800b4890ae0116e6ce26e6c55/packages/@aws-cdk/cfnspec/spec-source/specification/000_cfn/000_official/000_AWS_IoT.json#L1437-L1442) has a property `Timestamp` with a Type of [Timestamp](https://github.com/aws/aws-cdk/blob/a0ad49df7b2536d800b4890ae0116e6ce26e6c55/packages/@aws-cdk/cfnspec/spec-source/specification/000_cfn/000_official/000_AWS_IoT.json#L1727-L1742) When cfn2ts generates the TypeScript code it thinks that this is referring to the `PrimitiveType` `Timestamp` and coverts it to a `Date`. After running `gen` with these changes the only differences in the cfnspecs were `iot.generated.ts` ```diff @@ -10134,7 +10134,7 @@ export namespace CfnTopicRule { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iot-topicrule-locationaction.html#cfn-iot-topicrule-locationaction-timestamp */ - readonly timestamp?: Date | cdk.IResolvable; + readonly timestamp?: CfnTopicRule.TimestampProperty | cdk.IResolvable; /** * The name of the tracker resource in Amazon Location in which the location is updated. * @@ -10165,7 +10165,7 @@ function CfnTopicRule_LocationActionPropertyValidator(properties: any): cdk.Vali errors.collect(cdk.propertyValidator('longitude', cdk.validateString)(properties.longitude)); errors.collect(cdk.propertyValidator('roleArn', cdk.requiredValidator)(properties.roleArn)); errors.collect(cdk.propertyValidator('roleArn', cdk.validateString)(properties.roleArn)); - errors.collect(cdk.propertyValidator('timestamp', cdk.validateDate)(properties.timestamp)); + errors.collect(cdk.propertyValidator('timestamp', CfnTopicRule_TimestampPropertyValidator)(properties.timestamp)); errors.collect(cdk.propertyValidator('trackerName', cdk.requiredValidator)(properties.trackerName)); errors.collect(cdk.propertyValidator('trackerName', cdk.validateString)(properties.trackerName)); return errors.wrap('supplied properties not correct for "LocationActionProperty"'); @@ -10187,7 +10187,7 @@ function cfnTopicRuleLocationActionPropertyToCloudFormation(properties: any): an Latitude: cdk.stringToCloudFormation(properties.latitude), Longitude: cdk.stringToCloudFormation(properties.longitude), RoleArn: cdk.stringToCloudFormation(properties.roleArn), - Timestamp: cdk.dateToCloudFormation(properties.timestamp), + Timestamp: cfnTopicRuleTimestampPropertyToCloudFormation(properties.timestamp), TrackerName: cdk.stringToCloudFormation(properties.trackerName), }; } @@ -10206,7 +10206,7 @@ function CfnTopicRuleLocationActionPropertyFromCloudFormation(properties: any): ret.addPropertyResult('latitude', 'Latitude', cfn_parse.FromCloudFormation.getString(properties.Latitude)); ret.addPropertyResult('longitude', 'Longitude', cfn_parse.FromCloudFormation.getString(properties.Longitude)); ret.addPropertyResult('roleArn', 'RoleArn', cfn_parse.FromCloudFormation.getString(properties.RoleArn)); - ret.addPropertyResult('timestamp', 'Timestamp', properties.Timestamp != null ? cfn_parse.FromCloudFormation.getDate(properties.Timestamp) : undefined); + ret.addPropertyResult('timestamp', 'Timestamp', properties.Timestamp != null ? CfnTopicRuleTimestampPropertyFromCloudFormation(properties.Timestamp) : undefined); ret.addPropertyResult('trackerName', 'TrackerName', cfn_parse.FromCloudFormation.getString(properties.TrackerName)); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; ``` `sagemaker.generated.ts` ```diff @@ -2053,7 +2053,7 @@ export namespace CfnDataQualityJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-dataqualityjobdefinition-datasetformat.html#cfn-sagemaker-dataqualityjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnDataQualityJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnDataQualityJobDefinition.DatasetFormatProperty.Parquet` * @@ -2077,7 +2077,7 @@ function CfnDataQualityJobDefinition_DatasetFormatPropertyValidator(properties: errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnDataQualityJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnDataQualityJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -2095,7 +2095,7 @@ function cfnDataQualityJobDefinitionDatasetFormatPropertyToCloudFormation(proper CfnDataQualityJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnDataQualityJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnDataQualityJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -2111,7 +2111,7 @@ function CfnDataQualityJobDefinitionDatasetFormatPropertyFromCloudFormation(prop } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnDataQualityJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnDataQualityJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnDataQualityJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -11470,7 +11470,7 @@ export namespace CfnModelBiasJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-modelbiasjobdefinition-datasetformat.html#cfn-sagemaker-modelbiasjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnModelBiasJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnModelBiasJobDefinition.DatasetFormatProperty.Parquet` * @@ -11494,7 +11494,7 @@ function CfnModelBiasJobDefinition_DatasetFormatPropertyValidator(properties: an errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnModelBiasJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnModelBiasJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -11512,7 +11512,7 @@ function cfnModelBiasJobDefinitionDatasetFormatPropertyToCloudFormation(properti CfnModelBiasJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnModelBiasJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnModelBiasJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -11528,7 +11528,7 @@ function CfnModelBiasJobDefinitionDatasetFormatPropertyFromCloudFormation(proper } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnModelBiasJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnModelBiasJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnModelBiasJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -15402,7 +15402,7 @@ export namespace CfnModelExplainabilityJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-modelexplainabilityjobdefinition-datasetformat.html#cfn-sagemaker-modelexplainabilityjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnModelExplainabilityJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnModelExplainabilityJobDefinition.DatasetFormatProperty.Parquet` * @@ -15426,7 +15426,7 @@ function CfnModelExplainabilityJobDefinition_DatasetFormatPropertyValidator(prop errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnModelExplainabilityJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnModelExplainabilityJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -15444,7 +15444,7 @@ function cfnModelExplainabilityJobDefinitionDatasetFormatPropertyToCloudFormatio CfnModelExplainabilityJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnModelExplainabilityJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnModelExplainabilityJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -15460,7 +15460,7 @@ function CfnModelExplainabilityJobDefinitionDatasetFormatPropertyFromCloudFormat } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnModelExplainabilityJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnModelExplainabilityJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnModelExplainabilityJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -20808,7 +20808,7 @@ export namespace CfnModelQualityJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-modelqualityjobdefinition-datasetformat.html#cfn-sagemaker-modelqualityjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnModelQualityJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnModelQualityJobDefinition.DatasetFormatProperty.Parquet` * @@ -20832,7 +20832,7 @@ function CfnModelQualityJobDefinition_DatasetFormatPropertyValidator(properties: errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnModelQualityJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnModelQualityJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -20850,7 +20850,7 @@ function cfnModelQualityJobDefinitionDatasetFormatPropertyToCloudFormation(prope CfnModelQualityJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnModelQualityJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnModelQualityJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -20866,7 +20866,7 @@ function CfnModelQualityJobDefinitionDatasetFormatPropertyFromCloudFormation(pro } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnModelQualityJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnModelQualityJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnModelQualityJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -22693,7 +22693,7 @@ export namespace CfnMonitoringSchedule { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-monitoringschedule-datasetformat.html#cfn-sagemaker-monitoringschedule-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnMonitoringSchedule.JsonProperty | cdk.IResolvable; /** * `CfnMonitoringSchedule.DatasetFormatProperty.Parquet` * @@ -22717,7 +22717,7 @@ function CfnMonitoringSchedule_DatasetFormatPropertyValidator(properties: any): errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnMonitoringSchedule_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnMonitoringSchedule_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -22735,7 +22735,7 @@ function cfnMonitoringScheduleDatasetFormatPropertyToCloudFormation(properties: CfnMonitoringSchedule_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnMonitoringScheduleCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnMonitoringScheduleJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -22751,7 +22751,7 @@ function CfnMonitoringScheduleDatasetFormatPropertyFromCloudFormation(properties } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnMonitoringSchedule.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnMonitoringScheduleCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnMonitoringScheduleJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; ``` Closes #22732 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 1f4c4fa commit b76c182

File tree

2 files changed

+36
-12
lines changed

2 files changed

+36
-12
lines changed

Diff for: tools/@aws-cdk/cfn2ts/lib/genspec.ts

+14-6
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,18 @@ export function isPrimitive(type: CodeName): boolean {
282282
|| type.className === 'Date';
283283
}
284284

285-
function specTypeToCodeType(resourceContext: CodeName, type: string): CodeName {
285+
/**
286+
* @param resourceContext
287+
* @param type the name of the type
288+
* @param complexType whether the type is a complexType (true) or primitive type (false)
289+
*/
290+
function specTypeToCodeType(resourceContext: CodeName, type: string, complexType: boolean): CodeName {
286291
if (type.endsWith('[]')) {
287-
const itemType = specTypeToCodeType(resourceContext, type.slice(0, -2));
292+
const itemType = specTypeToCodeType(resourceContext, type.slice(0, -2), complexType);
288293
return CodeName.forPrimitive(`${itemType.className}[]`);
289294
}
290-
if (schema.isPrimitiveType(type)) {
291-
return specPrimitiveToCodePrimitive(type);
295+
if (!complexType) {
296+
return specPrimitiveToCodePrimitive(type as schema.PrimitiveType);
292297
} else if (type === 'Tag') {
293298
// Tags are not considered primitive by the CloudFormation spec (even though they are intrinsic)
294299
// so we won't consider them primitive either.
@@ -301,9 +306,12 @@ function specTypeToCodeType(resourceContext: CodeName, type: string): CodeName {
301306

302307
/**
303308
* Translate a list of type references in a resource context to a list of code names
309+
*
310+
* @param resourceContext
311+
* @param types name and whether the type is a complex type (true) or primitive type (false)
304312
*/
305-
export function specTypesToCodeTypes(resourceContext: CodeName, types: string[]): CodeName[] {
306-
return types.map(type => specTypeToCodeType(resourceContext, type));
313+
export function specTypesToCodeTypes(resourceContext: CodeName, types: { [name: string]: boolean }): CodeName[] {
314+
return Object.entries(types).map(([name, complexType]) => specTypeToCodeType(resourceContext, name, complexType));
307315
}
308316

309317
export interface PropertyVisitor<T> {

Diff for: tools/@aws-cdk/cfn2ts/lib/spec-utils.ts

+22-6
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,18 @@ export class PropertyAttributeName extends SpecName {
7171
}
7272

7373
/**
74-
* Return a list of all allowable item types (either primitive or complex).
74+
* Return a list of all allowable item types, separating out primitive and complex
75+
* types because sometimes a complex type can have the same name as a primitive type.
76+
* If we only return the names in this case then the primitive type will always override
77+
* the complex type (not what we want).
78+
*
79+
* @returns type name and whether the type is a complex type (true) or primitive type (false)
7580
*/
76-
export function itemTypeNames(spec: schema.CollectionProperty): string[] {
77-
return complexItemTypeNames(spec).concat(primitiveItemTypeNames(spec));
81+
export function itemTypeNames(spec: schema.CollectionProperty): { [name: string]: boolean } {
82+
const types = complexItemTypeNames(spec).map(type => [type, true])
83+
.concat(primitiveItemTypeNames(spec).map(type => [type, false]));
84+
85+
return Object.fromEntries(types);
7886
}
7987

8088
function complexItemTypeNames(spec: schema.CollectionProperty): string[] {
@@ -98,10 +106,18 @@ function primitiveItemTypeNames(spec: schema.CollectionProperty): string[] {
98106
}
99107

100108
/**
101-
* Return a list of all allowable types (either primitive or complex).
109+
* Return a list of all allowable item types, separating out primitive and complex
110+
* types because sometimes a complex type can have the same name as a primitive type.
111+
* If we only return the names in this case then the primitive type will always override
112+
* the complex type (not what we want).
113+
*
114+
* @returns type name and whether the type is a complex type (true) or primitive type (false)
102115
*/
103-
export function scalarTypeNames(spec: schema.ScalarProperty): string[] {
104-
return complexScalarTypeNames(spec).concat(primitiveScalarTypeNames(spec));
116+
export function scalarTypeNames(spec: schema.ScalarProperty): { [name: string]: boolean } {
117+
const types = complexScalarTypeNames(spec).map(type => [type, true] )
118+
.concat(primitiveScalarTypeNames(spec).map(type => [type, false]));
119+
120+
return Object.fromEntries(types);
105121
}
106122

107123
function complexScalarTypeNames(spec: schema.ScalarProperty): string[] {

0 commit comments

Comments
 (0)