refactor: props should not expose L1 types (awslint:props-no-cfn-type…

…s) (#2610)

Adds a new awslint:props-cfn-types rule which validates that props
do not expose L1 types (types that start with "Cfn").

This is in accordance with the new AWS Construct Library guidelines.

Fixes #2435

packages/@aws-cdk/cdk/package.json

@@ -35,7 +35,9 @@
     "exclude": [
       "construct-ctor:@aws-cdk/cdk.App.<initializer>",
       "construct-ctor:@aws-cdk/cdk.Root.<initializer>",
-      "construct-ctor:@aws-cdk/cdk.Stack.<initializer>.params*"
+      "construct-ctor:@aws-cdk/cdk.Stack.<initializer>.params*",
+      "props-no-cfn-types:@aws-cdk/cdk.CfnOutputProps*",
+      "props-no-cfn-types:@aws-cdk/cdk.StringListCfnOutputProps*"
     ]
   },
   "scripts": {

tools/awslint/lib/rules/construct.ts

@@ -14,6 +14,16 @@ export class ConstructReflection {
       .map(c => new ConstructReflection(c));
   }
 
+  public static getFqnFromTypeRef(typeRef: reflect.TypeReference) {
+    if (typeRef.arrayOfType) {
+      return typeRef.arrayOfType.fqn;
+    } else if (typeRef.mapOfType) {
+      return typeRef.mapOfType.fqn;
+    }
+
+    return typeRef.fqn;
+  }
+
   public readonly ROOT_CLASS: reflect.ClassType; // cdk.Construct
 
   public readonly fqn: string;
@@ -220,18 +230,32 @@ constructLinter.add({
     if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }
 
     for (const property of e.ctx.propsType.allProperties) {
-      const typeRef = property.type;
-      let fqn = typeRef.fqn;
+      const fqn = ConstructReflection.getFqnFromTypeRef(property.type);
 
-      if (typeRef.arrayOfType) {
-        fqn = typeRef.arrayOfType.fqn;
-      } else if (typeRef.mapOfType) {
-        fqn = typeRef.mapOfType.fqn;
+      const found = (fqn && e.ctx.sys.tryFindFqn(fqn));
+      if (found) {
+        e.assert(!(fqn === e.ctx.core.tokenClass.fqn), `${e.ctx.propsFqn}.${property.name}`);
       }
+    }
+  }
+});
+
+constructLinter.add({
+  code: 'props-no-cfn-types',
+  message: 'props should not expose L1 types (types which start with "Cfn")',
+  eval: e => {
+    if (!e.ctx.propsType) { return; }
+    if (!e.ctx.hasPropsArgument) { return; }
+
+    // this rule only applies to L2 constructs
+    if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }
+
+    for (const property of e.ctx.propsType.ownProperties) {
+      const fqn = ConstructReflection.getFqnFromTypeRef(property.type);
 
       const found = (fqn && e.ctx.sys.tryFindFqn(fqn));
       if (found) {
-        e.assert(!(fqn === e.ctx.core.tokenClass.fqn), `${e.ctx.propsFqn}.${property.name}`);
+        e.assert(!found.name.toLowerCase().startsWith('cfn'), `${e.ctx.propsFqn}.${property.name}`);
       }
     }
   }

tools/awslint/lib/rules/core-types.ts

@@ -20,6 +20,7 @@ export class CoreTypes {
   public static hasCoreModule(assembly: reflect.Assembly) {
     return (!assembly.system.assemblies.find(a => a.name === CORE_MODULE));
   }
+
   /**
    * @returns true if `classType` represents an L1 Cfn Resource
    */

tools/awslint/lib/rules/util.ts

@@ -2,16 +2,16 @@ import reflect = require('jsii-reflect');
 
 /**
  * Returns a documentation tag. Looks it up in inheritance hierarchy.
- * @param documetable starting point
+ * @param documentable starting point
  * @param tag the tag to search for
  */
-export function getDocTag(documetable: reflect.Documentable, tag: string): string | undefined {
-  const t = documetable.docs.customTag(tag);
+export function getDocTag(documentable: reflect.Documentable, tag: string): string | undefined {
+  const t = documentable.docs.customTag(tag);
   if (t) { return t; }
 
-  if ((documetable instanceof reflect.Property || documetable instanceof reflect.Method) && documetable.overrides) {
-    if (documetable.overrides.isClassType() || documetable.overrides.isInterfaceType()) {
-      const baseMembers = documetable.overrides.allMembers.filter(m => m.name === documetable.name);
+  if ((documentable instanceof reflect.Property || documentable instanceof reflect.Method) && documentable.overrides) {
+    if (documentable.overrides.isClassType() || documentable.overrides.isInterfaceType()) {
+      const baseMembers = documentable.overrides.allMembers.filter(m => m.name === documentable.name);
       for (const base of baseMembers) {
         const baseTag = getDocTag(base, tag);
         if (baseTag) {
@@ -21,17 +21,17 @@ export function getDocTag(documetable: reflect.Documentable, tag: string): strin
     }
   }
 
-  if (documetable instanceof reflect.ClassType || documetable instanceof reflect.InterfaceType) {
-    for (const base of documetable.interfaces) {
+  if (documentable instanceof reflect.ClassType || documentable instanceof reflect.InterfaceType) {
+    for (const base of documentable.interfaces) {
       const baseTag = getDocTag(base, tag);
       if (baseTag) {
          return baseTag;
       }
     }
   }
 
-  if (documetable instanceof reflect.ClassType && documetable.base) {
-    const baseTag = getDocTag(documetable.base, tag);
+  if (documentable instanceof reflect.ClassType && documentable.base) {
+    const baseTag = getDocTag(documentable.base, tag);
     if (baseTag) {
       return baseTag;
     }