-
Couldn't load subscription status.
- Fork 156
Remove T.unsafe from extension method call
#1047
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
Conversation
Sorbet doesn't know that `::FrozenRecord::Base` will have `::Tapioca::Dsl::Compilers::Extensions::FrozenRecord` prepended to its `singleton_class`. Therefore, it does not allow us to call the methods provided by that module. If we tell it that `ConstantType` will be the union of both, it allows us to do so.
| extend T::Sig | ||
|
|
||
| ConstantType = type_member { { fixed: T.class_of(::FrozenRecord::Base) } } | ||
| ConstantType = type_member { { fixed: T.all(T.class_of(::FrozenRecord::Base), Extensions::FrozenRecord) } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an alternative way to explicitly tell Sorbet that T.class_of(::FrozenRecord::Base).is_a?(Extensions::FrozenRecord)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intuition was that it should also be possible to say
sig { override.returns(T::Enumerable[ConstantType]) }
def self.gather_constants
# ...but this doesn't seem to be the case due to the differences between type_member and type_template, which I'm not super familiar with.
Is there a good way to specify both, or is the idea here that it's not worth it since generics are erased at runtime anyway, so Module is sufficient?
| extend T::Sig | ||
|
|
||
| ConstantType = type_member { { fixed: T.class_of(::FrozenRecord::Base) } } | ||
| ConstantType = type_member { { fixed: T.all(T.class_of(::FrozenRecord::Base), Extensions::FrozenRecord) } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so.
|
Concerning the return type of gather_constants, I think it would work to use |
So a few things here:
|
Motivation
Sorbet doesn't know that
::FrozenRecord::Basewill have::Tapioca::Dsl::Compilers::Extensions::FrozenRecordprepended to itssingleton_class. Therefore, it does not allow us to call the method provided by that module.T.unsafeis used to allow the call, which can be improved.Implementation
If we tell Sorbet that
ConstantTypewill be the union of both types, it allows us to call the method.I came up with this approach in #1046, which also makes use of an extension. I did not want to use
T.usafeif possible, so I came up with this approach.Tests
bundle exec srb tcis the test 😅