Skip to content

Commit

Permalink
refactor: Construct props must not use the 'any' type (awslint:props-…
Browse files Browse the repository at this point in the history
…no-any) (#2701)

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

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

BREAKING CHANGE: 
* SNS - Subscription `endpoint` is now type `string` (previously `any`)
* Step Functions - `result` in the Pass state is now type `map` (previously `any`)

**Fixes #2673**
  • Loading branch information
shivlaks authored May 31, 2019
1 parent 08a2852 commit cb2b334
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 13 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-sns/lib/subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export interface SubscriptionProps {
*
* The meaning of this value depends on the value for 'protocol'.
*/
readonly endpoint: any;
readonly endpoint: string;

/**
* The topic to subscribe to.
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-stepfunctions/lib/states/pass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface PassProps {
*
* @default No injected result
*/
readonly result?: any;
readonly result?: {[key: string]: any};
}

/**
Expand Down
3 changes: 3 additions & 0 deletions packages/@aws-cdk/cdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
"construct-ctor:@aws-cdk/cdk.App.<initializer>",
"construct-ctor:@aws-cdk/cdk.Root.<initializer>",
"construct-ctor:@aws-cdk/cdk.Stack.<initializer>.params*",
"props-no-any:@aws-cdk/cdk.CfnOutputProps.value",
"props-no-any:@aws-cdk/cdk.CfnParameterProps.default",
"props-no-any:@aws-cdk/cdk.CfnResourceProps.properties",
"props-no-cfn-types:@aws-cdk/cdk.CfnOutputProps*",
"props-no-cfn-types:@aws-cdk/cdk.StringListCfnOutputProps*"
]
Expand Down
5 changes: 5 additions & 0 deletions packages/@aws-cdk/runtime-values/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,10 @@
},
"engines": {
"node": ">= 8.10.0"
},
"awslint": {
"exclude": [
"props-no-any:@aws-cdk/runtime-values.RuntimeValueProps.value"
]
}
}
38 changes: 27 additions & 11 deletions tools/awslint/lib/rules/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,12 @@ constructLinter.add({

constructLinter.add({
code: 'props-no-unions',
message: 'props should not use TypeScript unions',
message: 'props must not use TypeScript unions',
eval: e => {
if (!e.ctx.propsType) { return; }
if (!e.ctx.hasPropsArgument) { return; }

// this rule only applies to L2 constructs
// this rule does not apply to L1 constructs
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.ownProperties) {
Expand All @@ -205,12 +205,12 @@ constructLinter.add({

constructLinter.add({
code: 'props-no-arn-refs',
message: 'props should use strong types instead of attributes. props should not have "arn" suffix',
message: 'props must use strong types instead of attributes. props should not have "arn" suffix',
eval: e => {
if (!e.ctx.propsType) { return; }
if (!e.ctx.hasPropsArgument) { return; }

// this rule only applies to L2 constructs
// this rule does not apply to L1 constructs
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.ownProperties) {
Expand All @@ -221,12 +221,12 @@ constructLinter.add({

constructLinter.add({
code: 'props-no-tokens',
message: 'props should not use the "Token" type',
message: 'props must not use the "Token" type',
eval: e => {
if (!e.ctx.propsType) { return; }
if (!e.ctx.hasPropsArgument) { return; }

// this rule only applies to L2 constructs
// this rule does not apply to L1 constructs
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.allProperties) {
Expand All @@ -242,12 +242,12 @@ constructLinter.add({

constructLinter.add({
code: 'props-no-cfn-types',
message: 'props should not expose L1 types (types which start with "Cfn")',
message: 'props must 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
// this rule does not apply to L1 constructs
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.ownProperties) {
Expand All @@ -263,17 +263,33 @@ constructLinter.add({

constructLinter.add({
code: 'props-default-doc',
message: 'All optional props should have @default documentation',
message: 'All optional props must have @default documentation',
eval: e => {
if (!e.ctx.propsType) { return; }
if (!e.ctx.hasPropsArgument) { return; }

// this rule only applies to L2 constructs
// this rule does not apply to L1 constructs
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.allProperties) {
if (!property.optional) { continue; }
e.assert(property.docs.docs.default !== undefined, `${e.ctx.propsFqn}.${property.name}`);
}
}
});
});

constructLinter.add({
code: 'props-no-any',
message: 'props must not use Typescript "any" type',
eval: e => {
if (!e.ctx.propsType) { return; }
if (!e.ctx.hasPropsArgument) { return; }

// this rule does not apply to L1 constructs
if (CoreTypes.isCfnResource(e.ctx.classType)) { return; }

for (const property of e.ctx.propsType.ownProperties) {
e.assert(!property.type.isAny, `${e.ctx.propsFqn}.${property.name}`);
}
}
});

0 comments on commit cb2b334

Please sign in to comment.