Skip to content

Share reflected method parameter compilation #2689

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
Morriar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Share reflected method parameter compilation #2689

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
26 changes: 2 additions & 24 deletions lib/tapioca/dsl/compiler.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,30 +161,8 @@ def compile_method_parameters_to_rbi(method_def)
parameters = method_def.parameters #: Array[[Symbol, Symbol?]]

parameters.each_with_index.map do |(type, name), index|
fallback_arg_name = "_arg#{index}"

name = name ? name.to_s : fallback_arg_name
name = fallback_arg_name unless valid_parameter_name?(name)
method_type = T.must(method_types[index])

case type
when :req
create_param(name, type: method_type)
when :opt
create_opt_param(name, type: method_type, default: "T.unsafe(nil)")
when :rest
create_rest_param(name, type: method_type)
when :keyreq
create_kw_param(name, type: method_type)
when :key
create_kw_opt_param(name, type: method_type, default: "T.unsafe(nil)")
when :keyrest
create_kw_rest_param(name, type: method_type)
when :block
create_block_param(name, type: method_type)
else
raise "Unknown type `#{type}`."
end
parameter, = create_method_parameter(type, name&.to_s, index)
create_typed_param(parameter, T.must(method_types[index]))
end
end

Expand Down
40 changes: 9 additions & 31 deletions lib/tapioca/gem/listeners/methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,8 @@ def compile_method(tree, symbol_name, constant, method, visibility = RBI::Public

parameters = method.parameters #: Array[[Symbol, Symbol?]]

sanitized_parameters = parameters.each_with_index.map do |(type, name), index|
fallback_arg_name = "_arg#{index}"

name = if name
compiled_parameters = parameters.each_with_index.map do |(type, name), index|
parameter_name = if name
name.to_s
else
# For attr_writer methods, Sorbet signatures have the name
Expand All @@ -119,17 +117,11 @@ def compile_method(tree, symbol_name, constant, method, visibility = RBI::Public
signature.arg_types.size == 1 &&
method_name[-1] == "="

if writer_method_with_sig
method_name.delete_suffix("=")
else
fallback_arg_name
end
method_name.delete_suffix("=") if writer_method_with_sig
end

# Sanitize param names
name = fallback_arg_name unless valid_parameter_name?(name)

[type, name]
parameter, signature_name = create_method_parameter(type, parameter_name, index)
[type, parameter, signature_name]
end

rbi_method = RBI::Method.new(
Expand All @@ -138,26 +130,12 @@ def compile_method(tree, symbol_name, constant, method, visibility = RBI::Public
visibility: visibility,
)

sanitized_parameters.each do |type, name|
case type
when :req
rbi_method << RBI::ReqParam.new(name)
when :opt
rbi_method << RBI::OptParam.new(name, "T.unsafe(nil)")
when :rest
rbi_method << RBI::RestParam.new(name)
when :keyreq
rbi_method << RBI::KwParam.new(name)
when :key
rbi_method << RBI::KwOptParam.new(name, "T.unsafe(nil)")
when :keyrest
rbi_method << RBI::KwRestParam.new(name)
when :block
rbi_method << RBI::BlockParam.new(name)
end
compiled_parameters.each do |_, parameter, _|
rbi_method << parameter
end

@pipeline.push_method(symbol_name, constant, method, rbi_method, signature, sanitized_parameters)
parameters_for_signature = compiled_parameters.map { |type, _, name| [type, name] }
@pipeline.push_method(symbol_name, constant, method, rbi_method, signature, parameters_for_signature)
tree << rbi_method
end

Expand Down
28 changes: 28 additions & 0 deletions lib/tapioca/helpers/rbi_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,34 @@ def extract_type_parameters(type_strings)
type_strings.join(", ").scan(TYPE_PARAMETER_MATCHER).flatten.uniq
end

#: (Symbol type, String? name, Integer index) -> [RBI::Param, String]
def create_method_parameter(type, name, index)
fallback_name = "_arg#{index}"
name ||= fallback_name
name = fallback_name unless valid_parameter_name?(name)
Comment on lines +106 to +108

@paracycle paracycle Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems we are allocating a fallback_name unnecessarily for each parameter here. Can we also refactor valid_parameter_name? to accept a String? and reject nil values, so that we can do:

Suggested change
fallback_name = "_arg#{index}"
name ||= fallback_name
name = fallback_name unless valid_parameter_name?(name)
name = "_arg#{index}" unless valid_parameter_name?(name)

This way the new string allocation will only happen for invalid or missing parameter names


parameter = case type
when :req
RBI::ReqParam.new(name)
when :opt
RBI::OptParam.new(name, "T.unsafe(nil)")
when :rest
RBI::RestParam.new(name)
when :keyreq
RBI::KwParam.new(name)
when :key
RBI::KwOptParam.new(name, "T.unsafe(nil)")
when :keyrest
RBI::KwRestParam.new(name)
when :block
RBI::BlockParam.new(name)
else
Kernel.raise "Unknown type `#{type}`."

@KaanOzkan KaanOzkan Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TIL, agent found that :nokey is a valid parameter kind for: def foo(*args, **nil). Apparently it's used to reject kwargs. It's not supported in rbi but Sorbet doesn't support it either so not raising might be enough.

@paracycle paracycle Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There will also be a :noblock version of it in Ruby 4.1: ruby/ruby@9967418

def foo(*args, &nil) = 42

method(:foo).parameters # => [[:rest, :args], [:noblock]]

end

[parameter, name]
end

#: (String name) -> bool
def valid_method_name?(name)
Prism.parse_success?("def self.#{name}(a); end")
Expand Down
Loading