Skip to content

feat(java): detect & rename members named after reserved words#705

Merged
RomainMuller merged 4 commits intomasterfrom
bmaizels/jsii-java-reserved-words
Aug 14, 2019
Merged

feat(java): detect & rename members named after reserved words#705
RomainMuller merged 4 commits intomasterfrom
bmaizels/jsii-java-reserved-words

Conversation

@bmaizels
Copy link
Copy Markdown
Contributor

@bmaizels bmaizels commented Aug 13, 2019


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…ds found in properties and method names to avoid compiler errors
@bmaizels bmaizels requested a review from a team as a code owner August 13, 2019 21:50
Copy link
Copy Markdown
Contributor

@RomainMuller RomainMuller left a comment

Choose a reason for hiding this comment

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

Would advise adding proactive collision checks, so we bail out with an explicit error message instead of generating code that won't build...

Comment thread packages/jsii-pacmak/lib/targets/java.ts Outdated
}

if (JavaGenerator.RESERVED_KEYWORDS.includes(propertyName)) {
return `${propertyName}Value`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might be worth checking whether this is the name of an existing property on the same type... If that is so, just bail out saying you're out of options there...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Difficult to do, so in the interests of unblocking CDK builds moving ahead without this functionality for now.

}

if (JavaGenerator.RESERVED_KEYWORDS.includes(methodName)) {
return `do${toPascalCase(methodName)}`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above, would be nice to have a collision check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Difficult to do, so in the interests of unblocking CDK builds moving ahead without this functionality for now.

@RomainMuller
Copy link
Copy Markdown
Contributor

Looks good - just waiting for a confirmation build of the CDK to succeed before I approve this... To be safe...

@RomainMuller RomainMuller changed the title feat(java): added automatic check and modification of reserved wor… feat(java): detect & rename members named after reserved words Aug 14, 2019
@RomainMuller RomainMuller merged commit 32bc117 into master Aug 14, 2019
@RomainMuller RomainMuller deleted the bmaizels/jsii-java-reserved-words branch August 14, 2019 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants