Serialize generic types with unspecified type variables #329
Conversation
paracycle
force-pushed
the
uk-fix-generic-const-type
branch
from
bb956b5
to
8cf9a74
Compare
The generic serialization code is serializing generic types with unspecified type variables incorrectly. Sorbet currently allows one to type `Foo.new` even if `Foo` is a generic type that expects type variables, and treats `Foo.new` as if it was `Foo[T.untyped].new` or similar. However, Sorbet complains about `T.let(T.unsafe(nil), Foo)` usage, with the error message saying that `Foo` is a generic class without type arguments. Thus, if the user code has a constant definition like `FOO = Foo.new` (where `Foo` is a generic class), then the generated RBI code for it, `FOO = T.let(T.unsafe(nil), Foo)` would end up raising a type checking error. The fix is to treat generic types differently when grabbing their name. If their name does not already have the type variables, then we add `T.untyped` for each of the type variables it would expect and generate the type name like that.
paracycle
force-pushed
the
uk-fix-generic-const-type
branch
from
8cf9a74
to
5e9ce77
Compare
|
|
||
| klass_name = if klass == ObjectSpace::WeakMap | ||
| # WeakMap is an implicit generic with one type variable | ||
| "ObjectSpace::WeakMap[T.untyped]" |
There was a problem hiding this comment.
Why can't we treat this one like the other classes extending T::Generic? https://github.com/sorbet/sorbet/blob/master/rbi/stdlib/objspace.rbi#L170-L171
There was a problem hiding this comment.
As you point out, the fact that this is generic is encoded in the RBI file, but no artifact of that exists at runtime.
There was a problem hiding this comment.
I wonder if such a list should exist in sorbet-runtime so it can "tag" known generic types?
There was a problem hiding this comment.
TBH, there aren't that many of them, and, IMO, ObjectSpace::WeakMap shouldn't be marked as a generic type anyway. I'll see what upstream thinks about making it non-generic.
There was a problem hiding this comment.
Looking at it again, I think it was meant to be:
class ObjectSpace::WeakMap < Object
include Enumerable
extend T::Generic
Elem = type_member(:out, fixed: T.untyped)
# ...like how other types that have to include Enumerable do it: https://github.com/Shopify/sorbet/blob/master/rbi/core/struct.rbi/#L31-L35
There was a problem hiding this comment.
I think sorbet-runtime could leave some info in the instance about this so we can pick it up from tapioca
Motivation
The generic serialization code is serializing generic types with unspecified type variables incorrectly. Sorbet currently allows one to type
Foo.neweven ifFoois a generic type that expects type variables, and treatsFoo.newas if it wasFoo[T.untyped].newor similar.However, Sorbet complains about
T.let(T.unsafe(nil), Foo)usage, with the error message saying thatFoois a generic class without type arguments.Thus, if the user code has a constant definition like
FOO = Foo.new(whereFoois a generic class), then the generated RBI code for it,FOO = T.let(T.unsafe(nil), Foo)would end up raising a type checking error.Implementation
The fix is to treat generic types differently when grabbing their name. If their name does not already have the type variables, then we add
T.untypedfor each of the type variables it would expect and generate the type name like that.Tests
Added an extra test to check for this specific case.