Fixes invalid parameterized type resolution for properties containing…

… nested aggregate types. (#208)

* Fixed type inference of properties containing unresolved intrinsics as well as nested aggregate types.

* Updated SAM CFN specification file.

* Normalized naming of aggregate type constant (i.e. `awsComplexTypes` -> `awsAggregateTypes`) and verification function (i.e. `isComplexType` -> `isAggregateType`).

* Fixed data corruption during type inference, added nullipotency tests for functions relating to the former as well as a regression test for issue #207.

* Update CHANGELOG.md

CHANGELOG.md

@@ -6,6 +6,8 @@ and this project adheres to [Semantic
 Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Fixed
+- Merge PR #208, fixing invalid parameterized type resolution for properties containing nested aggregate types
 
 ## [1.9.1] - 2018-10-30
 ### Changed

data/sam_20161031_custom_specification.json

@@ -1,5 +1,15 @@
 {
   "PropertyTypes": {
+    "AWS::Serverless::Function.IAMPolicyDocument": {
+      "Documentation": "",
+      "Properties": {
+        "Version": {
+          "Required": false,
+          "PrimitiveType": "String",
+          "UpdateType": "Mutable"
+        }
+      }
+    },
     "AWS::Serverless::Function.EventSource": {
       "Properties": {
         "Properties": {

src/awsData.ts

@@ -4,7 +4,7 @@ export const awsExtraDocs = require('../data/aws_extra_docs.json') as AWSExtraDo
 
 export const awsPrimitiveTypes = ['String', 'Integer', 'Boolean', 'Json', 'Double', 'Long', 'Timestamp'];
 
-export const awsComplexTypes = ['Map', 'List'];
+export const awsAggregateTypes = ['Map', 'List'];
 
 export type AWSPrimitiveType = 'String' | 'Integer' | 'Boolean' | 'Json' | 'Double' | 'Long' | 'Timestamp';
 

src/resourcesSpec.ts

@@ -284,7 +284,7 @@ export function rebaseTypeFormat(baseType: string, type: string): string {
         return parameterizeTypeFormat(typeName, typeArgument);
     }
 
-    if (isPrimitiveType(type) || isComplexType(type)) {
+    if (isPrimitiveType(type) || isAggregateType(type)) {
       return type;
     }
 
@@ -301,11 +301,11 @@ export function isPrimitiveType(type: string) {
     return false;
 }
 
-export function isComplexType(type: string) {
+export function isAggregateType(type: string) {
     if (isParameterizedTypeFormat(type)) {
         type = getParameterizedTypeName(type);
     }
-    if (!!~awsData.awsComplexTypes.indexOf(type)) {
+    if (!!~awsData.awsAggregateTypes.indexOf(type)) {
         return true;
     }
     return false;
@@ -445,7 +445,7 @@ export function getItemType(baseType: string, propType: string, key: string) {
     const property = getProperty(propType, key);
     if (!property.ItemType) {
         return undefined;
-    } else if (isComplexType(property.ItemType)) {
+    } else if (isAggregateType(property.ItemType)) {
         return property.ItemType;
     } else {
         return baseType + '.' + property.ItemType;

src/test/validatorTest.ts

@@ -9,6 +9,8 @@ import {awsResources} from '../awsData';
 import util = require('util');
 import { isObject } from 'util';
 
+const clone = require('clone');
+
 describe('validator', () => {
 
     before(() => {
@@ -915,6 +917,13 @@ describe('validator', () => {
             expect(result).to.have.deep.property('templateValid', true);
             expect(result['errors']['crit']).to.have.lengthOf(0);
         });
+
+        it('a template with properties containing nested aggregate types should be validated successfully', () => {
+            const input = 'testData/valid/yaml/sam_20161031_issue_207.yaml';
+            let result = validator.validateFile(input);
+            expect(result).to.have.deep.property('templateValid', true);
+            expect(result['errors']['crit']).to.have.lengthOf(0);
+        });
     });
 
     describe('parameters-validation', () => {
@@ -1472,6 +1481,30 @@ describe('validator', () => {
               expect(result).to.equal('Timestamp');
           });
 
+          it('should be nullipotent upon input data - typeof number', () => {
+              let input = 1;
+              let result = validator.__TESTING__.inferPrimitiveValueType(input);
+              expect(input).to.equal(1);
+          });
+
+          it('should be nullipotent upon input data - typeof string', () => {
+              let input = 'someString';
+              let result = validator.__TESTING__.inferPrimitiveValueType(input);
+              expect(input).to.equal('someString');
+          });
+
+          it('should be nullipotent upon input data - typeof boolean', () => {
+              let input = true;
+              let result = validator.__TESTING__.inferPrimitiveValueType(input);
+              expect(input).to.equal(true);
+          });
+
+          it('should be nullipotent upon input data - typeof object', () => {
+              let input = {someKey: 'someValue'};
+              let result = validator.__TESTING__.inferPrimitiveValueType(input);
+              expect(input).to.deep.equal({someKey: 'someValue'});
+          });
+
         });
 
         describe('inferStructureValueType', () => {
@@ -1514,6 +1547,26 @@ describe('validator', () => {
               expect(result).to.equal(null);
           });
 
+          it('should be nullipotent upon input data', () => {
+              let input = {
+                'Type': 'AWS::Serverless::Function',
+                'Properties': {
+                  'Variables': {
+                    'someKey': 'someValue'
+                  },
+                }
+              };
+              let result = validator.__TESTING__.inferPrimitiveValueType(input);
+              expect(input).to.deep.equal({
+                'Type': 'AWS::Serverless::Function',
+                'Properties': {
+                  'Variables': {
+                    'someKey': 'someValue'
+                  },
+                }
+              });
+          });
+
         });
 
         describe('inferAggregateValueType', () => {
@@ -1527,6 +1580,15 @@ describe('validator', () => {
               expect(result).to.equal('List<String>');
           });
 
+          it('should be able to infer a List of Json aggregate value type', () => {
+              let input = [
+                { 1: 'somethingBeautiful' }, { 2: 'somethingAwesome' }, { 3: 'somethingCool' }
+              ];
+              let candidateTypes: string[] = [];
+              let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
+              expect(result).to.equal('List<Json>');
+          });
+
           it('should be able to infer a Map of Strings aggregate value type', () => {
               let input = {
                 1: 'somethingBeautiful',
@@ -1538,13 +1600,76 @@ describe('validator', () => {
               expect(result).to.equal('Map<String>');
           });
 
+          it('should be able to infer a Map of Json aggregate value type', () => {
+              let input = {
+                1: { 1: 'somethingBeautiful' },
+                2: { 2: 'somethingAwesome' },
+                3: { 3: 'somethingCool' }
+              };
+              let candidateTypes: string[] = [];
+              let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
+              expect(result).to.equal('Map<Json>');
+          });
+
           it('should not be able to infer a non-aggregate value type', () => {
               let input = {};
               let candidateTypes: string[] = [];
               let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
               expect(result).to.equal(null);
           });
 
+          it('should be nullipotent upon input data - List of Strings aggregate value type', () => {
+              let input = [
+                'somethingBeautiful', 'somethingAwesome', 'somethingCool'
+              ];
+              let candidateTypes: string[] = [];
+              let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
+              expect(input).to.deep.equal([
+                'somethingBeautiful', 'somethingAwesome', 'somethingCool'
+              ]);
+          });
+
+          it('should be nullipotent upon input data - List of Json aggregate value type', () => {
+              let input = [
+                { 1: 'somethingBeautiful' }, { 2: 'somethingAwesome' }, { 3: 'somethingCool' }
+              ];
+              let candidateTypes: string[] = [];
+              let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
+              expect(input).to.deep.equal([
+                { 1: 'somethingBeautiful' }, { 2: 'somethingAwesome' }, { 3: 'somethingCool' }
+              ]);
+          });
+
+          it('should be nullipotent upon input data - Map of Strings aggregate value type', () => {
+              let input = {
+                1: 'somethingBeautiful',
+                2: 'somethingAwesome',
+                3: 'somethingCool'
+              };
+              let candidateTypes: string[] = [];
+              let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
+              expect(input).to.deep.equal({
+                1: 'somethingBeautiful',
+                2: 'somethingAwesome',
+                3: 'somethingCool'
+              });
+          });
+
+          it('should be nullipotent upon input data - Map of Json aggregate value type', () => {
+              let input = {
+                1: { 1: 'somethingBeautiful' },
+                2: { 2: 'somethingAwesome' },
+                3: { 3: 'somethingCool' }
+              };
+              let candidateTypes: string[] = [];
+              let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
+              expect(input).to.deep.equal({
+                1: { 1: 'somethingBeautiful' },
+                2: { 2: 'somethingAwesome' },
+                3: { 3: 'somethingCool' }
+              });
+          });
+
         });
 
         describe('inferValueType', () => {
@@ -1579,6 +1704,49 @@ describe('validator', () => {
 
     });
 
+    describe('templateHasUnresolvedIntrinsics', () => {
+
+      it('should be able to detect if a given template has top-level unresolved intrinsics', () => {
+          const input = {
+            'Ref': 'AWS::Region'
+          };
+          const result = validator.__TESTING__.templateHasUnresolvedIntrinsics(clone(input));
+          expect(result).to.equal(true);
+          expect(input).to.deep.equal(input);
+      });
+
+      it('should be able to detect if a given template has nested unresolved intrinsics', () => {
+          const input = {
+            'Resources': {
+              'MyBucket': {
+                'Type': 'AWS::S3::Bucket',
+                'BucketName': {
+                  'Ref': 'AWS::Region'
+                }
+              }
+            }
+          };
+          const result = validator.__TESTING__.templateHasUnresolvedIntrinsics(clone(input));
+          expect(result).to.equal(true);
+          expect(input).to.deep.equal(input);
+      });
+
+      it('should be able to detect if a given template does not contain unresolved intrinsics', () => {
+          const input = {
+            'Resources': {
+              'MyBucket': {
+                'Type': 'AWS::S3::Bucket',
+                'BucketName': 'somethingCool'
+              }
+            }
+          };
+          const result = validator.__TESTING__.templateHasUnresolvedIntrinsics(clone(input));
+          expect(result).to.equal(false);
+          expect(input).to.deep.equal(input);
+      });
+
+    });
+
     describe('SAM-20161031', function() {
 
         this.timeout(5000);