Skip to content

Commit

Permalink
fix(cfn2ts): doesn't handle property types with the same type as a pr…
Browse files Browse the repository at this point in the history
…imitive 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*
  • Loading branch information
corymhall authored May 8, 2023
1 parent 1f4c4fa commit b76c182
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 12 deletions.
20 changes: 14 additions & 6 deletions tools/@aws-cdk/cfn2ts/lib/genspec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,18 @@ export function isPrimitive(type: CodeName): boolean {
|| type.className === 'Date';
}

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

/**
* Translate a list of type references in a resource context to a list of code names
*
* @param resourceContext
* @param types name and whether the type is a complex type (true) or primitive type (false)
*/
export function specTypesToCodeTypes(resourceContext: CodeName, types: string[]): CodeName[] {
return types.map(type => specTypeToCodeType(resourceContext, type));
export function specTypesToCodeTypes(resourceContext: CodeName, types: { [name: string]: boolean }): CodeName[] {
return Object.entries(types).map(([name, complexType]) => specTypeToCodeType(resourceContext, name, complexType));
}

export interface PropertyVisitor<T> {
Expand Down
28 changes: 22 additions & 6 deletions tools/@aws-cdk/cfn2ts/lib/spec-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,18 @@ export class PropertyAttributeName extends SpecName {
}

/**
* Return a list of all allowable item types (either primitive or complex).
* Return a list of all allowable item types, separating out primitive and complex
* types because sometimes a complex type can have the same name as a primitive type.
* If we only return the names in this case then the primitive type will always override
* the complex type (not what we want).
*
* @returns type name and whether the type is a complex type (true) or primitive type (false)
*/
export function itemTypeNames(spec: schema.CollectionProperty): string[] {
return complexItemTypeNames(spec).concat(primitiveItemTypeNames(spec));
export function itemTypeNames(spec: schema.CollectionProperty): { [name: string]: boolean } {
const types = complexItemTypeNames(spec).map(type => [type, true])
.concat(primitiveItemTypeNames(spec).map(type => [type, false]));

return Object.fromEntries(types);
}

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

/**
* Return a list of all allowable types (either primitive or complex).
* Return a list of all allowable item types, separating out primitive and complex
* types because sometimes a complex type can have the same name as a primitive type.
* If we only return the names in this case then the primitive type will always override
* the complex type (not what we want).
*
* @returns type name and whether the type is a complex type (true) or primitive type (false)
*/
export function scalarTypeNames(spec: schema.ScalarProperty): string[] {
return complexScalarTypeNames(spec).concat(primitiveScalarTypeNames(spec));
export function scalarTypeNames(spec: schema.ScalarProperty): { [name: string]: boolean } {
const types = complexScalarTypeNames(spec).map(type => [type, true] )
.concat(primitiveScalarTypeNames(spec).map(type => [type, false]));

return Object.fromEntries(types);
}

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

0 comments on commit b76c182

Please sign in to comment.