Preserve comments inside Sorbet signatures by Morriar · Pull Request #653 · Shopify/rbi

Morriar · 2026-08-19T18:20:25Z

Summary

Preserve comments attached to parameters inside Sorbet signatures when parsing RBI files and printing RBS.

This work is based on and supersedes #612 by @KaanOzkan.

Move shared comment parsing into the base parser visitor so TreeBuilder and SigBuilder consume the same comment map.
Attach comments inside a signature to the closest representable node:
- signature-level comments to Sig#comments
- parameter comments to SigParam#comments
- comments outside the signature source range to the method
Bound parameter comment lookup to its containing params(...) call.
Preserve inline and leading parameter comments without capturing comments from surrounding calls.
Print ordinary RBS parameter comments at the end of their parameter line.
Print block parameter comments at the end of the complete signature line.
Keep block signatures inline after overload separators.
Retain the legacy TreeBuilder(..., comments:) keyword used by Spoom.
Regenerate the exported RBI.

Example

Given:

sig do
  params(
    # User identifier
    id: Integer,
    # Callback
    block: T.proc.void
  ).void
end
def foo(id, &block); end

RBS output becomes:

def foo: (
  Integer id # User identifier
) { -> void } -> void # Callback

Overloads remain inline:

def foo: -> void
       | { -> void } -> void # Callback

Multiple comments on an ordinary parameter remain aligned:

Integer id, # User identifier
            # Generated externally

Co-authored-by: Kaan Ozkan <kaanozkan97@gmail.com>

KaanOzkan · 2026-08-20T15:28:31Z

+      def initialize(source, file:, comments: nil, comments_by_line: {})
+        if comments
+          comments_by_line = comments.to_h { |comment| [comment.location.start_line, comment] }
+        end


Wdyt of raising if both comments and comments_by_line are supplied so we don't ignore it silently?

If we don't go that route: Is there a downside of prioritizing comments_by_line instead of calculating it based on comments? Using what's supplied feels better.

KaanOzkan · 2026-08-20T15:30:47Z


        visit(node.receiver)
        visit(node.block)
+        @current.comments.concat(comments_inside(node)) if node.message == "sig"


Agent brought up that sig comments aren't printed so this case fails now:

def test_preserves_comments_inside_sig_in_rbs rbi = parse_rbi(<<~RBI) sig do # keep me void end def foo; end RBI assert_includes(rbi.rbs_string, "# keep me") end

amomchilov

Great feature!

amomchilov · 2026-08-20T16:30:55Z

+          # If we find the start of a RBS comment, we create a new RBSComment
+          # Note that we ignore RDoc directives such as `:nodoc:`
+          # See https://ruby.github.io/rdoc/RDoc/MarkupReference.html#class-RDoc::MarkupReference-label-Directives
+          if text.start_with?("#:") && !(text =~ /^#:[a-z_]+:/)


esoteric syntax

Suggested change

if text.start_with?("#:") && !(text =~ /^#:[a-z_]+:/)

if text.start_with?("#:") && !text.match?(/^#:[a-z_]+:/)

amomchilov · 2026-08-20T16:31:18Z

+              continuation_text = rbs_comment.location.slice.sub(/^#\| ?/, "").strip
+              continuation_loc = Loc.from_prism(@file, rbs_comment.location)
+              loc = loc.join(continuation_loc)
+              text = "#{text}#{continuation_text}"


We can avoid the reallocation here

Suggested change

text = "#{text}#{continuation_text}"

text.concat(continuation_text)

amomchilov · 2026-08-20T16:31:40Z

+            end
+
+            rbs_continuation.clear
+            comments.unshift(RBSComment.new(text, loc: loc))


How about we << them to the end instead, and only reverse! at the end?

amomchilov · 2026-08-20T16:31:54Z

+            rbs_continuation.clear
+            comments.unshift(RBSComment.new(text, loc: loc))
+          else
+            # If we have unused continuation comments, we should inject them back to not lose them


Would be helpful to pair these comments with a visual example.

amomchilov · 2026-08-20T16:32:13Z

+          @comments_by_line.delete(line)
+        end
+
+        # If we have unused continuation comments, we should inject them back to not lose them


How does this differ from the same logic on line 220?

amomchilov · 2026-08-20T16:40:29Z

+      def parse_comment(node)
+        text = node.location.slice.sub(/^# ?/, "").rstrip
+        loc = Loc.from_prism(@file, node.location)
+        Comment.new(text, loc: loc)


Perhaps this should be a factory function on Comment

Suggested change

Comment.new(text, loc: loc)

Comment.from_prism_node(node)

amomchilov · 2026-08-20T16:41:48Z

-        super(source, file: file)
+      #: (String source, file: String, ?comments: Array[Prism::Comment]?,
+      #| ?comments_by_line: Hash[Integer, Prism::Comment]) -> void
+      def initialize(source, file:, comments: nil, comments_by_line: {})


This parameter pack is starting to get unwieldy. I think we should extract a class like something like SourceFile, which stores all this stuff

https://refactoring.guru/introduce-parameter-object

amomchilov · 2026-08-20T16:47:22Z

        comments = [] #: Array[Comment]

        sigs.each do |sig|
-          comments += sig.comments.dup
-          sig.comments.clear
+          inside, outside = sig.comments.partition { |comment| loc_inside?(comment.loc, sig.loc) }
+          comments.concat(outside)
+          sig.comments.replace(inside)
        end

        comments


How do you feel about flat_map with a sideffect?

Suggested change

comments = [] #: Array[Comment]

sigs.each do |sig|

comments += sig.comments.dup

sig.comments.clear

inside, outside = sig.comments.partition { |comment| loc_inside?(comment.loc, sig.loc) }

comments.concat(outside)

sig.comments.replace(inside)

end

comments

sigs.flat_map do |sig|

inside, outside = sig.comments.partition { |comment| loc_inside?(comment.loc, sig.loc) }

sig.comments.replace(inside)

outside

end

The partition+replace combo is a bit tricky.

What do you think of vendoring a copy of extract! from ActiveSupport?

https://github.com/rails/rails/blob/afa0f9244d22abc0b88b46c38c906b518b5fc3c1/activesupport/lib/active_support/core_ext/array/extract.rb#L5-L22

Then this could just be:

Suggested change

comments = [] #: Array[Comment]

sigs.each do |sig|

comments += sig.comments.dup

sig.comments.clear

inside, outside = sig.comments.partition { |comment| loc_inside?(comment.loc, sig.loc) }

comments.concat(outside)

sig.comments.replace(inside)

end

comments

sigs.flat_map do |sig|

sig.comments.extract! { |comment| !loc_inside?(comment.loc, sig.loc) }

end

amomchilov · 2026-08-20T16:50:58Z

        builder.current.loc = node_loc(node)
        builder.visit_call_node(node)
-        builder.current.comments = node_comments(node)
+        builder.current.comments = node_comments(node) + builder.current.comments


One less alloc + copy

Suggested change

builder.current.comments = node_comments(node) + builder.current.comments

builder.current.comments = node_comments(node).concat(builder.current.comments)

amomchilov · 2026-08-20T16:51:11Z


-      #: (String content, file: String) -> void
-      def initialize(content, file:)
+      # Bounds sig param comment lookup to comments inside the current `params(...)` call.


Morriar and others added 4 commits August 19, 2026 14:03

Move comment parsing to the base visitor

94db214

Co-authored-by: Kaan Ozkan <kaanozkan97@gmail.com>

Parse comments found inside signatures

807d347

Co-authored-by: Kaan Ozkan <kaanozkan97@gmail.com>

Print comments inside RBS signatures

2e29a81

Co-authored-by: Kaan Ozkan <kaanozkan97@gmail.com>

Update exported RBI

017bd2d

Co-authored-by: Kaan Ozkan <kaanozkan97@gmail.com>

Morriar requested a review from a team as a code owner August 19, 2026 18:20

KaanOzkan reviewed Aug 20, 2026

View reviewed changes

amomchilov reviewed Aug 20, 2026

View reviewed changes

	if text.start_with?("#:") && !(text =~ /^#:[a-z_]+:/)
	if text.start_with?("#:") && !text.match?(/^#:[a-z_]+:/)

	text = "#{text}#{continuation_text}"
	text.concat(continuation_text)

	builder.current.comments = node_comments(node) + builder.current.comments
	builder.current.comments = node_comments(node).concat(builder.current.comments)

Uh oh!

Conversation

Morriar commented Aug 19, 2026

Summary

Example

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amomchilov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants