working with <propName>Prop to avoid collisions

refactor to suffix attributes and remove resource names
refactoring to enable cfn resource class members to be undefined if not required
initialize class members from props artifact in constructor
refactoring to combine render and update, cleaning up code from review comments
fixing cfn-resource to by default handle tags and use this.properties
updating to use `ref` instead of `attrRef` for all references

packages/@aws-cdk/aws-apigateway/test/test.resource.ts

@@ -286,4 +286,4 @@ export = {
 
     }
   }
-};
+};

packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts

@@ -141,8 +141,8 @@ export class Alarm extends Resource implements IAlarm {
       ...metricJson(props.metric)
     });
 
-    this.alarmArn = alarm.alarmArn;
-    this.alarmName = alarm.alarmName;
+    this.alarmArn = alarm.attrArn;
+    this.alarmName = alarm.ref;
     this.metric = props.metric;
     this.annotation = {
       // tslint:disable-next-line:max-line-length

packages/@aws-cdk/aws-iam/lib/group.ts

@@ -9,11 +9,15 @@ import { AttachedPolicies, undefinedIfEmpty } from './util';
 
 export interface IGroup extends IIdentity {
   /**
+   * Returns the IAM Group Name
+   *
    * @attribute
    */
   readonly groupName: string;
 
   /**
+   * Returns the IAM Group ARN
+   *
    * @attribute
    */
   readonly groupArn: string;
@@ -131,8 +135,8 @@ export class Group extends GroupBase {
       path: props.path,
     });
 
-    this.groupName = group.groupName;
-    this.groupArn = group.groupArn;
+    this.groupName = group.ref;
+    this.groupArn = group.attrArn;
   }
 
   /**

packages/@aws-cdk/aws-iam/lib/role.ts

@@ -206,9 +206,9 @@ export class Role extends Resource implements IRole {
       maxSessionDuration: props.maxSessionDurationSec,
     });
 
-    this.roleId = role.roleId;
-    this.roleArn = role.roleArn;
-    this.roleName = role.roleName;
+    this.roleId = role.attrRoleId;
+    this.roleArn = role.attrArn;
+    this.roleName = role.ref;
     this.policyFragment = new ArnPrincipal(this.roleArn).policyFragment;
 
     function _flatten(policies?: { [name: string]: PolicyDocument }) {

packages/@aws-cdk/aws-iam/lib/user.ts

@@ -112,8 +112,8 @@ export class User extends Resource implements IIdentity {
       loginProfile: this.parseLoginProfile(props)
     });
 
-    this.userName = user.userName;
-    this.userArn = user.userArn;
+    this.userName = user.ref;
+    this.userArn = user.attrArn;
     this.policyFragment = new ArnPrincipal(this.userArn).policyFragment;
 
     if (props.groups) {

packages/@aws-cdk/aws-kms/lib/key.ts

@@ -224,7 +224,7 @@ export class Key extends KeyBase {
       keyPolicy: this.policy,
     });
 
-    this.keyArn = resource.keyArn;
+    this.keyArn = resource.attrArn;
     resource.options.deletionPolicy = props.retain === false
                                     ? DeletionPolicy.Delete
                                     : DeletionPolicy.Retain;

packages/@aws-cdk/cdk/lib/cfn-resource.ts

@@ -67,22 +67,10 @@ export class CfnResource extends CfnRefElement {
    */
   protected readonly properties: any;
 
-  /**
-   * AWS resource property overrides.
-   *
-   * During synthesis, the method "renderProperties(this.overrides)" is called
-   * with this object, and merged on top of the output of
-   * "renderProperties(this.properties)".
-   *
-   * Derived classes should expose a strongly-typed version of this object as
-   * a public property called `propertyOverrides`.
-   */
-  protected readonly untypedPropertyOverrides: any = { };
-
   /**
    * An object to be merged on top of the entire resource definition.
    */
-  private readonly rawOverrides: any = { };
+  private readonly rawOverrides: any = {};
 
   /**
    * Logical IDs of dependencies.
@@ -103,7 +91,7 @@ export class CfnResource extends CfnRefElement {
     }
 
     this.resourceType = props.type;
-    this.properties = props.properties || { };
+    this.properties = props.properties || {};
 
     // if aws:cdk:enable-path-metadata is set, embed the current construct's
     // path in the CloudFormation template, so it will be possible to trace
@@ -146,7 +134,7 @@ export class CfnResource extends CfnRefElement {
       // object overwrite it with an object.
       const isObject = curr[key] != null && typeof(curr[key]) === 'object' && !Array.isArray(curr[key]);
       if (!isObject) {
-        curr[key] = { };
+        curr[key] = {};
       }
 
       curr = curr[key];
@@ -205,33 +193,21 @@ export class CfnResource extends CfnRefElement {
    */
   public _toCloudFormation(): object {
     try {
-      // merge property overrides onto properties and then render (and validate).
-      const tags = TagManager.isTaggable(this) ? this.tags.renderTags() : undefined;
-      const properties = deepMerge(
-        this.properties || {},
-        { tags },
-        this.untypedPropertyOverrides
-      );
-
       const ret = {
         Resources: {
           // Post-Resolve operation since otherwise deepMerge is going to mix values into
           // the Token objects returned by ignoreEmpty.
           [this.logicalId]: new PostResolveToken({
             Type: this.resourceType,
-            Properties: ignoreEmpty(properties),
+            Properties: ignoreEmpty(this.renderProperties()),
             DependsOn: ignoreEmpty(renderDependsOn(this.dependsOn)),
-            CreationPolicy:  capitalizePropertyNames(this, this.options.creationPolicy),
+            CreationPolicy: capitalizePropertyNames(this, this.options.creationPolicy),
             UpdatePolicy: capitalizePropertyNames(this, this.options.updatePolicy),
             UpdateReplacePolicy: capitalizePropertyNames(this, this.options.updateReplacePolicy),
             DeletionPolicy: capitalizePropertyNames(this, this.options.deletionPolicy),
             Metadata: ignoreEmpty(this.options.metadata),
             Condition: this.options.condition && this.options.condition.logicalId
           }, props => {
-            // let derived classes to influence how properties are rendered (e.g. change capitalization)
-            props.Properties = this.renderProperties(props.Properties);
-
-            // merge overrides *after* rendering
             return deepMerge(props, this.rawOverrides);
           })
         }
@@ -262,8 +238,19 @@ export class CfnResource extends CfnRefElement {
     }
   }
 
-  protected renderProperties(properties: any): { [key: string]: any } {
-    return properties;
+  protected renderProperties(): { [key: string]: any } {
+    const tags = TagManager.isTaggable(this) ? this.tags.renderTags() : {};
+    return deepMerge(this.properties || {}, {tags});
+  }
+
+  /**
+   * Return properties modified after initiation
+   *
+   * Resources that expose mutable properties should override this function to
+   * collect and return the properties object for this resource.
+   */
+  protected get updatedProperites(): { [key: string]: any } {
+    return this.properties;
   }
 
   protected validateProperties(_properties: any) {
@@ -339,7 +326,7 @@ export function deepMerge(target: any, ...sources: any[]) {
         // if the value at the target is not an object, override it with an
         // object so we can continue the recursion
         if (typeof(target[key]) !== 'object') {
-          target[key] = { };
+          target[key] = {};
         }
 
         deepMerge(target[key], value);

packages/@aws-cdk/cdk/test/test.resource.ts

@@ -582,7 +582,7 @@ export = {
       test.done();
     },
 
-    'untypedPropertyOverrides': {
+    'updatedProperties': {
 
       'can be used by derived classes to specify overrides before render()'(test: Test) {
         const stack = new Stack();
@@ -591,7 +591,7 @@ export = {
           prop1: 'foo'
         });
 
-        r.setProperty('prop2', 'bar');
+        r.prop2 = 'bar';
 
         test.deepEqual(toCloudFormation(stack), { Resources:
           { MyResource:
@@ -605,7 +605,7 @@ export = {
 
         const r = new CustomizableResource(stack, 'MyResource');
 
-        r.setProperty('prop3', 'zoo');
+        r.prop3 = 'zoo';
 
         test.deepEqual(toCloudFormation(stack), { Resources:
           { MyResource:
@@ -619,8 +619,8 @@ export = {
 
         const r = new CustomizableResource(stack, 'MyResource', { });
 
-        r.setProperty('prop3', 'zoo');
-        r.setProperty('prop2', 'hey');
+        r.prop3 = 'zoo';
+        r.prop2 = 'hey';
 
         test.deepEqual(toCloudFormation(stack), { Resources:
           { MyResource:
@@ -696,26 +696,52 @@ class Counter extends CfnResource {
   public increment(by = 1) {
     this.properties.Count += by;
   }
+
+  protected get updatedProperites(): { [key: string]: any } {
+    return {Count: this.properties.Count};
+  }
 }
 
 function withoutHash(logId: string) {
   return logId.substr(0, logId.length - 8);
 }
 
 class CustomizableResource extends CfnResource {
+  public prop1: any;
+  public prop2: any;
+  public prop3: any;
+
   constructor(scope: Construct, id: string, props?: any) {
     super(scope, id, { type: 'MyResourceType', properties: props });
+    if (props !== undefined) {
+      this.prop1 = props.prop1;
+      this.prop2 = props.prop2;
+      this.prop3 = props.prop3;
+    }
   }
 
-  public setProperty(key: string, value: any) {
-    this.untypedPropertyOverrides[key] = value;
+  public renderProperties(): { [key: string]: any } {
+    const props = this.updatedProperites;
+    const render: { [key: string]: any } = {};
+    for (const key of Object.keys(props)) {
+      render[key.toUpperCase()] = props[key];
+    }
+    return render;
   }
 
-  public renderProperties(properties: any) {
-    return {
-      PROP1: properties.prop1,
-      PROP2: properties.prop2,
-      PROP3: properties.prop3
+  protected get updatedProperites(): { [key: string]: any } {
+    const props: { [key: string]: any } = {
+      prop1: this.prop1,
+      prop2: this.prop2,
+      prop3: this.prop3,
     };
+    const cleanProps: { [key: string]: any } = { };
+    for (const key of Object.keys(props)) {
+      if (props[key] === undefined) {
+        continue;
+      }
+      cleanProps[key] = props[key];
+    }
+    return cleanProps;
   }
 }

tools/awslint/lib/rules/cfn-resource.ts

@@ -67,12 +67,7 @@ export class CfnResourceReflection {
 
     this.namespace = fullname.split('::').slice(0, 2).join('::');
 
-    // special-case
-    const basename = this.basename
-      .replace(/VPC/g, 'Vpc')
-      .replace(/DB/g, 'Db');
-
-    this.attributePrefix = basename[0].toLowerCase() + basename.slice(1);
+    this.attributePrefix = 'attr';
 
     this.attributeNames = cls.ownProperties
       .filter(p => (p.docs.docs.custom || {}).cloudformationAttribute)
@@ -89,9 +84,6 @@ export class CfnResourceReflection {
       return 'securityGroupId';
     }
 
-    const cfnName = name.startsWith(this.basename) ? name.slice(this.basename.length) : name;
-
-    // if the CFN attribute name already have the type name as a prefix (i.e. RoleId), we only take the "Id" as the "name".
-    return this.attributePrefix + camelcase(cfnName, { pascalCase: true });
+    return this.attributePrefix + camelcase(name, { pascalCase: true });
   }
-}
+}

tools/awslint/lib/rules/construct.ts

@@ -292,4 +292,4 @@ constructLinter.add({
     e.assert(!property.type.isAny, `${e.ctx.propsFqn}.${property.name}`);
     }
   }
-});
+});

tools/awslint/lib/rules/resource.ts

@@ -69,15 +69,16 @@ export class ResourceReflection {
   private findAttributeProperties(): Attribute[] {
     const result = new Array<Attribute>();
 
+    const attrPrefix = this.basename.charAt(0).toLowerCase() + this.basename.slice(1);
     for (const p of this.construct.classType.allProperties) {
       if (p.protected) {
         continue; // skip any protected properties
       }
 
-      // an attribute property is a property which starts with the type name
-      // (e.g. "bucketXxx") and/or has an @attribute doc tag.
+      // an attribute property is a property which starts with `attr`
+      // and/or has an @attribute doc tag.
       const tag = getDocTag(p, 'attribute');
-      if (!p.name.startsWith(this.cfn.attributePrefix) && !tag) {
+      if (!p.name.startsWith(attrPrefix) && !tag) {
         continue;
       }
 
@@ -152,7 +153,12 @@ resourceLinter.add({
   message: 'resources must represent all cloudformation attributes as attribute properties. missing property: ',
   eval: e => {
     for (const name of e.ctx.cfn.attributeNames) {
-      const found = e.ctx.attributes.find(a => a.names.includes(name));
+      const basename = e.ctx.basename.charAt(0).toLowerCase() + e.ctx.basename.slice(1);
+      const stripAttr = name.replace('attr', '');
+      const lookup = stripAttr.toLowerCase().startsWith(basename) ?
+        stripAttr.charAt(0).toLowerCase() + stripAttr.slice(1) :
+        basename + stripAttr;
+      const found = e.ctx.attributes.find(a => a.names.includes(lookup));
       e.assert(found, `${e.ctx.fqn}.${name}`, name);
     }
   }