From 2f1ecd69fc1650d1e92e70c69bddd0b4fab509be Mon Sep 17 00:00:00 2001 From: Hashim1999164 <64767361+Hashim1999164@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:44:12 +0500 Subject: [PATCH 1/3] Rename colliding type parameters when translating sigs to RBS --- .../translate/sorbet_sigs_to_rbs_comments.rb | 128 ++++++++++++++++++ .../sorbet_sigs_to_rbs_comments_test.rb | 87 ++++++++++++ 2 files changed, 215 insertions(+) diff --git a/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb b/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb index 91608d88..035097f3 100644 --- a/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb +++ b/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb @@ -79,6 +79,10 @@ def visit_def_node(node) last_sigs.each do |node, sig| next if sig.is_abstract && !@translate_abstract_methods + # RBS type variables shadow constants and class type members with the + # same name. Rename colliding method type parameters before printing. + rename_colliding_type_params!(sig) + preserve_multiline_signatures = !!(@preserve_multiline_signatures && sig.loc&.multiline?) out = rbs_print( @@ -388,6 +392,50 @@ def delete_extend_t_generics @extend_t_generics.clear end + #: (RBI::Sig) -> void + def rename_colliding_type_params!(sig) + return unless sig.type_params? + + occupied = simple_type_names(sig) + class_type_member_names + mapping = {} #: Hash[String, String] + + sig.type_params.each do |type_param| + next unless occupied.any? { |name| name == type_param || name.start_with?("#{type_param}::") } + + mapping[type_param] = unique_type_param_name(type_param, occupied + mapping.values) + end + + return if mapping.empty? + + sig.type_params.map! { |type_param| mapping[type_param] || type_param } + TypeParameterRenamer.new(mapping).visit_sig(sig) + end + + #: (RBI::Sig) -> Array[String] + def simple_type_names(sig) + collector = SimpleTypeNameCollector.new + collector.visit_sig(sig) + collector.names + end + + #: -> Array[String] + def class_type_member_names + @type_members.map do |member| + member.sub(/^(in|out)\s+/, "").split(/[\s<=]/).first #: as String + end + end + + #: (String, Array[String]) -> String + def unique_type_param_name(type_param, occupied) + candidate = "#{type_param}T" + suffix = 2 + while occupied.include?(candidate) || occupied.any? { |name| name.start_with?("#{candidate}::") } + candidate = "#{type_param}T#{suffix}" + suffix += 1 + end + candidate + end + # Collects the last signatures visited and clears the current list #: -> Array[[Prism::CallNode, RBI::Sig]] def collect_last_sigs @@ -417,6 +465,86 @@ def rbs_print(indent, preserve_multiline_signatures:, &block) end end.join + "\n" end + + class SigTypeWalker + #: (RBI::Sig) -> void + def visit_sig(sig) + sig.params.each do |param| + type = coerce_type(param.type) + param.instance_variable_set(:@type, type) + visit_type(type) + end + return_type = coerce_type(sig.return_type) + sig.return_type = return_type + visit_type(return_type) + end + + #: (RBI::Type | String) -> RBI::Type + def coerce_type(type) + type.is_a?(String) ? RBI::Type.parse_string(type) : type + end + + #: (RBI::Type) -> void + def visit_type(type) + case type + when RBI::Type::Simple + visit_simple(type) + when RBI::Type::TypeParameter + visit_type_parameter(type) + when RBI::Type::Generic + type.params.each { |param| visit_type(param) } + when RBI::Type::All, RBI::Type::Any, RBI::Type::Tuple + type.types.each { |inner| visit_type(inner) } + when RBI::Type::Nilable, RBI::Type::Class, RBI::Type::Module + visit_type(type.type) + when RBI::Type::ClassOf + visit_type(type.type) + type.type_parameters.each { |param| visit_type(param) } + when RBI::Type::TypeAlias + visit_type(type.aliased_type) + when RBI::Type::Proc + type.proc_params.each_value { |param| visit_type(param) } + visit_type(type.proc_returns) if type.proc_returns + visit_type(type.proc_bind) if type.proc_bind + when RBI::Type::Shape + type.types.each_value { |inner| visit_type(inner) } + end + end + + #: (RBI::Type::Simple) -> void + def visit_simple(type); end + + #: (RBI::Type::TypeParameter) -> void + def visit_type_parameter(type); end + end + + class SimpleTypeNameCollector < SigTypeWalker + #: Array[String] + attr_reader :names + + #: -> void + def initialize + @names = [] #: Array[String] + end + + #: (RBI::Type::Simple) -> void + def visit_simple(type) + @names << type.name + end + end + + class TypeParameterRenamer < SigTypeWalker + #: (Hash[String, String]) -> void + def initialize(mapping) + @mapping = mapping #: Hash[String, String] + end + + #: (RBI::Type::TypeParameter) -> void + def visit_type_parameter(type) + new_name = @mapping[type.name.to_s] + type.instance_variable_set(:@name, new_name.to_sym) if new_name + end + end end end end diff --git a/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb b/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb index 4bd7a0fd..d0e91639 100644 --- a/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb +++ b/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb @@ -621,6 +621,93 @@ def baz(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, RBS end + def test_translate_type_parameter_that_collides_with_a_constant + contents = <<~RB + class Store + module Credential + module Multiple; end + end + + sig do + type_parameters(:Credential) + .params(type: T.all(T::Module[T.type_parameter(:Credential)], T::Module[Credential::Multiple])) + .returns(T::Array[T.type_parameter(:Credential)]) + end + def find(type) + [] + end + end + RB + + assert_equal(<<~RBS, sorbet_sigs_to_rbs_comments(contents)) + class Store + module Credential + module Multiple; end + end + + #: [CredentialT] ( + #| (Module[CredentialT] & Module[Credential::Multiple]) type + #| ) -> Array[CredentialT] + def find(type) + [] + end + end + RBS + end + + def test_translate_type_parameter_that_collides_with_a_class_type_member + contents = <<~RB + class Box + extend T::Generic + + Elem = type_member + + sig do + type_parameters(:Elem) + .params(value: T.type_parameter(:Elem)) + .returns(Elem) + end + def wrap(value) + value + end + end + RB + + assert_equal(<<~RBS, sorbet_sigs_to_rbs_comments(contents)) + #: [Elem] + class Box + #: [ElemT] ( + #| ElemT value + #| ) -> Elem + def wrap(value) + value + end + end + RBS + end + + def test_translate_type_parameter_without_name_collision + contents = <<~RB + sig do + type_parameters(:U) + .params(value: T.type_parameter(:U)) + .returns(T.type_parameter(:U)) + end + def identity(value) + value + end + RB + + assert_equal(<<~RBS, sorbet_sigs_to_rbs_comments(contents)) + #: [U] ( + #| U value + #| ) -> U + def identity(value) + value + end + RBS + end + private #: ( From 4482ddda0348c44c6359cbe1b6d63aade7167aec Mon Sep 17 00:00:00 2001 From: Hashim1999164 <64767361+Hashim1999164@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:48:19 +0500 Subject: [PATCH 2/3] Raise when type parameters collide during sig to RBS translation --- .../translate/sorbet_sigs_to_rbs_comments.rb | 39 +++---------------- .../sorbet_sigs_to_rbs_comments_test.rb | 39 +++++++------------ 2 files changed, 19 insertions(+), 59 deletions(-) diff --git a/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb b/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb index 035097f3..07677778 100644 --- a/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb +++ b/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb @@ -80,8 +80,9 @@ def visit_def_node(node) next if sig.is_abstract && !@translate_abstract_methods # RBS type variables shadow constants and class type members with the - # same name. Rename colliding method type parameters before printing. - rename_colliding_type_params!(sig) + # same name. Refuse colliding method type parameters so the user can + # rename them and keep names consistent. + raise_on_colliding_type_params!(sig) preserve_multiline_signatures = !!(@preserve_multiline_signatures && sig.loc&.multiline?) @@ -393,22 +394,16 @@ def delete_extend_t_generics end #: (RBI::Sig) -> void - def rename_colliding_type_params!(sig) + def raise_on_colliding_type_params!(sig) return unless sig.type_params? occupied = simple_type_names(sig) + class_type_member_names - mapping = {} #: Hash[String, String] sig.type_params.each do |type_param| next unless occupied.any? { |name| name == type_param || name.start_with?("#{type_param}::") } - mapping[type_param] = unique_type_param_name(type_param, occupied + mapping.values) + raise Error, "Type parameter `#{type_param}` collides with a constant or class type member. Rename the type parameter to avoid the collision." end - - return if mapping.empty? - - sig.type_params.map! { |type_param| mapping[type_param] || type_param } - TypeParameterRenamer.new(mapping).visit_sig(sig) end #: (RBI::Sig) -> Array[String] @@ -425,17 +420,6 @@ def class_type_member_names end end - #: (String, Array[String]) -> String - def unique_type_param_name(type_param, occupied) - candidate = "#{type_param}T" - suffix = 2 - while occupied.include?(candidate) || occupied.any? { |name| name.start_with?("#{candidate}::") } - candidate = "#{type_param}T#{suffix}" - suffix += 1 - end - candidate - end - # Collects the last signatures visited and clears the current list #: -> Array[[Prism::CallNode, RBI::Sig]] def collect_last_sigs @@ -532,19 +516,6 @@ def visit_simple(type) @names << type.name end end - - class TypeParameterRenamer < SigTypeWalker - #: (Hash[String, String]) -> void - def initialize(mapping) - @mapping = mapping #: Hash[String, String] - end - - #: (RBI::Type::TypeParameter) -> void - def visit_type_parameter(type) - new_name = @mapping[type.name.to_s] - type.instance_variable_set(:@name, new_name.to_sym) if new_name - end - end end end end diff --git a/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb b/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb index d0e91639..181271a5 100644 --- a/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb +++ b/test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb @@ -639,20 +639,13 @@ def find(type) end RB - assert_equal(<<~RBS, sorbet_sigs_to_rbs_comments(contents)) - class Store - module Credential - module Multiple; end - end - - #: [CredentialT] ( - #| (Module[CredentialT] & Module[Credential::Multiple]) type - #| ) -> Array[CredentialT] - def find(type) - [] - end - end - RBS + error = assert_raises(Translate::Error) do + sorbet_sigs_to_rbs_comments(contents) + end + assert_equal( + "Type parameter `Credential` collides with a constant or class type member. Rename the type parameter to avoid the collision.", + error.message, + ) end def test_translate_type_parameter_that_collides_with_a_class_type_member @@ -673,17 +666,13 @@ def wrap(value) end RB - assert_equal(<<~RBS, sorbet_sigs_to_rbs_comments(contents)) - #: [Elem] - class Box - #: [ElemT] ( - #| ElemT value - #| ) -> Elem - def wrap(value) - value - end - end - RBS + error = assert_raises(Translate::Error) do + sorbet_sigs_to_rbs_comments(contents) + end + assert_equal( + "Type parameter `Elem` collides with a constant or class type member. Rename the type parameter to avoid the collision.", + error.message, + ) end def test_translate_type_parameter_without_name_collision From 85505f62f89ae60d024d90668588a1341be4eef4 Mon Sep 17 00:00:00 2001 From: Hashim Khan <64767361+Hashim1999164@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:38:06 +0500 Subject: [PATCH 3/3] Fix Sorbet nilable narrowing for proc return and bind types Assign proc_returns and proc_bind to locals before visit_type so Sorbet can narrow T.nilable(RBI::Type) down to RBI::Type. --- lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb b/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb index 07677778..5baa1e5b 100644 --- a/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb +++ b/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb @@ -488,8 +488,12 @@ def visit_type(type) visit_type(type.aliased_type) when RBI::Type::Proc type.proc_params.each_value { |param| visit_type(param) } - visit_type(type.proc_returns) if type.proc_returns - visit_type(type.proc_bind) if type.proc_bind + if (proc_returns = type.proc_returns) + visit_type(proc_returns) + end + if (proc_bind = type.proc_bind) + visit_type(proc_bind) + end when RBI::Type::Shape type.types.each_value { |inner| visit_type(inner) } end