Fix RBS signatures in send nodes like prepend

[allcre]

Fix RBS signature recognition in send node contexts

Ticket: https://github.com/Shopify/team-ruby-dx/issues/1451

Solution

SigsRewriter changes:

• Added logic to rewrite arguments in non-visibility send nodes
• This enables the rewriter to traverse into send node arguments and process any method definitions found there
• Specifically skip visibility modifier sends (like private def method_name) to prevent double-processing, since those method definitions are already handled directly by the main AST traversal

Example

# typed: strict

class A; end

A.prepend(Module.new do
  #: -> void
  def foo; end  # Now properly recognizes the RBS signature
end)

[allcre]

• Fix RBS signatures in send nodes like prepend #432 👈 (View in Graphite)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Morriar]

Why do we need this condition, can't we just visit them?

[allcre]

From my understanding, a method like this:

#: (Integer) -> void
private def method1(x)
  T.reveal_type(x)
end

has a DefMethod node and a Send node from the private keyword.

First, signaturesTarget() finds the DefMethod → creates signature.
Then, rewriteNodes processes the Send arguments → finds same DefMethod → creates duplicate signature.

This results in "Unused type annotation" errors if we do not include this check.

[Morriar]

I'm not sure I follow.

The comment should have been associated to the def:

sorbet/rbs/CommentsAssociator.cc

Line 456 in ad5cc43

associateSignatureCommentsToNode(send->args[0].get());

So this means we're visiting the same node twice?

[Morriar]

Ok, it seems we're visiting the send arg once through the body of the class, then a second type by visiting the send itself.

I wonder if we should instead attach the comment to the send in commentsAssociator so we don't have to check isVisibilitySend 3 times?

[allcre]

Thanks for the suggestion! I looked into this with @KaanOzkan, and if we associate comments with the Send node instead of the method def, the rewriter would need to extract the comment from the Send and apply it to the DefMethod in args[0]. This cross-node processing would add some complexity compared to the current approach. I can explore this if you think we should go with this route, or I can open the PR in Sorbet and see what they say.

[allcre]

I've prototyped it here: #436, let me know what you think!