Fix Using/TryFinally asyncValidation CE

src/FsToolkit.ErrorHandling/AsyncValidationCE.fs

@@ -23,82 +23,53 @@ module AsyncValidationCE =
         member inline this.Zero() : AsyncValidation<unit, 'error> = this.Return()
 
         member inline _.Delay
-            ([<InlineIfLambda>] generator: unit -> AsyncValidation<'ok, 'error>)
-            : unit -> AsyncValidation<'ok, 'error> =
-            generator
-
-        member inline _.Run
             ([<InlineIfLambda>] generator: unit -> AsyncValidation<'ok, 'error>)
             : AsyncValidation<'ok, 'error> =
-            generator ()
+            async.Delay generator
 
         member inline this.Combine
             (
-                result: AsyncValidation<unit, 'error>,
-                [<InlineIfLambda>] binder: unit -> AsyncValidation<'ok, 'error>
+                validation1: AsyncValidation<unit, 'error>,
+                validation2: AsyncValidation<'ok, 'error>
             ) : AsyncValidation<'ok, 'error> =
-            this.Bind(result, binder)
+            this.Bind(validation1, (fun () -> validation2))
 
-        member inline this.TryWith
+        member inline _.TryWith
             (
-                [<InlineIfLambda>] generator: unit -> AsyncValidation<'ok, 'error>,
+                computation: AsyncValidation<'ok, 'error>,
                 [<InlineIfLambda>] handler: exn -> AsyncValidation<'ok, 'error>
             ) : AsyncValidation<'ok, 'error> =
-            async {
-                return!
-                    try
-                        this.Run generator
-                    with e ->
-                        handler e
-            }
-
-        member inline this.TryFinally
+            async.TryWith(computation, handler)
+
+        member inline _.TryFinally
             (
-                [<InlineIfLambda>] generator: unit -> AsyncValidation<'ok, 'error>,
+                computation: AsyncValidation<'ok, 'error>,
                 [<InlineIfLambda>] compensation: unit -> unit
             ) : AsyncValidation<'ok, 'error> =
-            async {
-                return!
-                    try
-                        this.Run generator
-                    finally
-                        compensation ()
-            }
-
-        member inline this.Using
+            async.TryFinally(computation, compensation)
+
+        member inline _.Using
             (
                 resource: 'disposable :> IDisposable,
                 [<InlineIfLambda>] binder: 'disposable -> AsyncValidation<'okOutput, 'error>
             ) : AsyncValidation<'okOutput, 'error> =
-            this.TryFinally(
-                (fun () -> binder resource),
-                (fun () ->
-                    if not (obj.ReferenceEquals(resource, null)) then
-                        resource.Dispose()
-                )
-            )
+            async.Using(resource, binder)
 
         member inline this.While
             (
                 [<InlineIfLambda>] guard: unit -> bool,
-                [<InlineIfLambda>] generator: unit -> AsyncValidation<unit, 'error>
+                computation: AsyncValidation<unit, 'error>
             ) : AsyncValidation<unit, 'error> =
-            let mutable doContinue = true
-            let mutable result = Ok()
-
-            async {
-                while doContinue
-                      && guard () do
-                    let! x = generator ()
-
-                    match x with
-                    | Ok() -> ()
-                    | Error e ->
-                        doContinue <- false
-                        result <- Error e
-
-                return result
-            }
+            if guard () then
+                let mutable whileAsync = Unchecked.defaultof<_>
+
+                whileAsync <-
+                    this.Bind(computation, (fun () -> if guard () then whileAsync else this.Zero()))
+
+                whileAsync
+            else
+                this.Zero()
+
 
         member inline this.For
             (

tests/FsToolkit.ErrorHandling.Tests/AsyncOptionCE.fs

@@ -261,6 +261,32 @@ let ``AsyncOptionCE using Tests`` =
             Expect.equal actual (Some data) "Should be ok"
             Expect.isTrue isFinished ""
         }
+
+        testCaseAsync "disposable not disposed too early"
+        <| async {
+            let mutable disposed = false
+            let mutable finished = false
+            let f1 _ = Async.retn (Some 42)
+
+            let! actual =
+                asyncOption {
+                    use d =
+                        makeDisposable (fun () ->
+                            disposed <- true
+
+                            if not finished then
+                                failwith "Should not be disposed too early"
+                        )
+
+                    let! data = f1 d
+                    finished <- true
+                    return data
+                }
+
+            Expect.equal actual (Some 42) "Should be some"
+            Expect.isTrue disposed "Should be disposed"
+        }
+
 #if NET7_0
         testCaseAsync "use sync asyncdisposable"
         <| async {

tests/FsToolkit.ErrorHandling.Tests/AsyncResultCE.fs

@@ -279,7 +279,7 @@ let ``AsyncResultCE using Tests`` =
                 }
 
             Expect.equal actual (Result.Ok data) "Should be ok"
-            Expect.isTrue isFinished ""
+            Expect.isTrue isFinished "Expected disposable to be disposed"
         }
 #if NET7_0
         testCaseAsync "use sync asyncdisposable"
@@ -301,7 +301,7 @@ let ``AsyncResultCE using Tests`` =
                 }
 
             Expect.equal actual (Result.Ok data) "Should be ok"
-            Expect.isTrue isFinished ""
+            Expect.isTrue isFinished "Expected disposable to be disposed"
         }
         testCaseAsync "use async asyncdisposable"
         <| async {
@@ -326,7 +326,7 @@ let ``AsyncResultCE using Tests`` =
                 }
 
             Expect.equal actual (Result.Ok data) "Should be ok"
-            Expect.isTrue isFinished ""
+            Expect.isTrue isFinished "Expected disposable to be disposed"
         }
 #endif
         testCaseAsync "use! normal wrapped disposable"
@@ -344,6 +344,32 @@ let ``AsyncResultCE using Tests`` =
 
             Expect.equal actual (Result.Ok data) "Should be ok"
         }
+
+        testCaseAsync "disposable not disposed too early"
+        <| async {
+            let mutable disposed = false
+            let mutable finished = false
+            let f1 _ = AsyncResult.ok 42
+
+            let! actual =
+                asyncResult {
+                    use d =
+                        makeDisposable (fun () ->
+                            disposed <- true
+
+                            if not finished then
+                                failwith "Should not be disposed too early"
+                        )
+
+                    let! data = f1 d
+                    finished <- true
+                    return data
+                }
+
+            Expect.equal actual (Ok 42) "Should be ok"
+            Expect.isTrue disposed "Should be disposed"
+        }
+
 #if !FABLE_COMPILER && NETSTANDARD2_1
         // Fable can't handle null disposables you get
         // TypeError: Cannot read property 'Dispose' of null

tests/FsToolkit.ErrorHandling.Tests/AsyncResultOptionCE.fs

@@ -438,9 +438,9 @@ let ``AsyncResultOptionCE try Tests`` =
         }
     ]
 
-let makeDisposable () =
+let makeDisposable callback =
     { new System.IDisposable with
-        member this.Dispose() = ()
+        member this.Dispose() = callback ()
     }
 
 let makeAsyncDisposable (callback) =
@@ -454,14 +454,41 @@ let ``AsyncResultOptionCE using Tests`` =
         testCaseAsync "use normal disposable"
         <| async {
             let data = 42
+            let mutable isFinished = false
 
             let! actual =
                 asyncResultOption {
-                    use d = makeDisposable ()
+                    use d = makeDisposable (fun () -> isFinished <- true)
                     return data
                 }
 
             Expect.equal actual (OkSome data) "Should be ok"
+            Expect.isTrue isFinished "Expected disposable to be disposed"
+        }
+
+        testCaseAsync "disposable not disposed too early"
+        <| async {
+            let mutable disposed = false
+            let mutable finished = false
+            let f1 _ = AsyncResult.ok 42
+
+            let! actual =
+                asyncResultOption {
+                    use d =
+                        makeDisposable (fun () ->
+                            disposed <- true
+
+                            if not finished then
+                                failwith "Should not be disposed too early"
+                        )
+
+                    let! data = f1 d
+                    finished <- true
+                    return data
+                }
+
+            Expect.equal actual (Ok(Some 42)) "Should be ok"
+            Expect.isTrue disposed "Should be disposed"
         }
 
 #if NET7_0
@@ -509,24 +536,26 @@ let ``AsyncResultOptionCE using Tests`` =
                 }
 
             Expect.equal actual (OkSome data) "Should be ok"
-            Expect.isTrue isFinished ""
+            Expect.isTrue isFinished "Expected disposable to be disposed"
         }
 #endif
 
         testCaseAsync "use! normal wrapped disposable"
         <| async {
             let data = 42
+            let mutable isFinished = false
 
             let! actual =
                 asyncResultOption {
                     use! d =
-                        makeDisposable ()
+                        makeDisposable (fun () -> isFinished <- true)
                         |> Result.Ok
 
                     return data
                 }
 
             Expect.equal actual (OkSome data) "Should be ok"
+            Expect.isTrue isFinished "Expected disposable to be disposed"
         }
 #if !FABLE_COMPILER
         // Fable can't handle null disposables you get

tests/FsToolkit.ErrorHandling.Tests/AsyncValidationCE.fs

@@ -279,40 +279,74 @@ let ``AsyncValidationCE try Tests`` =
         }
     ]
 
-let makeDisposable () =
+let makeDisposable callback =
     { new System.IDisposable with
-        member this.Dispose() = ()
+        member _.Dispose() = callback ()
     }
 
 let ``AsyncValidationCE using Tests`` =
     testList "AsyncValidationCE using Tests" [
         testCaseAsync "use normal disposable"
         <| async {
             let data = 42
+            let mutable isFinished = false
 
             let! actual =
                 asyncValidation {
-                    use d = makeDisposable ()
+                    use d = makeDisposable (fun () -> isFinished <- true)
                     return data
                 }
 
-            Expect.equal actual (Ok data) "Should be ok"
+            Expect.equal actual (Result.Ok data) "Should be ok"
+            Expect.isTrue isFinished "Expected disposable to be disposed"
         }
+
         testCaseAsync "use! normal wrapped disposable"
         <| async {
             let data = 42
+            let mutable isFinished = false
 
             let! actual =
                 asyncValidation {
                     use! d =
-                        makeDisposable ()
+                        makeDisposable (fun () -> isFinished <- true)
                         |> Ok
 
                     return data
                 }
 
             Expect.equal actual (Ok data) "Should be ok"
+            Expect.isTrue isFinished "Expected disposable to be disposed"
         }
+
+        testCaseAsync "disposable not disposed too early"
+        <| async {
+            let mutable disposed = false
+            let mutable finished = false
+            let f1 _ = AsyncResult.ok 42
+
+            let! actual =
+                asyncValidation {
+                    use d =
+                        makeDisposable (fun () ->
+                            disposed <- true
+
+                            if not finished then
+                                failwith "Should not be disposed too early"
+                        )
+
+                    let! data = f1 d
+                    finished <- true
+                    return data
+                }
+
+            Expect.equal actual (Ok 42) "Should be ok"
+            Expect.isTrue disposed "Should be disposed"
+        }
+
+#if !FABLE_COMPILER && NETSTANDARD2_1
+        // Fable can't handle null disposables you get
+        // TypeError: Cannot read property 'Dispose' of null
         testCaseAsync "use null disposable"
         <| async {
             let data = 42
@@ -325,6 +359,7 @@ let ``AsyncValidationCE using Tests`` =
 
             Expect.equal actual (Ok data) "Should be ok"
         }
+#endif
     ]
 
 let ``AsyncValidationCE loop Tests`` =