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

Deprecated API element remover does the wrong thing in case of deprecated base class #3025

Open
1 of 5 tasks
rix0rrr opened this issue Oct 5, 2021 · 0 comments
Open
1 of 5 tasks
Labels
bug This issue is a bug. language/dotnet Related to .NET bindings (C#, F#, ...) module/pacmak Issues affecting the `jsii-pacmak` module p1

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 5, 2021

🐛 Bug Report

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)
  • Go

What is the problem?

The deprecated API element remover generates incorrect code based on the following sources:

interface ISomeInterface {
  someMethod(): void;
}

/** @deprecated */
class SomeBaseClass implements ISomeInterface {
  public someMethod() {
    console.log('some method');
  }
}


class Subclass extends SomeBaseclass {
}

What happens?

SomeBaseClass gets completely removed, but the fact that Subclass implements ISomeInterface does get propagated (as it should).

End result is that Subclass does not have an implementation for someMethod, and so in fact DOES NOT end up implementing ISomeInterface.

This definitely fails in C#... might affect other languages too. In C# the class ends up like this:

class Subclass : DeputyBase, ISomeInterface {
  // <constructors here, not interesting>
}

And we then get an error that the implementation for SomeMethod is missing.

@rix0rrr rix0rrr added bug This issue is a bug. language/dotnet Related to .NET bindings (C#, F#, ...) needs-triage This issue or PR still needs to be triaged. labels Oct 5, 2021
rix0rrr added a commit to aws/aws-cdk that referenced this issue Oct 5, 2021
This class was deprecated because it should not have been used directly:

- It is used more as a base class for other images
- It should be accessed through factory functions on `MachineImage`.
- It has an uppercased acronym `SSM`, which should have been spelled as
  `Ssm`.

The deprecated API stripper does the wrong thing when a base class is
deprecated however, and leaves `WindowsImage` in a broken state. See
aws/jsii#3025

Temporarily rolling this back until we can fix jsii.
mergify bot pushed a commit to aws/aws-cdk that referenced this issue Oct 5, 2021
This class was deprecated because it should not have been used directly:

- It is used more as a base class for other images
- It should be accessed through factory functions on `MachineImage`.
- It has an uppercased acronym `SSM`, which should have been spelled as
  `Ssm`.

The deprecated API stripper does the wrong thing when a base class is
deprecated however, and leaves `WindowsImage` in a broken state. See
aws/jsii#3025

Temporarily rolling this back until we can fix jsii.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit to aws/aws-cdk that referenced this issue Oct 5, 2021
This class was deprecated because it should not have been used directly:

- It is used more as a base class for other images
- It should be accessed through factory functions on `MachineImage`.
- It has an uppercased acronym `SSM`, which should have been spelled as
  `Ssm`.

The deprecated API stripper does the wrong thing when a base class is
deprecated however, and leaves `WindowsImage` in a broken state. See
aws/jsii#3025

Temporarily rolling this back until we can fix jsii.

----

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

(cherry picked from commit 5dfc507)
@NGL321 NGL321 added module/pacmak Issues affecting the `jsii-pacmak` module p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jan 7, 2022
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. language/dotnet Related to .NET bindings (C#, F#, ...) module/pacmak Issues affecting the `jsii-pacmak` module p1
Projects
None yet
Development

No branches or pull requests

2 participants