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

Identical static methods in parent/child classes are mistakenly marked as overrides in java resulting in a compilation error #2358

Closed
2 of 4 tasks
iliapolo opened this issue Dec 15, 2020 · 1 comment · Fixed by #2373
Assignees
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort language/dotnet Related to .NET bindings (C#, F#, ...) language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module p1

Comments

@iliapolo
Copy link
Contributor

🐛 Bug Report

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...) [Probably]

General Information

  • JSII Version: 1.15.0 (build 585166b), typescript 3.9.7
  • Platform: MacOS, CodeBuild (jsii/superchain)

What is the problem?

We encountered a scenario in aws-cdk that generates illegal java code, annotating static methods with the @Override annotation.

This happens when a child class has the same static method as its parent.

export class Parent {
  public static hello() {}
}

export class Child extends Parent {
  public static hello() {}
}

This code will generate a JSII assembly that marks the Child.hello function both as static and as overrides of Parent.hello.
The generated java code will look like:

public class Child extends Parent {

  @Override
  public static hello() {}

}

Resulting in a compilation error: method does not override or implement a method from a supertype

See aws/aws-cdk#12091 for the full scenario.

@iliapolo iliapolo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 15, 2020
@SomayaB SomayaB added language/dotnet Related to .NET bindings (C#, F#, ...) language/java Related to Java bindings labels Dec 16, 2020
@RomainMuller RomainMuller added effort/small Small work item – less than a day of effort module/pacmak Issues affecting the `jsii-pacmak` module p1 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 21, 2020
@RomainMuller RomainMuller added this to the 🎄 December Release milestone Dec 21, 2020
@aws aws deleted a comment from allcontributors bot Dec 21, 2020
@aws aws deleted a comment from allcontributors bot Dec 21, 2020
RomainMuller added a commit that referenced this issue Dec 21, 2020
Stop emitting "overrides" in the respecting language idiomatic way for
static methods and properties (while ES6 supports those, this is not
true of Java and C#).

In Java, this is mostly getting rid of `@Overrides` on static
declarations. C# *allows* (but does not require) explicitly hiding the
parent declaration using the `new` keyword. This was introduced as it
provides additional safeguards against our generating incorrect code.

Fixes #2358
RomainMuller added a commit that referenced this issue Dec 21, 2020
Stop emitting "overrides" in the respecting language idiomatic way for
static methods and properties (while ES6 supports those, this is not
true of Java and C#).

In Java, this is mostly getting rid of `@Overrides` on static
declarations. C# *allows* (but does not require) explicitly hiding the
parent declaration using the `new` keyword. This was introduced as it
provides additional safeguards against our generating incorrect code.

Fixes #2358
RomainMuller added a commit that referenced this issue Dec 21, 2020
Stop emitting "overrides" in the respecting language idiomatic way for
static methods and properties (while ES6 supports those, this is not
true of Java and C#).

In Java, this is mostly getting rid of `@Overrides` on static
declarations. C# *allows* (but does not require) explicitly hiding the
parent declaration using the `new` keyword. This was introduced as it
provides additional safeguards against our generating incorrect code.

Fixes #2358
@mergify mergify bot closed this as completed in #2373 Dec 22, 2020
mergify bot pushed a commit that referenced this issue Dec 22, 2020
Stop emitting "overrides" in the respecting language idiomatic way for
static methods and properties (while ES6 supports those, this is not
true of Java and C#).

In Java, this is mostly getting rid of `@Overrides` on static
declarations. C# *allows* (but does not require) explicitly hiding the
parent declaration using the `new` keyword. This was introduced as it
provides additional safeguards against our generating incorrect code.

Fixes #2358



---

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

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort language/dotnet Related to .NET bindings (C#, F#, ...) language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants