Preserve anonymous parameters in generated RBIs - #2687
Conversation
| anonymous_parameter = anonymous_parameter_name?(type, name) | ||
| name = fallback_arg_name unless anonymous_parameter || valid_parameter_name?(name) | ||
|
|
||
| [type, name] | ||
| [type, name, anonymous_parameter] |
There was a problem hiding this comment.
| anonymous_parameter = anonymous_parameter_name?(type, name) | |
| name = fallback_arg_name unless anonymous_parameter || valid_parameter_name?(name) | |
| [type, name] | |
| [type, name, anonymous_parameter] | |
| is_anonymous_parameter = anonymous_parameter_name?(type, name) | |
| name = fallback_arg_name unless is_anonymous_parameter || valid_parameter_name?(name) | |
| [type, name, is_anonymous_parameter] |
| ) | ||
|
|
||
| sanitized_parameters.each do |type, name| | ||
| sanitized_parameters.each do |type, name, anonymous_parameter| |
There was a problem hiding this comment.
| sanitized_parameters.each do |type, name, anonymous_parameter| | |
| sanitized_parameters.each do |type, name, is_anonymous_parameter| |
There was a problem hiding this comment.
Do we need to thread the 3rd parameter through like this?
Could we instead modify the definition of name above, where anonymous_parameter is first set?
There was a problem hiding this comment.
We need to pass something because the method param needs to be generated with a nil name but the signature param lookup needs the "*" name.
I change the tuple to use both names instead of passing the anonymous flag 👍
3a3dbeb to
095ee15
Compare
| else | ||
| false | ||
| end | ||
| end |
There was a problem hiding this comment.
I think we have to fix this for DSL RBIs too in
tapioca/lib/tapioca/dsl/compiler.rb
Line 156 in 5e9c26a
There was a problem hiding this comment.
Good point. Fixed in 1a1f4cc.
I moved the anonymous parameter check into RBIHelper and reused it from the DSL compiler so create_method_from_def preserves anonymous *, **, and & parameters too.
f928e6b to
1a1f4cc
Compare
KaanOzkan
left a comment
There was a problem hiding this comment.
LGTM, just requires an expectation update
|
Confirming this in the wild, in case a real-world data point helps with prioritisation. Versions: tapioca 0.19.2, rbi 0.4.3, sorbet-static-and-runtime 0.6.13427, Ruby 3.4, Rails 8. A gem we consume defines callback macros in a Rails concern using anonymous forwarding: module ClassMethods
def on_like_received(method_name = nil, **, &)
register_social_callback(:on_like_received, method_name, **, &)
end
def on_delete_requested(*)
set_callback(:on_delete_requested, *)
end
private
# named params — generates a correct sig
def register_social_callback(callback_name, method_name = nil, **options, &block)
...
end
end
sig { params(method_name: T.nilable(::Symbol), _arg1: , _arg2: ).returns(T.untyped) }and 13 errors in that app, all of them inside generated gem RBIs — no user code involved. Two details that match your diagnosis and might be useful as test cases:
Workaround for anyone hitting this before the fix lands: post-process the generated RBI, replacing the empty type We briefly considered asking the gem to rename its parameters, but |
Keep anonymous splat, keyword splat, and block parameter names for signature lookup while rendering them as anonymous RBI parameters.
8b278d6 to
dd26093
Compare
Ruby reflects anonymous splat, keyword splat, and block parameters using pseudo-names:
Tapioca was treating those names as invalid parameter names and replacing them with fallback names like
_arg0,_arg1, and_arg2. That caused two issues:"*","**", and"&".For example, this could generate invalid RBI like:
This change preserves anonymous parameter names for signature lookup, while rendering them as anonymous RBI parameters:
It also bumps the minimum
rbidependency to0.4.1, since older versions don’t support anonymousRestParam,KwRestParam, andBlockParamnodes represented withnilnames.