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

Compile T::Enum with the enums block #243

Merged
merged 3 commits into from
Mar 2, 2021

Conversation

jeffcarbs
Copy link
Contributor

@jeffcarbs jeffcarbs commented Mar 2, 2021

Motivation

With sorbet's T::Enum the enum values themselves are actually instances of the class that inherits from T::Enum. Tapioca doesn't handle these specially which affects sorbet's ability to perform exhaustiveness checking. Here's a repro on sorbet.run.

Implementation

Specially handle T::Enum to define the enums using the enums do ... end block and ignore the enum values from the normal compilation.

Tests

See updated tests.

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.

Looks great, thank you. Just a couple of comments.

lib/tapioca/compilers/symbol_table/symbol_generator.rb Outdated Show resolved Hide resolved
lib/tapioca/compilers/symbol_table/symbol_generator.rb Outdated Show resolved Hide resolved
lib/tapioca/compilers/symbol_table/symbol_generator.rb Outdated Show resolved Hide resolved
@jeffcarbs
Copy link
Contributor Author

@paracycle - Thanks for the suggestions. Just committed them so this should be good for another look.

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.

Yeap, this looks great!

@paracycle
Copy link
Member

@jeffcarbs You had signed the CLA before, right? I think it might be our CLA bot acting up. Do you mind amending your last commit and force-pushing to trigger it again?

Co-authored-by: Ufuk Kayserilioglu <ufuk@paralaus.com>
@jeffcarbs
Copy link
Contributor Author

Amending and force-pushing worked 👍

@paracycle paracycle merged commit 311224d into Shopify:master Mar 2, 2021
@jeffcarbs jeffcarbs deleted the compile-t-enum branch March 2, 2021 22:07
This was referenced Mar 10, 2021
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