From d90525d2d226a3837fa7120fa70829d3df4a7e48 Mon Sep 17 00:00:00 2001 From: Kaan Ozkan Date: Tue, 31 Aug 2021 10:30:48 -0400 Subject: [PATCH 1/3] Use qualified name for types 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. https://github.com/Shopify/tapioca/pull/466 --- lib/tapioca/sorbet_ext/name_patch.rb | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/tapioca/sorbet_ext/name_patch.rb b/lib/tapioca/sorbet_ext/name_patch.rb index 294cb18e8..681008702 100644 --- a/lib/tapioca/sorbet_ext/name_patch.rb +++ b/lib/tapioca/sorbet_ext/name_patch.rb @@ -6,7 +6,22 @@ module Types class Simple module NamePatch def name - @name ||= ::Tapioca::Reflection.name_of(@raw_type).freeze + @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 From 427de13b8f2d76494da78a71283347b6ae8e4eb2 Mon Sep 17 00:00:00 2001 From: Kaan Ozkan Date: Tue, 31 Aug 2021 16:30:24 -0400 Subject: [PATCH 2/3] Apply necessary changes to the tests --- .../dsl/action_controller_helpers_spec.rb | 6 +- .../compilers/dsl/action_mailer_spec.rb | 2 +- spec/tapioca/compilers/dsl/active_job_spec.rb | 4 +- .../dsl/active_record_columns_spec.rb | 16 ++--- .../active_support_current_attributes_spec.rb | 2 +- .../compilers/dsl/sidekiq_worker_spec.rb | 6 +- .../compilers/symbol_table_compiler_spec.rb | 68 ++++++++++--------- 7 files changed, 53 insertions(+), 51 deletions(-) diff --git a/spec/tapioca/compilers/dsl/action_controller_helpers_spec.rb b/spec/tapioca/compilers/dsl/action_controller_helpers_spec.rb index 628a0c601..02ea8ce5c 100644 --- a/spec/tapioca/compilers/dsl/action_controller_helpers_spec.rb +++ b/spec/tapioca/compilers/dsl/action_controller_helpers_spec.rb @@ -161,7 +161,7 @@ module HelperMethods sig { returns(T.untyped) } def current_user_name; end - sig { params(user_id: Integer).void } + sig { params(user_id: ::Integer).void } def notify_user(user_id); end end @@ -206,7 +206,7 @@ module HelperMethods sig { returns(T.untyped) } def current_user_name; end - sig { params(user_id: Integer).void } + sig { params(user_id: ::Integer).void } def notify_user(user_id); end end @@ -251,7 +251,7 @@ module HelperMethods sig { params(user: T.untyped).returns(T.untyped) } def greet(user); end - sig { params(user_id: Integer).void } + sig { params(user_id: ::Integer).void } def notify_user(user_id); end end diff --git a/spec/tapioca/compilers/dsl/action_mailer_spec.rb b/spec/tapioca/compilers/dsl/action_mailer_spec.rb index 2d9d01c02..25c2ebebd 100644 --- a/spec/tapioca/compilers/dsl/action_mailer_spec.rb +++ b/spec/tapioca/compilers/dsl/action_mailer_spec.rb @@ -99,7 +99,7 @@ def notify_customer(customer_id) # typed: strong class NotifierMailer - sig { params(customer_id: Integer).returns(::ActionMailer::MessageDelivery) } + sig { params(customer_id: ::Integer).returns(::ActionMailer::MessageDelivery) } def self.notify_customer(customer_id); end end RBI diff --git a/spec/tapioca/compilers/dsl/active_job_spec.rb b/spec/tapioca/compilers/dsl/active_job_spec.rb index e45ace7aa..5d50a33c0 100644 --- a/spec/tapioca/compilers/dsl/active_job_spec.rb +++ b/spec/tapioca/compilers/dsl/active_job_spec.rb @@ -87,10 +87,10 @@ def perform(user_id) # typed: strong class NotifyJob - sig { params(user_id: Integer).returns(T.any(NotifyJob, FalseClass)) } + sig { params(user_id: ::Integer).returns(T.any(NotifyJob, FalseClass)) } def self.perform_later(user_id); end - sig { params(user_id: Integer).void } + sig { params(user_id: ::Integer).void } def self.perform_now(user_id); end end RBI diff --git a/spec/tapioca/compilers/dsl/active_record_columns_spec.rb b/spec/tapioca/compilers/dsl/active_record_columns_spec.rb index 3266472e2..a7ebc268b 100644 --- a/spec/tapioca/compilers/dsl/active_record_columns_spec.rb +++ b/spec/tapioca/compilers/dsl/active_record_columns_spec.rb @@ -575,10 +575,10 @@ class Post < ActiveRecord::Base RUBY expected = indented(<<~RBI, 4) - sig { returns(T.nilable(CustomType)) } + sig { returns(T.nilable(::CustomType)) } def cost; end - sig { params(value: T.nilable(CustomType)).returns(T.nilable(CustomType)) } + sig { params(value: T.nilable(::CustomType)).returns(T.nilable(::CustomType)) } def cost=(value); end RBI @@ -624,10 +624,10 @@ class Post < ActiveRecord::Base RUBY expected = indented(<<~RBI, 4) - sig { returns(T.nilable(T.any(CustomType, Numeric))) } + sig { returns(T.nilable(T.any(::CustomType, ::Numeric))) } def cost; end - sig { params(value: T.nilable(T.any(CustomType, Numeric))).returns(T.nilable(T.any(CustomType, Numeric))) } + sig { params(value: T.nilable(T.any(::CustomType, ::Numeric))).returns(T.nilable(T.any(::CustomType, ::Numeric))) } def cost=(value); end RBI @@ -672,10 +672,10 @@ class Post < ActiveRecord::Base RUBY expected = indented(<<~RBI, 4) - sig { returns(T.nilable(CustomType)) } + sig { returns(T.nilable(::CustomType)) } def cost; end - sig { params(value: T.nilable(CustomType)).returns(T.nilable(CustomType)) } + sig { params(value: T.nilable(::CustomType)).returns(T.nilable(::CustomType)) } def cost=(value); end RBI @@ -717,10 +717,10 @@ class Post < ActiveRecord::Base RUBY expected = indented(<<~RUBY, 4) - sig { returns(T.nilable(ValueType[Integer])) } + sig { returns(T.nilable(ValueType[::Integer])) } def cost; end - sig { params(value: T.nilable(ValueType[Integer])).returns(T.nilable(ValueType[Integer])) } + sig { params(value: T.nilable(ValueType[::Integer])).returns(T.nilable(ValueType[::Integer])) } def cost=(value); end RUBY diff --git a/spec/tapioca/compilers/dsl/active_support_current_attributes_spec.rb b/spec/tapioca/compilers/dsl/active_support_current_attributes_spec.rb index d2baf9b27..8c9bda7ed 100644 --- a/spec/tapioca/compilers/dsl/active_support_current_attributes_spec.rb +++ b/spec/tapioca/compilers/dsl/active_support_current_attributes_spec.rb @@ -110,7 +110,7 @@ def self.account; end sig { params(value: T.untyped).returns(T.untyped) } def self.account=(value); end - sig { params(user_id: Integer).void } + sig { params(user_id: ::Integer).void } def self.authenticate(user_id); end sig { returns(T.untyped) } diff --git a/spec/tapioca/compilers/dsl/sidekiq_worker_spec.rb b/spec/tapioca/compilers/dsl/sidekiq_worker_spec.rb index 698e1508f..c5317a5e3 100644 --- a/spec/tapioca/compilers/dsl/sidekiq_worker_spec.rb +++ b/spec/tapioca/compilers/dsl/sidekiq_worker_spec.rb @@ -85,13 +85,13 @@ def perform(customer_id) # typed: strong class NotifierWorker - sig { params(customer_id: Integer).returns(String) } + sig { params(customer_id: ::Integer).returns(String) } 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) } def self.perform_at(interval, customer_id); end - sig { params(interval: Numeric, customer_id: Integer).returns(String) } + sig { params(interval: Numeric, customer_id: ::Integer).returns(String) } def self.perform_in(interval, customer_id); end end RBI diff --git a/spec/tapioca/compilers/symbol_table_compiler_spec.rb b/spec/tapioca/compilers/symbol_table_compiler_spec.rb index eb247c4e1..5942f1b36 100644 --- a/spec/tapioca/compilers/symbol_table_compiler_spec.rb +++ b/spec/tapioca/compilers/symbol_table_compiler_spec.rb @@ -92,7 +92,7 @@ module Bar end Bar::Arr = T.let(T.unsafe(nil), Array) - Bar::Foo = T.type_alias { T.any(String, Symbol) } + Bar::Foo = T.type_alias { T.any(::String, ::Symbol) } RBI assert_equal(output, compile) @@ -113,7 +113,7 @@ class B; end module Bar; end module Bar::A; end class Bar::A::B; end - Bar::A::Foo = T.type_alias { T.any(Bar::A, Bar::A::B, String, Symbol) } + Bar::A::Foo = T.type_alias { T.any(::Bar::A, ::Bar::A::B, ::String, ::Symbol) } RBI assert_equal(output, compile) @@ -2268,9 +2268,9 @@ class Adt::Foo end class Bar < ::T::Struct - prop :bar, String - const :baz, T::Hash[String, T.untyped] - const :foo, Integer + prop :bar, ::String + const :baz, T::Hash[::String, T.untyped] + const :foo, ::Integer prop :quux, T.untyped, default: T.unsafe(nil) class << self @@ -2288,25 +2288,25 @@ def do_it; end end class Buzz - prop :bar, String - const :baz, T.proc.params(arg0: String).void - const :foo, Integer + prop :bar, ::String + const :baz, T.proc.params(arg0: ::String).void + const :foo, ::Integer end class Foo - sig { params(a: Integer, b: String).returns(Integer) } + sig { params(a: ::Integer, b: ::String).returns(::Integer) } def bar(a, b:); end sig { type_parameters(:U).params(a: T.type_parameter(:U)).returns(T.type_parameter(:U)) } def baz(a); end - sig { params(a: Integer, b: String).void } + sig { params(a: ::Integer, b: ::String).void } def foo(a, b:); end - sig { params(a: Integer, b: Integer, c: Integer, d: Integer, e: Integer, f: Integer, blk: T.proc.void).void } + sig { params(a: ::Integer, b: ::Integer, c: ::Integer, d: ::Integer, e: ::Integer, f: ::Integer, blk: T.proc.void).void } def many_kinds_of_args(*a, b, c, d:, e: T.unsafe(nil), **f, &blk); end - sig { returns(T.proc.params(x: String).void) } + sig { returns(T.proc.params(x: ::String).void) } def some_attribute; end class << self @@ -2324,8 +2324,8 @@ class Generics::ComplexGenericType B = type_template(:out) C = type_template D = type_member(fixed: Integer) - E = type_member(fixed: Integer, upper: T::Array[Numeric]) - F = type_member(fixed: Integer, lower: T.any(Complex, T::Hash[Symbol, T::Array[Integer]]), upper: T.nilable(Numeric)) + E = type_member(fixed: Integer, upper: T::Array[::Numeric]) + F = type_member(fixed: Integer, lower: T.any(::Complex, T::Hash[::Symbol, T::Array[::Integer]]), upper: T.nilable(::Numeric)) class << self extend T::Generic @@ -2355,30 +2355,30 @@ def complex(foo); end def something(foo); end end - Generics::SimpleGenericType::NullGenericType = T.let(T.unsafe(nil), Generics::SimpleGenericType[Integer]) + Generics::SimpleGenericType::NullGenericType = T.let(T.unsafe(nil), Generics::SimpleGenericType[::Integer]) module Quux interface! - sig { abstract.returns(Integer) } + sig { abstract.returns(::Integer) } def something; end end class Quux::Concrete include ::Quux - sig { returns(String) } + sig { returns(::String) } def bar; end - sig { params(baz: T::Hash[String, Object]).returns(T::Hash[String, Object]) } + sig { params(baz: T::Hash[::String, ::Object]).returns(T::Hash[::String, ::Object]) } def baz=(baz); end - sig { returns(T::Array[Integer]) } + sig { returns(T::Array[::Integer]) } def foo; end def foo=(_arg0); end - sig { override.returns(Integer) } + sig { override.returns(::Integer) } def something; end end RBI @@ -2399,7 +2399,7 @@ def foo(params) output = template(<<~RBI) class Foo - sig { params(params: {"foo" => Integer, bar: String, :"foo bar" => Class}).void } + sig { params(params: {"foo" => ::Integer, bar: ::String, :"foo bar" => ::Class}).void } def foo(params); end end RBI @@ -2562,19 +2562,19 @@ class Foo < T::Struct output = template(<<~RBI) class Foo < ::T::Struct - prop :a, T.nilable(Integer), default: T.unsafe(nil) + prop :a, T.nilable(::Integer), default: T.unsafe(nil) prop :b, T::Boolean, default: T.unsafe(nil) prop :c, T::Boolean, default: T.unsafe(nil) - prop :d, Symbol, default: T.unsafe(nil) - prop :e, String, default: T.unsafe(nil) - prop :f, Integer, default: T.unsafe(nil) - prop :g, Float, default: T.unsafe(nil) - prop :h, T::Array[String], default: T.unsafe(nil) - prop :i, T::Hash[String, Integer], default: T.unsafe(nil) - prop :k, Foo, default: T.unsafe(nil) - prop :l, T::Array[Foo], default: T.unsafe(nil) - prop :m, T::Hash[Foo, Foo], default: T.unsafe(nil) - prop :n, Foo, default: T.unsafe(nil) + prop :d, ::Symbol, default: T.unsafe(nil) + prop :e, ::String, default: T.unsafe(nil) + prop :f, ::Integer, default: T.unsafe(nil) + prop :g, ::Float, default: T.unsafe(nil) + prop :h, T::Array[::String], default: T.unsafe(nil) + prop :i, T::Hash[::String, ::Integer], default: T.unsafe(nil) + prop :k, ::Foo, default: T.unsafe(nil) + prop :l, T::Array[::Foo], default: T.unsafe(nil) + prop :m, T::Hash[::Foo, ::Foo], default: T.unsafe(nil) + prop :n, ::Foo, default: T.unsafe(nil) class << self def inherited(s); end @@ -2712,6 +2712,8 @@ def c RUBY output = template(<<~RBI) + ::Foo::FooAttachedClass = Foo::FooAttachedClass + class Foo class << self sig { returns(T.attached_class) } @@ -2720,7 +2722,7 @@ def a; end sig { returns(T::Hash[T.attached_class, T::Array[T.attached_class]]) } def b; end - sig { returns(Foo::FooAttachedClass) } + sig { returns(::Foo::FooAttachedClass) } def c; end end end From fb329842c6a3ede2dd6447ad96cf4eb440b87ab1 Mon Sep 17 00:00:00 2001 From: Ufuk Kayserilioglu Date: Wed, 1 Sep 2021 21:45:05 +0300 Subject: [PATCH 3/3] Strip leading `::` when adding to symbol queue 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. --- lib/tapioca/compilers/symbol_table/symbol_generator.rb | 4 ++++ spec/tapioca/compilers/symbol_table_compiler_spec.rb | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/tapioca/compilers/symbol_table/symbol_generator.rb b/lib/tapioca/compilers/symbol_table/symbol_generator.rb index 906be2c48..930249746 100644 --- a/lib/tapioca/compilers/symbol_table/symbol_generator.rb +++ b/lib/tapioca/compilers/symbol_table/symbol_generator.rb @@ -38,7 +38,11 @@ def generate private + sig { params(name: T.nilable(String)).void } def add_to_symbol_queue(name) + return unless name + + name = name.delete_prefix("::") @symbol_queue << name unless symbols.include?(name) || symbol_ignored?(name) end diff --git a/spec/tapioca/compilers/symbol_table_compiler_spec.rb b/spec/tapioca/compilers/symbol_table_compiler_spec.rb index 5942f1b36..1d96d1b5a 100644 --- a/spec/tapioca/compilers/symbol_table_compiler_spec.rb +++ b/spec/tapioca/compilers/symbol_table_compiler_spec.rb @@ -2712,8 +2712,6 @@ def c RUBY output = template(<<~RBI) - ::Foo::FooAttachedClass = Foo::FooAttachedClass - class Foo class << self sig { returns(T.attached_class) }