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

Abstract properties are generated in abstract classes. #240

Closed
costleya opened this issue Sep 21, 2018 · 0 comments · Fixed by #1128
Closed

Abstract properties are generated in abstract classes. #240

costleya opened this issue Sep 21, 2018 · 0 comments · Fixed by #1128
Assignees
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress Issue is being actively worked on. language/dotnet Related to .NET bindings (C#, F#, ...) p1

Comments

@costleya
Copy link
Contributor

costleya commented Sep 21, 2018

The .NET generator will generate jsii properties marked abstract as concrete properties with implementations. This does not break anything - superclasses will override these properties with the same body. However, it is unnecessary.

Example:

"jsii-calc.AbstractClassBase": {
      "abstract": true,
      "assembly": "jsii-calc",
      "fqn": "jsii-calc.AbstractClassBase",
      "initializer": {
        "initializer": true
      },
      "kind": "class",
      "name": "AbstractClassBase",
      "properties": [
        {
          "abstract": true,
          "immutable": true,
          "name": "abstractProperty",
          "type": {
            "primitive": "string"
          }
        }
      ]
    },

Generates:

[JsiiClass(typeof(AbstractClassBase), "jsii-calc.AbstractClassBase", "[]")]
public abstract class AbstractClassBase : DeputyBase
{
    protected AbstractClassBase(): base(new DeputyProps(new object[]{}))
    {
    }

    protected AbstractClassBase(ByRefValue reference): base(reference)
    {
    }

    protected AbstractClassBase(DeputyProps props): base(props)
    {
    }

    [JsiiProperty("abstractProperty", "{\"primitive\":\"string\"}")]
    public virtual string AbstractProperty
    {
       get => GetInstanceProperty<string>();
    }
 }
@costleya costleya added bug This issue is a bug. language/dotnet Related to .NET bindings (C#, F#, ...) labels Sep 21, 2018
@costleya costleya changed the title Abstract properties have are generated. Abstract properties are generated in abstract classes. Sep 21, 2018
@fulghum fulghum added the p0 label Apr 2, 2019
@fulghum fulghum added p1 effort/small Small work item – less than a day of effort and removed p0 labels Apr 22, 2019
@RomainMuller RomainMuller added this to the .NET Support milestone Jul 30, 2019
RomainMuller added a commit that referenced this issue Dec 16, 2019
The generated code for abstract properties in Java and C# included fully
concrete implementations, instead of an abstract declaration. This made
it possible to subclass those types without actually implementing those
members, resulting in invalid code.

This changes the code generation to actually emit the `abstract` keyword
and not generate a full concrete implementation.

Fixes #240
Fixes #1011
@SomayaB SomayaB added the in-progress Issue is being actively worked on. label Dec 17, 2019
@mergify mergify bot closed this as completed in #1128 Dec 19, 2019
mergify bot pushed a commit that referenced this issue Dec 19, 2019
…1128)

* fix(java,dotnet): abstract properties have concrete implementations

The generated code for abstract properties in Java and C# included fully
concrete implementations, instead of an abstract declaration. This made
it possible to subclass those types without actually implementing those
members, resulting in invalid code.

This changes the code generation to actually emit the `abstract` keyword
and not generate a full concrete implementation.

Fixes #240
Fixes #1011

* add some test coverage
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 in-progress Issue is being actively worked on. language/dotnet Related to .NET bindings (C#, F#, ...) p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants