Fix invalid sig generated for anonymous block param (&)#935
Fix invalid sig generated for anonymous block param (&)#935prattik-wav wants to merge 5 commits into
Conversation
When a method uses Ruby 3.1+ anonymous block forwarding (def foo(&); end), rbs_comments_to_sorbet_sigs was generating `&block:` inside params(), which is a syntax error. Named block params (`&block`) were already handled correctly. Detect a nil-named BlockParameterNode in the def and strip the leading `&` from any block param name in the translated sig. Fixes Shopify#928
| sig { params(request: String, block: ::T.nilable(::T.proc.params(arg0: String).void)).returns(String) } | ||
| def bar(request, &); end |
There was a problem hiding this comment.
I don't think this generated code is correct either: https://sorbet.run/#%23%20typed%3A%20true%0Aclass%20Foo%0A%20%20extend%20T%3A%3ASig%0A%0A%20%20sig%20%7B%20params%28request%3A%20String%2C%20block%3A%20%3A%3AT.nilable%28%3A%3AT.proc.params%28arg0%3A%20String%29.void%29%29.void%20%7D%0A%20%20def%20bar%28request%2C%20%26%29%3B%20nil%3B%20end%0Aend
I think it should be:
| sig { params(request: String, block: ::T.nilable(::T.proc.params(arg0: String).void)).returns(String) } | |
| def bar(request, &); end | |
| sig { params(request: String, "&": ::T.nilable(::T.proc.params(arg0: String).void)).returns(String) } | |
| def bar(request, &); end |
| if anonymous_block_param?(def_node) | ||
| sig.params.each do |param| | ||
| param.name = param.name.delete_prefix("&") if param.name.start_with?("&") | ||
| end | ||
| end |
There was a problem hiding this comment.
I am not sure if this is the correct place to fix this problem. We need to solve it at the source.
There was a problem hiding this comment.
Yeah the correct place to fix this is in the RBI gem that Spoom uses for this translation:
|
|
||
| # Must also be valid Ruby | ||
| assert RubyVM::InstructionSequence.compile(res) |
There was a problem hiding this comment.
This is overkill, we don't need to waste time compiling it. Perhaps assert Prism.parse_success?(res) at most, but we match the exact expected source we want, so there's no need to confirm this here
| # Must also be valid Ruby | |
| assert RubyVM::InstructionSequence.compile(res) |
| if anonymous_block_param?(def_node) | ||
| sig.params.each do |param| | ||
| param.name = param.name.delete_prefix("&") if param.name.start_with?("&") | ||
| end | ||
| end |
There was a problem hiding this comment.
Yeah the correct place to fix this is in the RBI gem that Spoom uses for this translation:
Fixes #928
Problem
rbs_comments_to_sorbet_sigsgenerates invalid Ruby when a method uses Ruby 3.1+ anonymous block forwarding (def foo(&); end). The generated sig contains&block:insideparams(), which is a syntax error:Named block params (
&block) were already translated correctly.Root Cause
Prism::BlockParameterNode#nameisnilfor anonymous&. TheRBI::RBS::MethodTypeTranslatordoesn't distinguish this case and falls back to the default name&block, which renders as&block:insideparams()— a syntax error because&is not valid in a keyword argument position.Fix
After
translator.resultproduces the sig, check whether the originaldef_nodehas an anonymous block parameter (aBlockParameterNodewithnilname). If so, strip the leading&from any block param name in the generated sig, producing the validblock:form.Testing
test_translate_to_rbi_anonymous_block_param— covers the broken case from the issuetest_translate_to_rbi_named_block_param_unchanged— regression guard confirming named&blockstill worksRubyVM::InstructionSequence.compileFixes #928