From 208fac3a68d24ba3fe25846e67c750e77261349e Mon Sep 17 00:00:00 2001 From: Kaan Ozkan Date: Tue, 14 Apr 2026 16:16:07 -0400 Subject: [PATCH] Preserve sig param comments in RBS translation Comments inside `params(...)` were lost because Spoom replaced the full Sorbet `sig` range with the generated RBS annotation. Pass parsed comments into `RBI::Parser::SigBuilder` so the `rbi` gem can attach parameter comments to `RBI::SigParam` and force multiline RBS output. Spoom still preserves any non-parameter comments left inside the sig body, such as comments before `returns`. Resolves https://github.com/Shopify/spoom/issues/900 --- .../translate/sorbet_sigs_to_rbs_comments.rb | 18 +++++- lib/spoom/sorbet/translate/translator.rb | 1 + rbi/spoom.rbi | 3 + .../sorbet_sigs_to_rbs_comments_test.rb | 58 +++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) 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 7aa1baa0..5267d834 100644 --- a/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb +++ b/lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb @@ -78,6 +78,7 @@ def visit_def_node(node) out = rbs_print(node.location.start_column) do |printer| printer.print_method_sig(rbi_node, sig) end + out = add_sig_comments(node, out) @rewriter << Source::Replace.new(node.location.start_offset, node.location.end_offset, out) end @@ -179,7 +180,7 @@ def visit_scope(node, &block) def visit_sig(node) return unless sorbet_sig?(node) - builder = RBI::Parser::SigBuilder.new(@ruby_contents, file: @file) + builder = RBI::Parser::SigBuilder.new(@ruby_contents, comments_by_line: @comments_by_line, file: @file) builder.current.loc = node.location builder.visit_call_node(node) builder.current.comments = [] @@ -207,6 +208,7 @@ def visit_attr(node) out = rbs_print(node.location.start_column) do |printer| printer.print_attr_sig(rbi_node, sig) end + out = add_sig_comments(node, out) @rewriter << Source::Replace.new(node.location.start_offset, node.location.end_offset, out) end end @@ -379,6 +381,20 @@ def delete_extend_t_generics @extend_t_generics.clear end + # `RBI::SigBuilder` consumes parameter comments and attaches them to `RBI::SigParam`. + # Preserve any other comments left inside the sig body, like comments before `returns`. + #: (Prism::CallNode, String) -> String + def add_sig_comments(sig_node, out) + comments = @comments_by_line.values.select do |comment| + comment.location.start_offset > sig_node.location.start_offset && + comment.location.end_offset <= sig_node.location.end_offset + end + return out if comments.empty? + + indent = " " * sig_node.location.start_column + comments.map { |comment| "#{comment.slice}\n#{indent}" }.join + out + end + # Collects the last signatures visited and clears the current list #: -> Array[[Prism::CallNode, RBI::Sig]] def collect_last_sigs diff --git a/lib/spoom/sorbet/translate/translator.rb b/lib/spoom/sorbet/translate/translator.rb index b931ebe6..969c4228 100644 --- a/lib/spoom/sorbet/translate/translator.rb +++ b/lib/spoom/sorbet/translate/translator.rb @@ -22,6 +22,7 @@ def initialize(ruby_contents, file:) node, comments = Spoom.parse_ruby(ruby_contents, file: file) @node = node #: Prism::Node @comments = comments #: Array[Prism::Comment] + @comments_by_line = comments.to_h { |comment| [comment.location.start_line, comment] } #: Hash[Integer, Prism::Comment] @ruby_bytes = ruby_contents.bytes #: Array[Integer] @rewriter = Spoom::Source::Rewriter.new #: Source::Rewriter end diff --git a/rbi/spoom.rbi b/rbi/spoom.rbi index 10d9b27c..ada93b61 100644 --- a/rbi/spoom.rbi +++ b/rbi/spoom.rbi @@ -3023,6 +3023,9 @@ class Spoom::Sorbet::Translate::SorbetSigsToRBSComments < ::Spoom::Sorbet::Trans private + sig { params(sig_node: ::Prism::CallNode, out: ::String).returns(::String) } + def add_sig_comments(sig_node, out); end + sig do params( parent: T.any(::Prism::ClassNode, ::Prism::ModuleNode, ::Prism::SingletonClassNode), 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 92619922..55436f49 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 @@ -466,6 +466,64 @@ def foo(param1:, param2:); end RBS end + def test_translate_to_rbs_sig_with_inline_param_comments + contents = <<~RB + sig do + params( + # First param + a: Integer, + # Second param + b: String + ).void + end + def foo(a, b); end + + sig do + # A comment + returns(Integer) + end + attr_reader :x + RB + + assert_equal(<<~RBS, sorbet_sigs_to_rbs_comments(contents)) + #: ( + #| # First param + #| Integer a, + #| # Second param + #| String b + #| ) -> void + def foo(a, b); end + + # A comment + #: Integer + attr_reader :x + RBS + end + + def test_translate_to_rbs_sig_with_inline_param_comments_indented + contents = <<~RB + class Foo + sig do + params( + # First param + a: Integer + ).void + end + def foo(a); end + end + RB + + assert_equal(<<~RBS, sorbet_sigs_to_rbs_comments(contents)) + class Foo + #: ( + #| # First param + #| Integer a + #| ) -> void + def foo(a); end + end + RBS + end + def test_translate_to_rbs_defs_within_send contents = <<~RB sig { void }