Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make the add_sig block nilable #371

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

matthewarkin
Copy link

Sig.new defines the block to be nilable and there are plenty of cases where you can add a sig without needing/wanting to use the block syntax.

Sig.new defines the block to be nilable and there are plenty of cases where you can add a sig without needing/wanting to use the block syntax.
@matthewarkin matthewarkin requested a review from a team as a code owner October 8, 2024 17:02
@paracycle
Copy link
Member

What's the use-case where you would not need to supply the block?

@matthewarkin
Copy link
Author

I'm using tapioca to generate a method with multiple signatures, I have one signature that takes a block and is a void otherwise it returns an enumerator

https://github.com/Shopify/tapioca/blob/3948457fe17ada17ee21a55987dd9d0499860e24/lib/tapioca/rbi_ext/model.rb#L90

Since tapioca doesn't return the method here, the only way I can access it to add the sigs is to use the block on RBI::Method.new (though arguably, tapioca should allow me to pass sigs to create_method or return the method so I can then do method.sigs <<

within the block, I would like to just do:

parameters = [
  RBI::SigParam.new("start", "T.untyped"),
  RBI::SigParam.new("finish", "T.untyped"),
]
create_method("method_name) do |method|
  method.add_sig(
    params: parameters + [RBI::SigParam.new("block", "T.proc.params(object: T::Array[T.untyped]).void")],
    return_type: "void",
  )
  method.add_sig(
    params: parameters,
    return_type: "T::Enumerator[T::Enumerator[T.untyped]]",
  )
end

but since the block is required I can't do that, I gotta do:

method.add_sig(
  params: parameters + [RBI::SigParam.new("block", "T.proc.params(object: T::Array[T.untyped]).void")],
  return_type: "void",
) do |sig|
end

Though of course, I've now realized that I can do:

parameters = [
  RBI::SigParam.new("start", "T.untyped"),
  RBI::SigParam.new("finish", "T.untyped"),
]
create_method("method_name) do |method|
  method.sigs << RBI::Sig.new(
    params: parameters + [RBI::SigParam.new("block", "T.proc.params(object: T::Array[T.untyped]).void")],
    return_type: "void",
  )
  method.sigs << RBI::Sig.new(
    params: parameters,
    return_type: "T::Enumerator[T::Enumerator[T.untyped]]",
  )
end

which gets me what I want, but I don't see why the block should be required on the add_sig call

@@ -581,7 +581,7 @@ def add_block_param(name)
is_final: T::Boolean,
type_params: T::Array[String],
checked: T.nilable(Symbol),
block: T.proc.params(node: Sig).void,
block: T.nilable(T.proc.params(node: Sig).void),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind adding a test for this under https://github.com/Shopify/rbi/blob/main/test/rbi/model_test.rb#L209 please? 🙏

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the delay here, test added.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants