[Kernel] Tweak signatures for conversion methods, and add unit tests in #2683
Conversation
sampersand
force-pushed
the
sampersand/2025-10-17/update-conversion-methods
branch
from
666a86a to
1106b9c
Compare
| | (Numeric numeric, ?exception: bool) -> Complex | ||
| | (String real_or_both, ?exception: true) -> Complex | ||
| | (untyped real_or_both, exception: bool) -> Complex? | ||
| | (Numeric | String real, Numeric | String imag, ?exception: true) -> Complex | ||
| | (Numeric | String real, Integer | Float | Rational | Complex imag, exception: bool) -> Complex | ||
| | (Numeric | String real, untyped imag, exception: bool) -> Complex? |
There was a problem hiding this comment.
This one's been changed because, after looking into it, Complex is quite messy:
Complex(Numeric)can never returnnil, regardless ofexceptionComplex(Numeric | String, denom, exception: false)will actually returnnilwhen the denom is a non-Integer | Float | Complex | RationalNumeric. Wonky, becauseexception: true/omitted doesn't do that, and it's only for the second argument
| | (string str, int base, ?exception: true) -> Integer | ||
| | (string str, int base, exception: bool) -> Integer? | ||
| | (untyped, ?untyped, ?exception: bool) -> Integer? | ||
| | (untyped, ?int base, exception: bool) -> Integer? |
There was a problem hiding this comment.
If provided, the second argument actually has to be an int, otherwise an error will be thrown (regardless of exception)
| | (int | _ToR numer, int | _ToR denom, ?exception: true) -> Rational | ||
| | (int | _ToR numer, int | _ToR denom, exception: bool) -> Rational? |
There was a problem hiding this comment.
I simplified the signature of this a bit, collapsing the int | _ToR numer, ?... into the ones above it
| type.literal == val | ||
| rescue NoMethodError | ||
| raise if defined?(val.==) | ||
| false |
There was a problem hiding this comment.
This is needed because one of the Rational's signatures has a 1 in it, but is compared against a ToInt, which doesn't define ==.
| errors = typecheck.method_call(method, method_type, trace, errors: []) | ||
|
|
||
| assert_empty errors.map {|x| RBS::Test::Errors.to_string(x) }, "Call trace does not match with given method type: #{trace.inspect}" | ||
| assert_empty errors.map {|x| RBS::Test::Errors.to_string(x) }, proc { "Call trace does not match with given method type: #{trace.inspect}" } |
There was a problem hiding this comment.
Instead of always creating the string for the traces, passing a proc in delays it until later. This is necessary because some of the Complex methods would be valid (eg Complex(1, Class.new(Numeric).new)), but the trace.inspect would end up calling #inspect on the resulting Complex, which raised an exception.
sampersand
force-pushed
the
sampersand/2025-10-17/update-conversion-methods
branch
from
1106b9c to
0772d0f
Compare
sampersand
force-pushed
the
sampersand/2025-10-17/update-conversion-methods
branch
from
0772d0f to
c6706bd
Compare
|
@sampersand |
2 participants
This PR tweaks some of the
Kernelconversion methods' signatures, as well as cleaning up/adding relevant tests.Of particular note is that the numeric conversions (
Integer/Float/Complex/Rational)'s final signature (the(untyped, ?exception: bool) -> type?one) now marks theexception: boolas required.