Skip to content

Commit

Permalink
refactor: L2 constructs should not use the "Token" type (awslint:prop…
Browse files Browse the repository at this point in the history
…s-no-tokens) (aws#2542)

Adds a new awslint:props-no-tokens rule which validates that props do not use the "Token" type.

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

Fixes aws#2434
  • Loading branch information
shivlaks authored May 22, 2019
1 parent c51596e commit c41a0f6
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 81 deletions.
9 changes: 4 additions & 5 deletions tools/awslint/lib/rules/api.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import reflect = require('jsii-reflect');
import { Linter } from '../linter';
import { CfnResourceReflection } from './cfn-resource';
import { ConstructReflection } from './construct';
import { ResourceReflection } from './resource';
import { CoreTypes } from './core-types';

const EXCLUDE_ANNOTATION_REF_VIA_INTERFACE = '[disable-awslint:ref-via-interface]';

// lint all constructs that are not L1 resources
export const apiLinter = new Linter(a => ConstructReflection
.findAllConstructs(a)
.filter(c => !CfnResourceReflection.isCfnResource(c.classType)));
.filter(c => !CoreTypes.isCfnResource(c.classType)));

apiLinter.add({
code: 'ref-via-interface',
Expand Down Expand Up @@ -123,11 +122,11 @@ apiLinter.add({

// classes are okay as long as they are not resource constructs
if (type.type && type.type.isClassType()) {
if (!ResourceReflection.isResourceClass(type.type)) {
if (!CoreTypes.isResourceClass(type.type)) {
return;
}

if (type.type.fqn === ConstructReflection.CONSTRUCT_FQN) {
if (type.type.fqn === e.ctx.core.constructClass.fqn) {
return;
}

Expand Down
32 changes: 2 additions & 30 deletions tools/awslint/lib/rules/cfn-resource.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import camelcase = require('camelcase');
import reflect = require('jsii-reflect');
import { Linter } from '../linter';
import { CORE_MODULE } from './common';
import { ConstructReflection } from './construct';
import { CoreTypes } from './core-types';
import { ResourceReflection } from './resource';
const CFN_RESOURCE_BASE_CLASS_FQN = `${CORE_MODULE}.CfnResource`;

// this linter verifies that we have L2 coverage. it finds all "Cfn" classes and verifies
// that we have a corresponding L1 class for it that's identified as a resource.
Expand Down Expand Up @@ -40,36 +38,10 @@ export class CfnResourceReflection {
*/
public static findAll(assembly: reflect.Assembly) {
return assembly.classes
.filter(c => this.isCfnResource(c))
.filter(c => CoreTypes.isCfnResource(c))
.map(c => new CfnResourceReflection(c));
}

public static isCfnResource(c: reflect.ClassType) {
if (!c.system.includesAssembly(CORE_MODULE)) {
return false;
}

// skip CfnResource itself
if (c.fqn === CFN_RESOURCE_BASE_CLASS_FQN) {
return false;
}

if (!ConstructReflection.isConstructClass(c)) {
return false;
}

const cfnResourceClass = c.system.findFqn(CFN_RESOURCE_BASE_CLASS_FQN);
if (!c.extends(cfnResourceClass)) {
return false;
}

if (!c.name.startsWith('Cfn')) {
return false;
}

return true;
}

public readonly classType: reflect.ClassType;
public readonly fullname: string; // AWS::S3::Bucket
public readonly namespace: string; // AWS::S3
Expand Down
1 change: 0 additions & 1 deletion tools/awslint/lib/rules/common.ts

This file was deleted.

68 changes: 38 additions & 30 deletions tools/awslint/lib/rules/construct.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,16 @@
import reflect = require('jsii-reflect');
import { Linter, MethodSignatureParameterExpectation } from '../linter';
import { CORE_MODULE } from './common';
import { CoreTypes } from './core-types';

export const constructLinter = new Linter<ConstructReflection>(assembly => assembly.classes
.filter(t => ConstructReflection.isConstructClass(t))
.filter(t => CoreTypes.isConstructClass(t))
.map(construct => new ConstructReflection(construct)));

export class ConstructReflection {

public static readonly CONSTRUCT_FQN = `${CORE_MODULE}.Construct`;
public static readonly CONSTRUCT_INTERFACE_FQN = `${CORE_MODULE}.IConstruct`;

/**
* Determines if a class is a construct.
*/
public static isConstructClass(c: reflect.ClassType) {
if (!c.system.includesAssembly(CORE_MODULE)) {
return false;
}

if (!c.isClassType()) {
return false;
}

if (c.abstract) {
return false;
}

return c.extends(c.system.findFqn(this.CONSTRUCT_FQN));
}

public static findAllConstructs(assembly: reflect.Assembly) {
return assembly.classes
.filter(c => ConstructReflection.isConstructClass(c))
.filter(c => CoreTypes.isConstructClass(c))
.map(c => new ConstructReflection(c));
}

Expand All @@ -46,11 +24,13 @@ export class ConstructReflection {
public readonly initializer?: reflect.Initializer;
public readonly hasPropsArgument: boolean;
public readonly sys: reflect.TypeSystem;
public readonly core: CoreTypes;

constructor(public readonly classType: reflect.ClassType) {
this.fqn = classType.fqn;
this.sys = classType.system;
this.ROOT_CLASS = this.sys.findClass(ConstructReflection.CONSTRUCT_FQN);
this.core = new CoreTypes(this.sys);
this.ROOT_CLASS = this.sys.findClass(this.core.constructClass.fqn);
this.interfaceFqn = `${classType.assembly.name}.I${classType.name}`;
this.propsFqn = `${classType.assembly.name}.${classType.name}Props`;
this.interfaceType = this.tryFindInterface();
Expand Down Expand Up @@ -105,7 +85,7 @@ constructLinter.add({

expectedParams.push({
name: 'scope',
type: ConstructReflection.CONSTRUCT_FQN
type: e.ctx.core.constructClass.fqn
});

expectedParams.push({
Expand Down Expand Up @@ -181,7 +161,7 @@ constructLinter.add({
message: 'construct interface must extend core.IConstruct',
eval: e => {
if (!e.ctx.interfaceType) { return; }
const interfaceBase = e.ctx.sys.findInterface(ConstructReflection.CONSTRUCT_INTERFACE_FQN);
const interfaceBase = e.ctx.sys.findInterface(e.ctx.core.constructInterface.fqn);
e.assert(e.ctx.interfaceType.extends(interfaceBase), e.ctx.interfaceType.fqn);
}
});
Expand All @@ -205,7 +185,7 @@ constructLinter.add({
if (!e.ctx.hasPropsArgument) { return; }

// this rule only applies to L2 constructs
if (e.ctx.classType.name.startsWith('Cfn')) { return; }
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.ownProperties) {
e.assert(!property.type.unionOfTypes, `${e.ctx.propsFqn}.${property.name}`);
Expand All @@ -221,10 +201,38 @@ constructLinter.add({
if (!e.ctx.hasPropsArgument) { return; }

// this rule only applies to L2 constructs
if (e.ctx.classType.name.startsWith('Cfn')) { return; }
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.ownProperties) {
e.assert(!property.name.toLowerCase().endsWith('arn'), `${e.ctx.propsFqn}.${property.name}`);
}
}
});

constructLinter.add({
code: 'props-no-tokens',
message: 'props should not use the "Token" type',
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.allProperties) {
const typeRef = property.type;
let fqn = typeRef.fqn;

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}`);
}
}
}
});
129 changes: 129 additions & 0 deletions tools/awslint/lib/rules/core-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import reflect = require("jsii-reflect");
import { TypeSystem } from "jsii-reflect";
import { getDocTag } from "./util";

const CORE_MODULE = "@aws-cdk/cdk";
enum CoreTypesFqn {
CfnResource = "@aws-cdk/cdk.CfnResource",
Construct = "@aws-cdk/cdk.Construct",
ConstructInterface = "@aws-cdk/cdk.IConstruct",
Resource = "@aws-cdk/cdk.Resource",
ResourceInterface = "@aws-cdk/cdk.IResource",
Token = "@aws-cdk/cdk.Token"
}

export class CoreTypes {

/**
* @returns true if assembly has the Core module
*/
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
*/
public static isCfnResource(c: reflect.ClassType) {
if (!c.system.includesAssembly(CORE_MODULE)) {
return false;
}

// skip CfnResource itself
if (c.fqn === CoreTypesFqn.CfnResource) {
return false;
}

if (!this.isConstructClass(c)) {
return false;
}

const cfnResourceClass = c.system.findFqn(CoreTypesFqn.CfnResource);
if (!c.extends(cfnResourceClass)) {
return false;
}

if (!c.name.startsWith("Cfn")) {
return false;
}

return true;
}

/**
* @returns true if `classType` represents a Construct
*/
public static isConstructClass(c: reflect.ClassType) {
if (!c.system.includesAssembly(CORE_MODULE)) {
return false;
}

if (!c.isClassType()) {
return false;
}

if (c.abstract) {
return false;
}

return c.extends(c.system.findFqn(CoreTypesFqn.Construct));
}

/**
* @returns true if `classType` represents an AWS resource (i.e. extends `cdk.Resource`).
*/
public static isResourceClass(classType: reflect.ClassType) {
const baseResource = classType.system.findClass(CoreTypesFqn.Resource);
return classType.extends(baseResource) || getDocTag(classType, "resource");
}

/**
* @returns `classType` for the core type Construct
*/
public get constructClass() {
return this.sys.findClass(CoreTypesFqn.Construct);
}

/**
* @returns `interfacetype` for the core type Construct
*/
public get constructInterface() {
return this.sys.findInterface(CoreTypesFqn.ConstructInterface);
}

/**
* @returns `classType` for the core type Construct
*/
public get resourceClass() {
return this.sys.findClass(CoreTypesFqn.Resource);
}

/**
* @returns `interfaceType` for the core type Resource
*/
public get resourceInterface() {
return this.sys.findInterface(CoreTypesFqn.ResourceInterface);
}

/**
* @returns `classType` for the core type Token
*/
public get tokenClass() {
return this.sys.findClass(CoreTypesFqn.Token);
}

private readonly sys: TypeSystem;

constructor(sys: reflect.TypeSystem) {
this.sys = sys;
if (!sys.includesAssembly(CORE_MODULE)) {
// disable-all-checks
return;
}

for (const fqn of Object.values(CoreTypesFqn)) {
if (!this.sys.tryFindFqn(fqn)) {
throw new Error(`core FQN type not found: ${fqn}`);
}
}
}
}
Loading

0 comments on commit c41a0f6

Please sign in to comment.