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.

Sign up for GitHub

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

Jump to bottom

Java: method overloads are not generated (regression) #233

Closed
eladb opened this issue Sep 17, 2018 · 7 comments
Closed

Java: method overloads are not generated (regression) #233

eladb opened this issue Sep 17, 2018 · 7 comments
Assignees
@RomainMuller
Labels
bug This issue is a bug.

Comments

@eladb
Copy link
Contributor

eladb commented Sep 17, 2018 •
edited
Loading

Oddly, method overloads are no longer generated (see CDK Bucket constructor, and Bucket.grantRead examples)
@eladb eladb added the bug This issue is a bug. label Sep 17, 2018
@eladb eladb changed the title Java: method overloads are not generated, at least for ctors (regression) Java: method overloads are not generated (regression) Sep 17, 2018
eladb pushed a commit to aws/aws-cdk that referenced this issue Sep 17, 2018
Fix java init template
6fa9748 
Fixes #711

Seems like jsii stopped generating method overloads
which is the reason some of the method signatures
caused compilation errors.

Also, IIdentityResource was renamed to IPrincipal.

We should revisit once aws/jsii#233 is fixed.

But at least we want to make sure the init template
compiles.
@eladb eladb mentioned this issue Sep 17, 2018
Merged
@RomainMuller RomainMuller self-assigned this Sep 17, 2018
@RomainMuller
Copy link
Contributor

Looking.

@RomainMuller
Copy link
Contributor

RomainMuller commented Sep 17, 2018 •
edited
Loading

One thing I see already is that in the constructor of Bucket, props is not modelled as optional, despite it carries a default value. This is a bug in the jsii compiler. I am almost certain the issue for Bucket.grantRead is the same.

eladb pushed a commit to aws/aws-cdk that referenced this issue Sep 17, 2018
fix(cdk): temporary fix to java "cdk init" template (#732)
281c083 
Fixes #711

Seems like jsii stopped generating method overloads
which is the reason some of the method signatures
caused compilation errors.

Also, IIdentityResource was renamed to IPrincipal.

We should revisit once aws/jsii#233 is fixed.

But at least we want to make sure the init template
compiles.
@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 17, 2018

We should prohibit overloads at the jsii level anyway.

Golang doesn't support them, for example.

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 17, 2018

Nor Rust, should we ever want to target that.

@RomainMuller
Copy link
Contributor

RomainMuller commented Sep 17, 2018 •
edited
Loading

They're not really overrides per the model... We're making overrides without the trailing optional arguments to make Javaists' lives nicer.

@eladb
Copy link
Contributor Author

eladb commented Sep 17, 2018

Exactly. So languages that do not support them can only generate the method and allow nulls to be passed for optional arguments

RomainMuller added a commit that referenced this issue Sep 17, 2018
@RomainMuller
fix(jsii): Defaulted parameters were not rendered as optional
29ed0b1 
Parameters with a default value were not represented as optional in the assembly
documents due to an oversight when re-writing `jsii`. There was also no coverage
in the test corpus for this use-case, so I've added some.

Fixes #233
RomainMuller added a commit that referenced this issue Sep 17, 2018
@RomainMuller
fix(jsii): Defaulted parameters were not rendered as optional
8208b67 
Parameters with a default value were not represented as optional in the assembly
documents due to an oversight when re-writing `jsii`. There was also no coverage
in the test corpus for this use-case, so I've added some.

Fixes #233
@RomainMuller RomainMuller mentioned this issue Sep 17, 2018
Merged
@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 17, 2018

Oh I see, overloads as an implementation mechanism. That's fine.

RomainMuller added a commit that referenced this issue Sep 17, 2018
@RomainMuller
fix(jsii): Defaulted parameters were not rendered as optional (#234)
578bf9c 
Parameters with a default value were not represented as optional in the assembly
documents due to an oversight when re-writing `jsii`. There was also no coverage
in the test corpus for this use-case, so I've added some.

Fixes #233
costleya added a commit to costleya/jsii that referenced this issue Sep 20, 2018
@costleya
fix: dotnet abstract classes should have proxy implementations
eaa4dba 
Abstract classes have can implementations that are not shared thorugh jsii. When trying to instantiate one for these types as a
return value, the runtime crashes. This change creates proxy class types for abstract classes. These types can be instantiated,
and have implementations for any methods/properties marked as abstract.

Fixes aws#233
eladb pushed a commit that referenced this issue Sep 20, 2018
v0.7.6 (#239)
04cb6bb 
Bug Fixes
======================
 * Sphinx generated incorrect type references for display ([#232](#232)) ([b664805](b664805))
* **jsii:** Defaulted parameters were not rendered as optional ([#234](#234)) ([578bf9c](578bf9c)), closes [#233](#233)
* **jsii:** Don't skip emit on TS errors when in "watch" mode ([#236](#236)) ([30d1491](30d1491)), closes [#235](#235)
* **jsii:** Optional `any` represented as required ([#237](#237)) ([91074f3](91074f3)), closes [#230](#230)

Features
======================
 * **sphinx:** allow readme file to define sphinx header and reorganize topic ([#229](#229)) ([405da9c](405da9c)), closes [#228](#228) [#185](#185)
* Document overriden/inherited members ([#238](#238)) ([7a6278a](7a6278a)), closes [#196](#196)
eladb pushed a commit to aws/aws-cdk that referenced this issue Sep 20, 2018
v0.9.2
0636316 
__NOTICE__: This release includes a framework-wide [__breaking
change__](#712) which changes the type
of all the string resource attributes across the framework. Instead of using
strong-types that extend `cdk.Token` (such as `QueueArn`, `TopicName`, etc), we
now represent all these attributes as normal `string`s, and codify the tokens
into the string (using the feature introduced in [#168](#168)).

Furthermore, the `cdk.Arn` type has been removed. In order to format/parse ARNs,
use the static methods on `cdk.ArnUtils`.

See motivation and discussion in [#695](#695).

* **cfn2ts:** use stringified tokens for resource attributes instead of strong types ([#712](#712)) ([6508f78](6508f78)), closes [#518](#518) [#695](#695) [#744](#744)
* **aws-dynamodb:** Attribute type for keys, changes the signature of the `addPartitionKey` and `addSortKey` methods to be consistent across the board. ([#720](#720)) ([e6cc189](e6cc189))
* **aws-codebuild:** fix typo "priviledged" -> "privileged

* **assets:** cab't use multiple assets in the same stack ([#725](#725)) ([bba2e5b](bba2e5b)), closes [#706](#706)
* **aws-codebuild:** typo in BuildEnvironment "priviledged" -> "privileged     ([#734](#734)) ([72fec36](72fec36))
* **aws-ecr:** fix addToResourcePolicy ([#737](#737)) ([eadbda5](eadbda5))
* **aws-events:** ruleName can now be specified ([#726](#726)) ([a7bc5ee](a7bc5ee)), closes [#708](#708)
* **aws-lambda:** jsii use no long requires 'sourceAccount' ([#728](#728)) ([9e7d311](9e7d311)), closes [#714](#714)
* **aws-s3:** remove `policy` argument ([#730](#730)) ([a79190c](a79190c)), closes [#672](#672)
* **cdk:** "cdk init" java template is broken ([#732](#732)) ([281c083](281c083)), closes [#711](#711) [aws/jsii#233](aws/jsii#233)

* **aws-apigateway:** new API Gateway Construct Library ([#665](#665)) ([b0f3857](b0f3857))
* **aws-cdk:** detect presence of EC2 credentials ([#724](#724)) ([8e8c295](8e8c295)), closes [#702](#702) [#130](#130)
* **aws-codepipeline:** make the Stage insertion API in CodePipeline more flexible ([#460](#460)) ([d182818](d182818))
* **aws-codepipeline:** new "Pipeline#addStage" convenience method ([#647](#647)) ([25c9fa0](25c9fa0))
* **aws-rds:** add support for parameter groups ([#729](#729)) ([2541508](2541508)), closes [#719](#719)
* **docs:** add documentation for CDK toolkit plugings ([#733](#733)) ([965b918](965b918))
* **dependencies:** upgrade to [jsii 0.7.6](https://github.com/awslabs/jsii/releases/tag/v0.7.6)
eladb pushed a commit to aws/aws-cdk that referenced this issue Sep 20, 2018
v0.9.2 (#749)
d68b6c3 
* v0.9.2

__NOTICE__: This release includes a framework-wide [__breaking
change__](#712) which changes the type
of all the string resource attributes across the framework. Instead of using
strong-types that extend `cdk.Token` (such as `QueueArn`, `TopicName`, etc), we
now represent all these attributes as normal `string`s, and codify the tokens
into the string (using the feature introduced in [#168](#168)).

Furthermore, the `cdk.Arn` type has been removed. In order to format/parse ARNs,
use the static methods on `cdk.ArnUtils`.

See motivation and discussion in [#695](#695).

* **cfn2ts:** use stringified tokens for resource attributes instead of strong types ([#712](#712)) ([6508f78](6508f78)), closes [#518](#518) [#695](#695) [#744](#744)
* **aws-dynamodb:** Attribute type for keys, changes the signature of the `addPartitionKey` and `addSortKey` methods to be consistent across the board. ([#720](#720)) ([e6cc189](e6cc189))
* **aws-codebuild:** fix typo "priviledged" -> "privileged

* **assets:** cab't use multiple assets in the same stack ([#725](#725)) ([bba2e5b](bba2e5b)), closes [#706](#706)
* **aws-codebuild:** typo in BuildEnvironment "priviledged" -> "privileged     ([#734](#734)) ([72fec36](72fec36))
* **aws-ecr:** fix addToResourcePolicy ([#737](#737)) ([eadbda5](eadbda5))
* **aws-events:** ruleName can now be specified ([#726](#726)) ([a7bc5ee](a7bc5ee)), closes [#708](#708)
* **aws-lambda:** jsii use no long requires 'sourceAccount' ([#728](#728)) ([9e7d311](9e7d311)), closes [#714](#714)
* **aws-s3:** remove `policy` argument ([#730](#730)) ([a79190c](a79190c)), closes [#672](#672)
* **cdk:** "cdk init" java template is broken ([#732](#732)) ([281c083](281c083)), closes [#711](#711) [aws/jsii#233](aws/jsii#233)

* **aws-apigateway:** new API Gateway Construct Library ([#665](#665)) ([b0f3857](b0f3857))
* **aws-cdk:** detect presence of EC2 credentials ([#724](#724)) ([8e8c295](8e8c295)), closes [#702](#702) [#130](#130)
* **aws-codepipeline:** make the Stage insertion API in CodePipeline more flexible ([#460](#460)) ([d182818](d182818))
* **aws-codepipeline:** new "Pipeline#addStage" convenience method ([#647](#647)) ([25c9fa0](25c9fa0))
* **aws-rds:** add support for parameter groups ([#729](#729)) ([2541508](2541508)), closes [#719](#719)
* **docs:** add documentation for CDK toolkit plugings ([#733](#733)) ([965b918](965b918))
* **dependencies:** upgrade to [jsii 0.7.6](https://github.com/awslabs/jsii/releases/tag/v0.7.6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RomainMuller RomainMuller

Labels
bug This issue is a bug.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@RomainMuller @rix0rrr @eladb