create_sig takes hash instead of TypedParams #1856
Conversation
lib/tapioca/rbi_ext/model.rb
Outdated
| @@ -126,15 +129,12 @@ def create_method_with_sigs(name, sigs:, parameters: [], class_method: false, vi | |||
|
|
|||
| sig do | |||
| params( | |||
| parameters: T::Array[RBI::TypedParam], | |||
| parameters: T::Array[RBI::SigParam], | |||
| return_type: String, | |||
| ).returns(RBI::Sig) | |||
| end | |||
| def create_sig(parameters: [], return_type: "T.untyped") | |||
There was a problem hiding this comment.
Since this method now doesn't do anything of value, any objections to removing it?
| RBI::SigParam.new("start", "T.untyped"), | ||
| RBI::SigParam.new("finish", "T.untyped"), | ||
| RBI::SigParam.new("batch_size", "Integer"), | ||
| RBI::SigParam.new("error_on_ignore", "T.untyped"), | ||
| *(RBI::SigParam.new("order", "Symbol") if order), | ||
| RBI::SigParam.new("block", "T.proc.params(object: #{constant_name}).void"), |
There was a problem hiding this comment.
I suspect:
| RBI::SigParam.new("start", "T.untyped"), | |
| RBI::SigParam.new("finish", "T.untyped"), | |
| RBI::SigParam.new("batch_size", "Integer"), | |
| RBI::SigParam.new("error_on_ignore", "T.untyped"), | |
| *(RBI::SigParam.new("order", "Symbol") if order), | |
| RBI::SigParam.new("block", "T.proc.params(object: #{constant_name}).void"), | |
| start: "T.untyped", | |
| finish: "T.untyped", | |
| batch_size: "Integer", | |
| error_on_ignore: "T.untyped", | |
| **({order: "Symbol"} if order), | |
| block: "T.proc.params(object: #{constant_name}).void", |
is a nicer and easier to read API, but it doesn't quite work due to **nil not being valid (until Ruby 3.4, at least).
There was a problem hiding this comment.
Oh, this works:
| RBI::SigParam.new("start", "T.untyped"), | |
| RBI::SigParam.new("finish", "T.untyped"), | |
| RBI::SigParam.new("batch_size", "Integer"), | |
| RBI::SigParam.new("error_on_ignore", "T.untyped"), | |
| *(RBI::SigParam.new("order", "Symbol") if order), | |
| RBI::SigParam.new("block", "T.proc.params(object: #{constant_name}).void"), | |
| start: "T.untyped", | |
| finish: "T.untyped", | |
| batch_size: "Integer", | |
| error_on_ignore: "T.untyped", | |
| **({order: "Symbol"} if order).to_h, | |
| block: "T.proc.params(object: #{constant_name}).void", |
even though the conditional part is a little ugly.
There was a problem hiding this comment.
I like the hash API and implemented that! Thanks for the suggestion.
The **().to_h trick is neat but the syntax is 😅. I ended up going with a simpler compact instead, but can change it if you strongly feel about using this style.
TypeParam accepts a default, which is confusing since it's not used in sigs. SigParam is the right thing to use here, and create_sig will build these from a hash.
bdewater
force-pushed
the
overloaded-sig-clarification
branch
from
0bd71af
to
cdd80d1
Compare
|
@bdewater thanks for the changes, I like what you've done with the hash. In the meanwhile, @at and I paired to reimagine how adding params and signatures could be more fluent and came up with Shopify/rbi#284 |
bdewater
changed the title
Motivation
Follow-up to #1799. Based on this comment here: #1830 (comment) -
TypeParamaccepts a default, which is confusing since it's not used in sigs.Implementation
Breaking change for custom compilers relying on the previous implementation, but it's a) not too hard to change with a regex-based find/replace and b) recent addition so probably not a lot usage yet.
Tests
Existing tests cover this.