Conversation
|
We have a policy of testing changes to Sorbet against Stripe's codebase before Stripe employees can see the build results here: → https://go/builds/bui_NQ7H7u7bqmkoBn |
|
We have a policy of testing changes to Sorbet against Stripe's codebase before Stripe employees can see the build results here: → https://go/builds/bui_NQ8JRPoOUUAqBB |
|
We have a policy of testing changes to Sorbet against Stripe's codebase before Stripe employees can see the build results here: → https://go/builds/bui_NQ8UKcD7YkM3Ik |
jez
force-pushed
the
jez-attachable
branch
2 times, most recently
from
7384135 to
651b14a
Compare
jez
force-pushed
the
jez-t-class
branch
2 times, most recently
from
6d250f6 to
ee5222d
Compare
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
There was a problem hiding this comment.
The changes in here are one of the subtle points—previously we used rewriter to say "you can only do this in a module" but now we use namer. It still shows up correctly on the fast path, but shows up in a place where we can allow Symbols::Class as the one class that allows this too (no other classes).
rbi/core/class.rbi
Outdated
| extend T::Generic | ||
| has_attached_class!(:out) |
There was a problem hiding this comment.
The hard part of this change is just two lines. (before has_attached_class!, this change involved a lot of work and scoping).
core/types/calls.cc
Outdated
| // TODO(jez) After T::Class change: | ||
| // We have some weird handling for `initialize` in dispatchCallSymbol to avoid reporting | ||
| // "method does not exist" error. We can simply delete that, and check | ||
| // `res.main.method.exists()` and only report the res.main.errors if we actually dispatched | ||
| // to a real `initialize` method. |
There was a problem hiding this comment.
Related: we can delete the SelfNew intrinsic after this T::Class of PR
| // TODO(jez) After T::Class change: fix the payload, fix all the codebases, and remove this check. | ||
| // (Leaving at least one version in between, so that there is a published version that | ||
| // supports both `Class` and `T::Class` as valid syntax.) | ||
| if (klass != core::Symbols::Class() && | ||
| (klass.isBuiltinGenericForwarder() || klass.data(ctx)->typeArity(ctx) > 0)) { | ||
| // Class is not isLegacyStdlibGeneric (because its type members don't default to T.untyped), | ||
| // but we want to report this syntax error at `# typed: strict` like other stdlib classes. | ||
| auto level = klass.isLegacyStdlibGeneric() || klass == core::Symbols::Class() | ||
| ? core::errors::Resolver::GenericClassWithoutTypeArgsStdlib | ||
| : core::errors::Resolver::GenericClassWithoutTypeArgs; |
There was a problem hiding this comment.
This should let users incrementally migrate, so there will be at least one version published that allows T::Class but does not require it.
I don't think that this refactoring is worth doing. I spent a few minutes looking more carefully at it and decided it wasn't going to work the way I had expected it might.
|
This is so cool! |
Sorbet now considers `Class` to be a generic (sorbet/sorbet#6781), which means that annotations continuing to use the bare `Class` in type signatures will cause type checking errors on newer versions of Sorbet.
* wip: Relax constraint that `initializable!` can only be in module * Add T::Class * wip * wip * Fix Object#class * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * Fixup tests * wip * wip * wip * wip * Remove special case * fixup * Write the docs * Add another test * wip * update cli tests * wip * update exp files * wip * Actually, delete this TODO I don't think that this refactoring is worth doing. I spent a few minutes looking more carefully at it and decided it wasn't going to work the way I had expected it might. * Fix rubocop * Code review fixes * Forgot to implement a T::Types::Base method * Fix the tests * Another subtyping problem * I write so many of these
Since sorbet/sorbet#6781 was merged and released as 0.5.10820, we no longer need to check for the existence of generic class support in Sorbet. Since we are moving to >= 0.5.10820, we can remove all code related to the `generic_class` flag.
Since sorbet/sorbet#6781 was merged and released as 0.5.10820, we no longer need to check for the existence of generic class support in Sorbet. Since we are moving to >= 0.5.10820, we can remove all code related to the `generic_class` flag.
3 participants
Motivation
Fixes #62
Test plan
See included automated tests.