michaelbull / kotlin-result

A multiplatform Result monad for modelling success or failure operations.
ISC License
1.02k stars 57 forks source link

Result 2.0: unsafe unwrapping of `Result.value` and `Result.error` #104

Open kirillzh opened 3 months ago

kirillzh commented 3 months ago

I noticed that the new inline value class in API 2.0 lets us access result.value directly without checking if it’s actually safe, which wasn’t the case before. Here’s what I mean:

val result: Result<Int, SomeError> = getResult()
result.value // This compiles even if the result is an error, leading to potential runtime errors.

This could lead to runtime exceptions that were harder to make before when the API required type checking and more explicit unwrapping using getOrThrow() (which is a lot easier to catch or denylist through code analysis).

It seems like API 2.0 introduces a downgrade in compile-time safety compared to the previous version. Could we consider reinstating some form of this safety, perhaps through Kotlin contracts or another mechanism, to prevent potential runtime errors?

michaelbull commented 3 months ago

Could we consider reinstating some form of this safety, perhaps through Kotlin contracts or another mechanism, to prevent potential runtime errors?

Do you have a proposal of which compiler contract would achieve this? We effectively need user-defined guards, such that you can't call .value unless you've done an isOk/isErr check - I don't think such a thing exists in Kotlin?

raamcosta commented 2 months ago

If value is always available, then maybe it should be nullable. This way we can check if it is null, and get smart cast.

It would also mean that people are more likely to use better approaches such as doing .onSuccess { value -> } given that inside the lambda, "value" wouldn't be null.

But I may be missing something?