From e76b5069fd9b59566158d31a8dba978dadadd869 Mon Sep 17 00:00:00 2001
From: Jacek Sieka <jacek@status.im>
Date: Thu, 11 May 2023 17:13:32 +0200
Subject: [PATCH] result: add result assignment extenstion point for `?`

When `?` exits early, it does so using variations `return v` - however,
in frameworks such as `chronos`, we need to change `return v` to
`future.complete(v)` - this feature allows chronos to inject a template
that performs this assignment instead of using the "ordinary" flow.

Risk:

`assignResult` might already be taken as a name, and multiple frameworks
might compete for the functionality.

Potential alternative:

Instead of `assignResult`, this construct could be called
`returnResult`, and it would be responsible for breaking the control
flow (performing the `return` in addition to assigning the result).

Other interactions:

https://github.com/status-im/nim-stew/pull/34 proposes a general-purpose
conversion framework - this could be used to automatically convert the
error based on existing conversions - the use case is that oftentimes,
one error needs to be converted to another in a call chain and injecting
a converter simplifies this flow - this might however create an
problematic interaction with an `assignResult` template in the future.
---
 stew/results.nim       | 44 +++++++++++++++++++++++++++++++++---------
 tests/test_results.nim | 41 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 9 deletions(-)

diff --git a/stew/results.nim b/stew/results.nim
index f60af4c8..30622d0f 100644
--- a/stew/results.nim
+++ b/stew/results.nim
@@ -1051,18 +1051,44 @@ template `?`*[T, E](self: Result[T, E]): auto =
   ## let v = ? funcWithResult()
   ## echo v # prints value, not Result!
   ## ```
+  ##
+  ## ``assignResult?`` extension point
+  ##
+  ## If a template / function named ``assignResult?`` exists in the scope, it will
+  ## be called instead of assigning the result - this can be used to intercept
+  ## the assignment to `result` that implicitly happens on early return in case
+  ## of error.
+  ##
+  ## To prevent conflicts, it is recommended that ``assignResult?`` is declared
+  ## close to the scope of `?` (as opposed to a globally visible symbol)
+  ##
+  ## ```nim
+  ## proc f(): Result[...] =
+  ##   template assignResult(v: Result) = ...
+  ##   let a = ? f2()
+  ## ```
+  ##
   ## Experimental
-  # TODO the v copy is here to prevent multiple evaluations of self - could
-  #      probably avoid it with some fancy macro magic..
-  let v = (self)
+  mixin `assignResult?`
+
+  let v = (self) # TODO avoid copy
   if not v.oResultPrivate:
-    when typeof(result) is typeof(v):
-      return v
-    else:
-      when E is void:
-        return err(typeof(result))
+    when declared(`assignResult?`):
+      when typeof(result) is typeof(v):
+        `assignResult?`(v)
+      elif E is void:
+        `assignResult?`(err(typeof(result)))
       else:
-        return err(typeof(result), v.eResultPrivate)
+        `assignResult?`(err(typeof(result), v.eResultPrivate))
+      return
+    else:
+      return
+        when typeof(result) is typeof(v):
+          v
+        elif E is void:
+          err(typeof(result))
+        else:
+          err(typeof(result), v.eResultPrivate)
 
   when not(T is void):
     v.vResultPrivate
diff --git a/tests/test_results.nim b/tests/test_results.nim
index ef767c8b..b08e9b05 100644
--- a/tests/test_results.nim
+++ b/tests/test_results.nim
@@ -440,3 +440,44 @@ block: # Constants
   proc checkIt(v: WithOpt) =
     doAssert v.opt.isNone()
   checkIt(noneWithOpt)
+
+proc testAssignResult() =
+  var assigned: bool
+  template `assignResult?`(v: Result[int, string]) =
+    assigned = true
+    result = v
+
+  proc failed(): Result[int, string] =
+    err("fail")
+
+  proc calling(): Result[int, string] =
+    let _ = ? failed()
+    doAssert false
+
+  let r = calling()
+  doAssert assigned
+  doAssert r == Result[int, string].err("fail")
+
+
+proc testAssignResultGeneric[T]() =
+  var assigned: bool
+  template `assignResult?`(v: Result[T, string]) =
+    mixin result
+    assigned = true
+    result = v
+
+  proc failed(): Result[int, string] =
+    err("fail")
+
+  proc calling(): Result[int, string] =
+    let _ = ? failed()
+    doAssert false
+
+  let r = calling()
+  doAssert assigned
+  doAssert r == Result[int, string].err("fail")
+
+
+testAssignResult()
+
+testAssignResultGeneric[int]()