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

Refactor generic patch to work properly with instance creation #268

Merged
merged 2 commits into from
Mar 24, 2021

Conversation

paracycle
Copy link
Member

Motivation

Our current generic patch was failing with initialize methods on generic types with signatures. The reason is that given a generic type Foo, its initialize method would be wrapped in a Sorbet signature wrapper with an ultimate bind(self).call to the original method in the wrapper.

However, if we have something like Foo[Bar].new then, the self in that wrapper would be an instance of a clone of the Foo class, and will not be an instance of the Foo class. Thus, self.is_a?(Foo) would not be true in the wrapper, and the bind call would fail with a TypeError.

Implementation

This was happening because we were cloning the constants that were given to us, which is fine for modules that don't have instances but fail like above for classes that will have instances. The fix is to create a subclass for classes instead of a clone, so that an instance of the class that represents Foo[Bar] will still be an instance of Foo.

Tests

Expanded the tests to add the failure cases and verified that the TypeError was raised in the same way with the changed tests. Then implemented the fix to ensure that everything is still in order.

@paracycle paracycle requested a review from a team March 23, 2021 22:34
@paracycle paracycle merged commit c7af49d into master Mar 24, 2021
@paracycle paracycle deleted the uk-improve-generic-patch branch March 24, 2021 15:47
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.

None yet

3 participants