fix(python): user defined __jsii_proxy_class
attributes are not preserved
#4625
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #4611, we added the
_jsii_proxy_class__
attributes to the@jsii.interface
implementations. This was required in order to comply withtypeguard
protocol checking. We didn't implement it correctly, accidentally overriding user defined proxy classes.Note
I have been wrecking my brain trying to understand if this bug has any runtime implications, and I couldn't find any.
How so?
At runtime, from what I could gather, the
__jsii_proxy_class__
attribute is only used when we try to instantiate a subclass of an abstract class:jsii/packages/@jsii/python-runtime/src/jsii/_reference_map.py
Lines 65 to 70 in dc77d6c
However, for abstract classes, we assign an explicit value to
__jsii_proxy_class__
:jsii/packages/jsii-pacmak/lib/targets/python.ts
Lines 1496 to 1501 in dc77d6c
Luckily, this happens AFTER the
@jsii.implements
decorator has finished, thus overriding the mistake in the decorator.Presumably, this would still be a problem for user defined abstract classes (since they don't have this assignment). However, reference resolving for user defined classes is done via native reference lookup:
jsii/packages/@jsii/python-runtime/src/jsii/_reference_map.py
Lines 48 to 54 in dc77d6c
This is also why I couldn't come up with a real life test case, and had to resort to an artificial one.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.