Skip to content

Support new type variable syntax#902

Merged
paracycle merged 14 commits intomainfrom
ko/new-type-member
Apr 29, 2022
Merged

Support new type variable syntax#902
paracycle merged 14 commits intomainfrom
ko/new-type-member

Conversation

@KaanOzkan
Copy link
Contributor

@KaanOzkan KaanOzkan commented Apr 19, 2022

Motivation

Fixes: #901

Implementation

The implementation is best read commit by commit, but generally does the following:

  1. Changes the signature and processing of arguments for the type_member and type_template method patches so that it supports bounds being supplied both as keyword arguments and as the result of calling the block.
  2. Extracted the type variable serialization logic to a helper module, which does a Sorbet feature check to generate the correct type variable serialization (i.e. parameter based one for old Sorbet versions and block based one for new Sorbet versions)
  3. Refactored the create_type_member RBI helper method to be a more generic and finer-grained create_type_variable helper method that expects all type variable parts (that is type, variance, fixed, upper and lower) and serializes it using the serialization helper mentioned in the previous step. This allows all DSL RBIs to properly serialize their type variable annotations.
  4. Finally, bumped the Sorbet version and ported all Tapioca codebase to use the new type variable syntax.

Tests

  • Updated existing tests to test the ability to read both the old and the new type variable syntax and to generate the new type variable syntax in RBI files. Since Tapioca depends on a recent Sorbet version for development now, we can test outputting old type variable syntax in these tests.
  • Added new gem CLI tests to run against an old and a new Sorbet version to test both reading and writing the old and new syntax type variable syntax. These tests complete the coverage for this feature.

@KaanOzkan KaanOzkan force-pushed the ko/new-type-member branch 2 times, most recently from dce46d5 to 0b444e3 Compare April 19, 2022 20:46
@KaanOzkan KaanOzkan requested a review from a team April 20, 2022 14:56
Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

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

Thank you, this is good start. In general, it would be great to have this working both ways, depending on the version of Sorbet this code is working against, so that we can backport this to the 0.7.x series and unblock users.

@paracycle paracycle force-pushed the ko/new-type-member branch 2 times, most recently from 73964b7 to ebe4554 Compare April 26, 2022 22:37
Copy link
Contributor

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Can we add a test showing the different behavior with an old Sorbet version? 🙏

@paracycle paracycle force-pushed the ko/new-type-member branch 3 times, most recently from caf864b to 782948e Compare April 28, 2022 16:38
@paracycle paracycle force-pushed the ko/new-type-member branch from 782948e to 876dcb5 Compare April 28, 2022 20:12
@paracycle paracycle marked this pull request as ready for review April 28, 2022 20:12
@paracycle paracycle requested review from a team, Morriar and vinistock April 28, 2022 20:14
@paracycle paracycle changed the title [WIP] Support new type variable syntax Support new type variable syntax Apr 28, 2022
Copy link
Contributor

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Amazing, thanks! ❤️

@paracycle paracycle merged commit 5c3eb2f into main Apr 29, 2022
@paracycle paracycle deleted the ko/new-type-member branch April 29, 2022 14:22
paracycle added a commit that referenced this pull request Apr 29, 2022
Support new type variable syntax
@paracycle paracycle added the backported Backported to stable branch label Apr 29, 2022
paracycle added a commit that referenced this pull request Apr 29, 2022
Support new type variable syntax
paracycle added a commit that referenced this pull request Apr 29, 2022
Support new type variable syntax
@shopify-shipit shopify-shipit bot temporarily deployed to production May 13, 2022 23:08 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to 0-8-stable May 30, 2022 13:45 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported Backported to stable branch bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The type_member syntax for bounds has changed to use a block instead of keyword args

5 participants