Fix root namespace #472
Fix root namespace #472
Conversation
| def self.perform_async(customer_id); end | ||
|
|
||
| sig { params(interval: T.any(DateTime, Time), customer_id: Integer).returns(String) } | ||
| sig { params(interval: T.any(DateTime, Time), customer_id: ::Integer).returns(String) } |
There was a problem hiding this comment.
String, DateTime, Time not affected by the name patch.
There was a problem hiding this comment.
Hmm, this is interesting. I would have expected this to have been fixed too.
There was a problem hiding this comment.
The reason these aren't changing is because they're hard-coded in the code: https://github.com/Shopify/tapioca/blob/main/lib/tapioca/compilers/dsl/sidekiq_worker.rb#L59.
The Integer param is derived from the live ruby code:
https://github.com/Shopify/tapioca/blob/main/lib/tapioca/compilers/dsl/sidekiq_worker.rb#L59
I wonder if there should be some helper that allows us to specify these manual types via code and have it get compiled to a string.
lib/tapioca/sorbet_ext/name_patch.rb
Outdated
| @name ||= ::Tapioca::Reflection.qualified_name_of(@raw_type).freeze | ||
| end | ||
| end | ||
|
|
||
| T::Types::Simple::Private::Pool.instance_variable_set(:@cache, ObjectSpace::WeakMap.new) |
There was a problem hiding this comment.
The problem apparently was that existing instances of T::Types::Simple had memoized their names in the @name ivar, so they were holding on to that even after we prepend the patch.
The fix is to use a different memoizing ivar in the prepended module, so that they all start fresh. However, that causes an infinite loop descending into the qualified_name_of method on Reflection since the method has a signature which tries to do the same thing recursively.
Thus, we need a copy of the qualified_name_of method here without a signature to fix everything:
| @name ||= ::Tapioca::Reflection.qualified_name_of(@raw_type).freeze | |
| end | |
| end | |
| T::Types::Simple::Private::Pool.instance_variable_set(:@cache, ObjectSpace::WeakMap.new) | |
| @qualified_name ||= qualified_name_of(@raw_type).freeze | |
| end | |
| private | |
| NAME_METHOD = Module.instance_method(:name) | |
| def qualified_name_of(constant) | |
| name = NAME_METHOD.bind(constant).call | |
| return if name.nil? | |
| if name.start_with?("::") | |
| name | |
| else | |
| "::#{name}" | |
| end | |
| end | |
| end |
There was a problem hiding this comment.
This fixed the T.nilable conversions. However, there are still some types that are unaffected by the patch in the tests. Not sure if it's a blocker for merging this PR.
There was a problem hiding this comment.
Which ones for example? This should only affect the things we generate inside sigs and other Sorbet runtime related things, like enums and tstructs. Everything else, we need to do manually.
There was a problem hiding this comment.
Qualified name specifies the name from the root namespace. This will prevent errors in the future where we have clashing constant names that were separated by their fully qualified names. During RBI generation we used to drop the leading `::` anchors and the constant would point to a wrong type than what the developer intended. e.g. #466
KaanOzkan
force-pushed
the
ko/fix-root-namespace
branch
from
6b9ca80 to
6f03ce6
Compare
KaanOzkan
force-pushed
the
ko/fix-root-namespace
branch
from
6f03ce6 to
427de13
Compare
The symbol queue operates on the full names of constants and does not know how to handle a name that starts with a `::`. If such a name is added to the queue, it looks like it is an alias of a constant with the same name but without the `::` prefix.
|
Superseded by #780 |
Motivation
Implementation
Tests