Skip to content

Conversation

@dstufft
Copy link
Contributor

@dstufft dstufft commented Mar 12, 2019

In the JSII, we generate a proxy class for any Abstract classes. This proxy class inherits from the abstract class, and does nothing but provide implementations of any abstract methods/properties that just call into the JSII. However, this only worked for abstract methods defined on this specific abstract class, not any parent abstract classes.

This PR resolves that issue, by building up a list of all abstract classes in an abstract class's inheritance tree, and then has the proxy depend on it.

Because the actual name of the abstract class is not part of a module's public interface, this PR adds a jsii.proxy_for function, which takes an abstract class and returns the proxy class for it.

Before:

class Value(scope.jsii_calc_base.Base, metaclass=jsii.JSIIAbstractClass, jsii_type="@scope/jsii-calc-lib.Value"):
    @staticmethod
    def __jsii_proxy_class__():
        return _ValueProxy

    @property
    @jsii.member(jsii_name="value")
    @abc.abstractmethod
    def value(self) -> jsii.Number:
        ...

    ...


class _ValueProxy(Value):
    @property
    @jsii.member(jsii_name="value")
    def value(self) -> jsii.Number:
        return jsii.get(self, "value")

After:

class Value(scope.jsii_calc_base.Base, metaclass=jsii.JSIIAbstractClass, jsii_type="@scope/jsii-calc-lib.Value"):
    @staticmethod
    def __jsii_proxy_class__():
        return _ValueProxy

    @property
    @jsii.member(jsii_name="value")
    @abc.abstractmethod
    def value(self) -> jsii.Number:
        ...

    ...


class _ValueProxy(Value, jsii.proxy_for(scope.jsii_calc_base.Base)):
    @property
    @jsii.member(jsii_name="value")
    def value(self) -> jsii.Number:
        return jsii.get(self, "value")

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

@dstufft dstufft requested a review from a team as a code owner March 12, 2019 22:54
const base = this.findType(cls.base.fqn);

if (!spec.isClassType(base)) {
throw new Error("Class inheritence that isn't a class?");
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a super-useful error message. Under what circumstances would we end up here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Theoretically never I think. We would end up here if a class inherited from something that wasn't a class, like a primitive type or an interface or something, which shouldn't be possible AFAIK. However the types technically allow it so it's possible that a bug or something causes the JSII gets into a state where it has a class inheriting from something that isn't a class.

This basically exists for two reasons:

  • findType doesn't return a ClassType, it returns a Type and using spec.IsClassType like this informs the compiler that base is a ClassType if it gets past this if statement.
  • Emulates an assert statement to verify that some assumption of the world holds true, but that should never not be true.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, understood.

@dstufft dstufft merged commit 6f1c9c0 into aws:master Mar 13, 2019
@dstufft dstufft deleted the abstract-proxy branch March 13, 2019 11:14
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.

3 participants