Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Construct props must not use the 'any' type (awslint:props-no-any) #2701

Merged
merged 1 commit into from
May 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call this out as a BREAKING CHANGE in the commit message that you merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do


/**
* 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};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call this out as a BREAKING CHANGE in the commit message that you merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

}

/**
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}`);
}
}
});