feat: implement `Result#either` for easier result unpacking

[maxveldink]

This is largely inspired by Dry Monad's either implementation on their Result type.

This method accepts two procs and runs the first if the result is a success and the second if it's a failure. nil payloads and errors are not passed to the given procs.

One downside is the runtime type erasure of the generic parameters that each proc returns. I'm going to add an issue to investigate if we can prevent the runtime return type to be seen as T.untyped. Maybe @iMacTia has some ideas for how to improve that 😄

[iMacTia]

One downside is the runtime type erasure of the generic parameters that each proc returns.

I was gonna raise exactly that, I remember when I tried implementing either some time ago I had exactly this issue. The implementation looks fine, but I can't see what the static/runtime types are without a test_data test. Do you think you could add one there so if we can figure a way to make this work nicely later we can improve those?

[maxveldink]

Yep! I can add one back. I had a working one, but it seemed to be testing exactly what they regular minitest test case was, so I removed it. I'll push one in a few and see if we can figure something out

[maxveldink]

@iMacTia I just pushed up a first attempt at a typecheck test. The latter two test methods are where I'm having issues/I don't think we get full type checking from Sorbet.

[iMacTia]

Yeah so I spent some time looking into this, and the issue is that when you define a proc using proc { ... } or -> { ... }, sorbet treats them as params(T.untyped).returns(T.untyped). And because of the Parametrised definition of either, that means the result of either is also T.untyped. See example in the sorbet playground.

Now, there's nothing we can do apparently "inside" the procs, that's a limitation of sorbet, but based on how you pass them to either, you can at least get a proper return type. See this example:

sig { params(should_succeed: T::Boolean).returns(Typed::Result[Integer, String]) }
def do_something(should_succeed)
  if should_succeed
    Typed::Success.new(123)
  else
    Typed::Failure.new("error")
  end
end

sig { returns(T.proc.params(arg0: Integer).returns(String)) }
def on_success
  ->(payload) { "Payload was #{payload}" }
end

sig { returns(T.proc.params(arg0: String).returns(String)) }
def on_failure
  ->(error) { raise error }
end

sig { returns(String) }
def test_return_type_check
  res = do_something(true).either(
    on_success,
    on_failure
  )

  T.assert_type!(res, String)
end

At least this way, sorbet knows that res has type String. The trick is using HOFs (on_success, on_failure) to provide a type for the procs.

My suggestion is to clearly spell this out in the README, together with code examples. Personally, I'd discourage using either and instead favour and_then and on_error as these provide the maximum type-checking, but I guess it will be up to the user what they want to use, as far as they know and understand the trade-offs

[iMacTia]

Another way to provide a signature for the procs without using HOFs, is to use T.let:

sig { returns(String) }
def test_return_type_check
  res = do_something(true).either(
    T.let(
      ->(payload) { "Payload was #{payload}" },
      T.proc.params(arg0: Integer).returns(String)
    ),
    T.let(
      ->(error) { raise error },
      T.proc.params(arg0: String).returns(String)
    ),
  )

  T.assert_type!(res, String)
end

[maxveldink]

Yeah so I spent some time looking into this, and the issue is that when you define a proc using proc { ... } or -> { ... }, sorbet treats them as params(T.untyped).returns(T.untyped).

Aha, yea I didn't realize that's what Sorbet was doing but now that makes more sense.

Yea, I agree and_then and on_error is probably the path we should encourage. I'm hesitant to merge this in with a large caveat of needing T.let or HoF to communicate the typing to Sorbet; that doesn't seem ergonomic enough to be worth it.

I'm going to leave this open to see if anyone else has any thoughts, but will probably opt to close it in a few days.

[maxveldink]

I'm closing this PR for now for the aforementioned reasons. I don't think we should support this method right now with the current runtime generic erasures.