Skip to content

Preserve inline comments inside sig blocks during RBS translation #901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KaanOzkan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Preserve inline comments inside sig blocks during RBS translation #901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_inline_sig_comments(node, out)
@rewriter << Source::Replace.new(node.location.start_offset, node.location.end_offset, out)
end

Expand Down Expand Up @@ -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_inline_sig_comments(node, out)
@rewriter << Source::Replace.new(node.location.start_offset, node.location.end_offset, out)
end
end
Expand Down Expand Up @@ -379,6 +381,19 @@ def delete_extend_t_generics
@extend_t_generics.clear
end

#: (Prism::CallNode, String) -> String
def add_inline_sig_comments(sig_node, out)
inline_comments = @comments.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 inline_comments.empty?

indent_str = " " * sig_node.location.start_column
comment_string = inline_comments.map { |c| "#{c.slice}\n#{indent_str}" }.join
comment_string + out
end

# Collects the last signatures visited and clears the current list
#: -> Array[[Prism::CallNode, RBI::Sig]]
def collect_last_sigs
Expand Down
3 changes: 3 additions & 0 deletions rbi/spoom.rbi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_inline_sig_comments(sig_node, out); end

sig do
params(
parent: T.any(::Prism::ClassNode, ::Prism::ModuleNode, ::Prism::SingletonClassNode),
Expand Down
53 changes: 53 additions & 0 deletions test/spoom/sorbet/translate/sorbet_sigs_to_rbs_comments_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,59 @@ 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
# Second param
#: (Integer a, String b) -> void
Copy link
Copy Markdown
Contributor

@Morriar Morriar Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should force multi line rendering when we find comments.

Consider this:

params(
  # Some documentation
  a: Integer
).void
def foo(a); end

when translated we get this:

# Some documentation
#: (a) -> void
def foo(a); end

it will look as if the documentation is about foo and not a.

Copy link
Copy Markdown
Contributor Author

@KaanOzkan KaanOzkan Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah this makes sense. I think we can either:

  1. Associate comments with parameters in RBI and teach RBSPrinter about outputting it
  2. Keep doing the string post-processing in Spoom, similar to the new add_inline_sig_comments. We could figure out the multiline aspect by looking at the indentation and insert the comment to the relevant line. We'd also need to add a new flag or set max_line_length to 0 for the RBSPrinter to print multiline comment.

I'm leaning towards 1 since we're getting too much into the printing territory. Wdyt?

Copy link
Copy Markdown
Contributor

@Morriar Morriar Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, 1. 👍

def foo(a, b); end

# A comment
#: Integer
attr_reader :x
Comment thread
KaanOzkan marked this conversation as resolved.
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 }
Expand Down
Loading