AND-221: fix ResultFlow polymorphism#64
Conversation
| * @see flow | ||
| */ | ||
| @JvmName("resultFlowResult") | ||
| public fun <T> resultFlow(block: suspend () -> Result<T>): Flow<Result<T>> { |
There was a problem hiding this comment.
Instead of removing this signature we could add @Deprecated(DeprecationLevel.HIDDEN) this way already compiled code will still work with new version of the library
There was a problem hiding this comment.
I'd argue against HIDDEN here:
- The original signature is the root cause of the polymorphism bug — preserving binary compat for it locks in broken behaviour for downstream code.
- HIDDEN doesn't fix the source-level issue: resultFlow { repo.fetchData() } (returning Result) would fall through to the (suspend () -> T) overload and reproduce the double-wrap. Only DeprecationLevel.ERROR stops it at the call site.
- We can't have both — two overloads with identical Kotlin signatures aren't allowed, so it's HIDDEN or the tripwire, not both.
| "or use `flow { emit(block()) }` to emit the Result as-is.", | ||
| level = DeprecationLevel.ERROR, | ||
| ) | ||
| @JvmName("-resultFlowResultDoubleWrap") |
There was a problem hiding this comment.
Alternatively @JvmSynthetic could be applied to make the function invisible from Java.
There was a problem hiding this comment.
Apply @JvmSynthetic. But @JvmSynthetic only sets the ACC_SYNTHETIC flag — it doesn't rename the method. I keeps both annotations, @JvmName disambiguates the JVM signature
| * @see flow | ||
| * @see toResultFlow | ||
| */ | ||
| public fun <T> resultFlow(block: suspend () -> T): Flow<Result<T>> { |
There was a problem hiding this comment.
Btw, can't inline + crossinline be used here?
There was a problem hiding this comment.
Yes, it can be applied. Fix that
sWift-sai
requested review from
AleksandrTabolin
and removed request for
sWift-sai
TopHlop
force-pushed
the
fix/AND-221-result-flow
branch
from
44b4cf9 to
2496bf6
Compare
2 participants
No description provided.