From 53e239139270fcf4d28fb61a77c41d19734c184f Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 16:16:12 +0100 Subject: [PATCH 01/11] Add null-checks for each function that takes an `IAsyncEnumerable` or otherwise nullable type --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 26 ++++++++++ src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 48 +++++++++++++++++-- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index c4690d86..5d052348 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -30,6 +30,7 @@ module TaskSeq = // let toList (source: taskSeq<'T>) = [ + Internal.checkNonNull (nameof source) source let e = source.GetAsyncEnumerator(CancellationToken()) try @@ -40,6 +41,7 @@ module TaskSeq = ] let toArray (source: taskSeq<'T>) = [| + Internal.checkNonNull (nameof source) source let e = source.GetAsyncEnumerator(CancellationToken()) try @@ -50,6 +52,7 @@ module TaskSeq = |] let toSeq (source: taskSeq<'T>) = seq { + Internal.checkNonNull (nameof source) source let e = source.GetAsyncEnumerator(CancellationToken()) try @@ -74,6 +77,8 @@ module TaskSeq = // let ofArray (source: 'T[]) = taskSeq { + Internal.checkNonNull (nameof source) source + for c in source do yield c } @@ -84,16 +89,22 @@ module TaskSeq = } let ofSeq (source: 'T seq) = taskSeq { + Internal.checkNonNull (nameof source) source + for c in source do yield c } let ofResizeArray (source: 'T ResizeArray) = taskSeq { + Internal.checkNonNull (nameof source) source + for c in source do yield c } let ofTaskSeq (source: #Task<'T> seq) = taskSeq { + Internal.checkNonNull (nameof source) source + for c in source do let! c = c yield c @@ -106,12 +117,16 @@ module TaskSeq = } let ofTaskArray (source: #Task<'T> array) = taskSeq { + Internal.checkNonNull (nameof source) source + for c in source do let! c = c yield c } let ofAsyncSeq (source: Async<'T> seq) = taskSeq { + Internal.checkNonNull (nameof source) source + for c in source do let! c = task { return! c } yield c @@ -124,6 +139,8 @@ module TaskSeq = } let ofAsyncArray (source: Async<'T> array) = taskSeq { + Internal.checkNonNull (nameof source) source + for c in source do let! c = Async.toTask c yield c @@ -148,21 +165,29 @@ module TaskSeq = } let concat (sources: taskSeq<#taskSeq<'T>>) = taskSeq { + Internal.checkNonNull (nameof sources) sources + for ts in sources do yield! (ts :> taskSeq<'T>) } let append (source1: #taskSeq<'T>) (source2: #taskSeq<'T>) = taskSeq { + Internal.checkNonNull (nameof source1) source1 + Internal.checkNonNull (nameof source2) source2 yield! (source1 :> IAsyncEnumerable<'T>) yield! (source2 :> IAsyncEnumerable<'T>) } let appendSeq (source1: #taskSeq<'T>) (source2: #seq<'T>) = taskSeq { + Internal.checkNonNull (nameof source1) source1 + Internal.checkNonNull (nameof source2) source2 yield! (source1 :> IAsyncEnumerable<'T>) yield! (source2 :> seq<'T>) } let prependSeq (source1: #seq<'T>) (source2: #taskSeq<'T>) = taskSeq { + Internal.checkNonNull (nameof source1) source1 + Internal.checkNonNull (nameof source2) source2 yield! (source1 :> seq<'T>) yield! (source2 :> IAsyncEnumerable<'T>) } @@ -242,6 +267,7 @@ module TaskSeq = } let indexed (source: taskSeq<'T>) = taskSeq { + Internal.checkNonNull (nameof source) source let mutable i = 0 for x in source do diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 89ce51e3..9549d0fe 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -44,6 +44,11 @@ type internal InitAction<'T, 'TaskT when 'TaskT :> Task<'T>> = | InitActionAsync of async_init_item: (int -> 'TaskT) module internal TaskSeqInternal = + /// Raise an NRE for arguments that are null. Only used for 'source' parameters, never for function parameters. + let inline checkNonNull argName arg = + if isNull arg then + nullArg argName + let inline raiseEmptySeq () = ArgumentException("The asynchronous input sequence was empty.", "source") |> raise @@ -61,6 +66,7 @@ module internal TaskSeqInternal = |> raise let isEmpty (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let! step = e.MoveNextAsync() return not step @@ -93,6 +99,7 @@ module internal TaskSeqInternal = /// Returns length unconditionally, or based on a predicate let lengthBy predicate (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let mutable i = 0 @@ -128,6 +135,7 @@ module internal TaskSeqInternal = /// Returns length unconditionally, or based on a predicate let lengthBeforeMax max (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let mutable i = 0 @@ -143,6 +151,7 @@ module internal TaskSeqInternal = } let tryExactlyOne (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) match! e.MoveNextAsync() with @@ -199,6 +208,7 @@ module internal TaskSeqInternal = } let iter action (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let! step = e.MoveNextAsync() @@ -240,6 +250,7 @@ module internal TaskSeqInternal = } let fold folder initial (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let mutable result = initial @@ -264,6 +275,7 @@ module internal TaskSeqInternal = } let toResizeArrayAsync source = task { + checkNonNull (nameof source) source let res = ResizeArray() do! source |> iter (SimpleAction(fun item -> res.Add item)) return res @@ -271,37 +283,41 @@ module internal TaskSeqInternal = let toResizeArrayAndMapAsync mapper source = (toResizeArrayAsync >> Task.map mapper) source - let map mapper (taskSequence: taskSeq<_>) = + let map mapper (source: taskSeq<_>) = + checkNonNull (nameof source) source + match mapper with | CountableAction mapper -> taskSeq { let mutable i = 0 - for c in taskSequence do + for c in source do yield mapper i c i <- i + 1 } | SimpleAction mapper -> taskSeq { - for c in taskSequence do + for c in source do yield mapper c } | AsyncCountableAction mapper -> taskSeq { let mutable i = 0 - for c in taskSequence do + for c in source do let! result = mapper i c yield result i <- i + 1 } | AsyncSimpleAction mapper -> taskSeq { - for c in taskSequence do + for c in source do let! result = mapper c yield result } let zip (source1: taskSeq<_>) (source2: taskSeq<_>) = taskSeq { + checkNonNull (nameof source1) source1 + checkNonNull (nameof source2) source2 use e1 = source1.GetAsyncEnumerator(CancellationToken()) use e2 = source2.GetAsyncEnumerator(CancellationToken()) let mutable go = true @@ -317,28 +333,37 @@ module internal TaskSeqInternal = } let collect (binder: _ -> #IAsyncEnumerable<_>) (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source + for c in source do yield! binder c :> IAsyncEnumerable<_> } let collectSeq (binder: _ -> #seq<_>) (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source + for c in source do yield! binder c :> seq<_> } let collectAsync (binder: _ -> #Task<#IAsyncEnumerable<_>>) (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source + for c in source do let! result = binder c yield! result :> IAsyncEnumerable<_> } let collectSeqAsync (binder: _ -> #Task<#seq<_>>) (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source + for c in source do let! result = binder c yield! result :> seq<_> } let tryLast (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let mutable last = ValueNone @@ -356,6 +381,7 @@ module internal TaskSeqInternal = } let tryHead (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) match! e.MoveNextAsync() with @@ -364,6 +390,7 @@ module internal TaskSeqInternal = } let tryTail (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) match! e.MoveNextAsync() with @@ -384,6 +411,8 @@ module internal TaskSeqInternal = } let tryItem index (source: taskSeq<_>) = task { + checkNonNull (nameof source) source + if index < 0 then // while the loop below wouldn't run anyway, we don't want to call MoveNext in this case // to prevent side effects hitting unnecessarily @@ -409,6 +438,7 @@ module internal TaskSeqInternal = } let tryPick chooser (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true @@ -441,6 +471,7 @@ module internal TaskSeqInternal = } let tryFind predicate (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true @@ -477,6 +508,7 @@ module internal TaskSeqInternal = } let tryFindIndex predicate (source: taskSeq<_>) = task { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true @@ -509,6 +541,8 @@ module internal TaskSeqInternal = } let choose chooser (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source + match chooser with | TryPick picker -> for item in source do @@ -524,6 +558,8 @@ module internal TaskSeqInternal = } let filter predicate (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source + match predicate with | Predicate predicate -> for item in source do @@ -642,6 +678,7 @@ module internal TaskSeqInternal = ValueTask.CompletedTask let except itemsToExclude (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let! step = e.MoveNextAsync() @@ -666,6 +703,7 @@ module internal TaskSeqInternal = } let exceptOfSeq itemsToExclude (source: taskSeq<_>) = taskSeq { + checkNonNull (nameof source) source use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let! step = e.MoveNextAsync() From 7fe636a369cc8a24bde3344da92966021148e584 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 16:59:09 +0100 Subject: [PATCH 02/11] Fix some signatures that used flexible types, but shouldn't have --- README.md | 6 +++--- src/FSharp.Control.TaskSeq/TaskSeq.fs | 18 +++++++++--------- src/FSharp.Control.TaskSeq/TaskSeq.fsi | 6 +++--- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index fcf750d0..12dee846 100644 --- a/README.md +++ b/README.md @@ -462,8 +462,8 @@ The following are the current surface area of the `TaskSeq` utility functions, o ```f# module TaskSeq = - val append: source1: #taskSeq<'T> -> source2: #taskSeq<'T> -> taskSeq<'T> - val appendSeq: source1: #taskSeq<'T> -> source2: #seq<'T> -> taskSeq<'T> + val append: source1: taskSeq<'T> -> source2: taskSeq<'T> -> taskSeq<'T> + val appendSeq: source1: taskSeq<'T> -> source2: seq<'T> -> taskSeq<'T> val box: source: taskSeq<'T> -> taskSeq val cast: source: taskSeq -> taskSeq<'T> val choose: chooser: ('T -> 'U option) -> source: taskSeq<'T> -> taskSeq<'U> @@ -522,7 +522,7 @@ module TaskSeq = val ofTaskSeq: source: seq<#Task<'T>> -> taskSeq<'T> val pick: chooser: ('T -> 'U option) -> source: taskSeq<'T> -> Task<'U> val pickAsync: chooser: ('T -> #Task<'U option>) -> source: taskSeq<'T> -> Task<'U> - val prependSeq: source1: #seq<'T> -> source2: #taskSeq<'T> -> taskSeq<'T> + val prependSeq: source1: seq<'T> -> source2: taskSeq<'T> -> taskSeq<'T> val singleton: source: 'T -> taskSeq<'T> val tail: source: taskSeq<'T> -> Task> val takeWhile: predicate: ('T -> bool) -> source: taskSeq<'T> -> Task> diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index 5d052348..08818110 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -171,25 +171,25 @@ module TaskSeq = yield! (ts :> taskSeq<'T>) } - let append (source1: #taskSeq<'T>) (source2: #taskSeq<'T>) = taskSeq { + let append (source1: taskSeq<'T>) (source2: taskSeq<'T>) = taskSeq { Internal.checkNonNull (nameof source1) source1 Internal.checkNonNull (nameof source2) source2 - yield! (source1 :> IAsyncEnumerable<'T>) - yield! (source2 :> IAsyncEnumerable<'T>) + yield! source1 + yield! source2 } - let appendSeq (source1: #taskSeq<'T>) (source2: #seq<'T>) = taskSeq { + let appendSeq (source1: taskSeq<'T>) (source2: seq<'T>) = taskSeq { Internal.checkNonNull (nameof source1) source1 Internal.checkNonNull (nameof source2) source2 - yield! (source1 :> IAsyncEnumerable<'T>) - yield! (source2 :> seq<'T>) + yield! source1 + yield! source2 } - let prependSeq (source1: #seq<'T>) (source2: #taskSeq<'T>) = taskSeq { + let prependSeq (source1: seq<'T>) (source2: taskSeq<'T>) = taskSeq { Internal.checkNonNull (nameof source1) source1 Internal.checkNonNull (nameof source2) source2 - yield! (source1 :> seq<'T>) - yield! (source2 :> IAsyncEnumerable<'T>) + yield! source1 + yield! source2 } // diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fsi b/src/FSharp.Control.TaskSeq/TaskSeq.fsi index 0a4cfc59..dc51941f 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fsi +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fsi @@ -130,7 +130,7 @@ module TaskSeq = /// The second input task sequence. /// The resulting task sequence. /// Thrown when either of the input sequences is null. - val append: source1: #taskSeq<'T> -> source2: #taskSeq<'T> -> taskSeq<'T> + val append: source1: taskSeq<'T> -> source2: taskSeq<'T> -> taskSeq<'T> /// /// Concatenates a task sequence with a non-async F# in @@ -141,7 +141,7 @@ module TaskSeq = /// The input F# sequence. /// The resulting task sequence. /// Thrown when either of the input sequences is null. - val appendSeq: source1: #taskSeq<'T> -> source2: #seq<'T> -> taskSeq<'T> + val appendSeq: source1: taskSeq<'T> -> source2: seq<'T> -> taskSeq<'T> /// /// Concatenates a non-async F# in with a task sequence in @@ -152,7 +152,7 @@ module TaskSeq = /// The input task sequence. /// The resulting task sequence. /// Thrown when either of the input sequences is null. - val prependSeq: source1: #seq<'T> -> source2: #taskSeq<'T> -> taskSeq<'T> + val prependSeq: source1: seq<'T> -> source2: taskSeq<'T> -> taskSeq<'T> /// Returns taskSeq as an array. This function is blocking until the sequence is exhausted and will properly dispose of the resources. val toList: source: taskSeq<'T> -> 'T list From b3dca4ba9f4cbadce4288bfda39f78b502fae487 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 16:59:27 +0100 Subject: [PATCH 03/11] Add first tests for this --- src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs index 629dd777..faf66209 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs @@ -23,6 +23,14 @@ let validateSequence ts = |> Task.map (should equal "1234567891012345678910") module EmptySeq = + let ``Null source is invalid`` () = + let test1 () = TaskSeq.empty |> TaskSeq.append null |> TaskSeq.toList + let test2 () = null |> TaskSeq.append TaskSeq.empty |> TaskSeq.toList + let test3 () = null |> TaskSeq.append null |> TaskSeq.toList + test1 |> should throw typeof + test2 |> should throw typeof + test3 |> should throw typeof + [)>] let ``TaskSeq-append both args empty`` variant = Gen.getEmptyVariant variant From 9cf0cb89ac80065c00a74b0bff2789ea17de8915 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 17:05:13 +0100 Subject: [PATCH 04/11] A bit of cleanup along the way, consistency in coding --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index 08818110..d6060df2 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -197,11 +197,8 @@ module TaskSeq = // let cast source : taskSeq<'T> = Internal.map (SimpleAction(fun (x: obj) -> x :?> 'T)) source - let box source = Internal.map (SimpleAction(fun x -> box x)) source - - let unbox<'U when 'U: struct> (source: taskSeq) : taskSeq<'U> = - Internal.map (SimpleAction(fun x -> unbox x)) source - + let box source = Internal.map (SimpleAction box) source + let unbox<'U when 'U: struct> (source: taskSeq) : taskSeq<'U> = Internal.map (SimpleAction unbox) source let iter action source = Internal.iter (SimpleAction action) source let iteri action source = Internal.iter (CountableAction action) source let iterAsync action source = Internal.iter (AsyncSimpleAction action) source @@ -210,12 +207,9 @@ module TaskSeq = let mapi (mapper: int -> 'T -> 'U) source = Internal.map (CountableAction mapper) source let mapAsync mapper source = Internal.map (AsyncSimpleAction mapper) source let mapiAsync mapper source = Internal.map (AsyncCountableAction mapper) source - let collect (binder: 'T -> #IAsyncEnumerable<'U>) source = Internal.collect binder source + let collect (binder: 'T -> #taskSeq<'U>) source = Internal.collect binder source let collectSeq (binder: 'T -> #seq<'U>) source = Internal.collectSeq binder source - - let collectAsync (binder: 'T -> #Task<#IAsyncEnumerable<'U>>) source : taskSeq<'U> = - Internal.collectAsync binder source - + let collectAsync (binder: 'T -> #Task<#taskSeq<'U>>) source : taskSeq<'U> = Internal.collectAsync binder source let collectSeqAsync (binder: 'T -> #Task<#seq<'U>>) source : taskSeq<'U> = Internal.collectSeqAsync binder source // From a001ccccf3bb49b971fd1d53146bcd79887cda7f Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 22:18:27 +0100 Subject: [PATCH 05/11] Add more null-check tests, null-source should raise immediately, not after MoveNext --- .../Nunit.Extensions.fs | 3 +++ .../TaskSeq.Append.Tests.fs | 15 +++++++++------ .../TaskSeq.Cast.Tests.fs | 3 +++ .../TaskSeq.Choose.Tests.fs | 14 ++++++++++---- .../TaskSeq.Collect.Tests.fs | 7 +++++++ .../TaskSeq.Concat.Tests.fs | 3 +++ .../TaskSeq.Contains.Tests.fs | 3 +++ .../TaskSeq.Using.Tests.fs | 2 +- 8 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs b/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs index 8c815e69..c32a1cfd 100644 --- a/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs +++ b/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs @@ -169,3 +169,6 @@ module ExtraCustomMatchers = $"Throws %s{ex.Name} (Below, XUnit does not show actual value properly)", (fun fn -> (testForThrowing (fn :?> (unit -> Task))).Result) ) + + let inline assertThrows ty (f: unit -> 'U) = f >> ignore |> should throw ty + let inline assertNullArg f = assertThrows typeof f diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs index faf66209..8eb39454 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Append.Tests.fs @@ -22,14 +22,17 @@ let validateSequence ts = |> Task.map (String.concat "") |> Task.map (should equal "1234567891012345678910") + module EmptySeq = + [] let ``Null source is invalid`` () = - let test1 () = TaskSeq.empty |> TaskSeq.append null |> TaskSeq.toList - let test2 () = null |> TaskSeq.append TaskSeq.empty |> TaskSeq.toList - let test3 () = null |> TaskSeq.append null |> TaskSeq.toList - test1 |> should throw typeof - test2 |> should throw typeof - test3 |> should throw typeof + assertNullArg + <| fun () -> TaskSeq.empty |> TaskSeq.append null + + assertNullArg + <| fun () -> null |> TaskSeq.append TaskSeq.empty + + assertNullArg <| fun () -> null |> TaskSeq.append null [)>] let ``TaskSeq-append both args empty`` variant = diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs index 378253f4..7006d523 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs @@ -23,6 +23,9 @@ let validateSequence ts = |> Task.map (should equal "12345678910") module EmptySeq = + [] + let ``Null source is invalid`` () = assertNullArg <| fun () -> TaskSeq.box null + [)>] let ``TaskSeq-box empty`` variant = Gen.getEmptyVariant variant |> TaskSeq.box |> verifyEmpty diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Choose.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Choose.Tests.fs index df74e3eb..b5376d5b 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Choose.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Choose.Tests.fs @@ -10,13 +10,19 @@ open FsToolkit.ErrorHandling open FSharp.Control // -// TaskSeq.map -// TaskSeq.mapi -// TaskSeq.mapAsync -// TaskSeq.mapiAsync +// TaskSeq.choose +// TaskSeq.chooseAsync // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg + <| fun () -> TaskSeq.choose (fun _ -> None) null + + assertNullArg + <| fun () -> TaskSeq.chooseAsync (fun _ -> Task.fromResult None) null + [)>] let ``TaskSeq-choose`` variant = task { let! empty = diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Collect.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Collect.Tests.fs index ba975595..6b71e075 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Collect.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Collect.Tests.fs @@ -12,6 +12,13 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg + <| fun () -> TaskSeq.collect (fun _ -> TaskSeq.empty) null + + assertNullArg + <| fun () -> TaskSeq.collectAsync (fun _ -> Task.fromResult TaskSeq.empty) null [)>] let ``TaskSeq-collect collecting emptiness`` variant = diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Concat.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Concat.Tests.fs index cfbd9464..6b9f3d68 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Concat.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Concat.Tests.fs @@ -21,6 +21,9 @@ let validateSequence ts = |> Task.map (should equal "123456789101234567891012345678910") module EmptySeq = + [] + let ``Null source is invalid`` () = assertNullArg <| fun () -> TaskSeq.concat null + [)>] let ``TaskSeq-concat with empty sequences`` variant = taskSeq { diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Contains.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Contains.Tests.fs index 55792fb5..3d84956f 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Contains.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Contains.Tests.fs @@ -11,6 +11,9 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = assertNullArg <| fun () -> TaskSeq.contains 42 null + [)>] let ``TaskSeq-contains returns false`` variant = Gen.getEmptyVariant variant diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Using.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Using.Tests.fs index 8ef92f5d..bf53f14d 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Using.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Using.Tests.fs @@ -1,4 +1,4 @@ -module TaskSeq.Test.Using +module TaskSeq.Tests.Using open System open System.Threading.Tasks From 41986ecbc9bcf27367799147b3c4bfea5e31ca91 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 22:19:21 +0100 Subject: [PATCH 06/11] Add multi-iteration side-effect tests for TaskSeq.isEmpty and TaskSeq.empty --- .../TaskSeq.Empty.Tests.fs | 26 +++++++++++++++++++ .../TaskSeq.IsEmpty.fs | 15 +++++++++++ src/FSharp.Control.TaskSeq/TaskSeq.fs | 1 + 3 files changed, 42 insertions(+) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Empty.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Empty.Tests.fs index ccdba831..0e73b90f 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Empty.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Empty.Tests.fs @@ -62,3 +62,29 @@ let ``TaskSeq-empty multiple times in a taskSeq context`` () = task { Array.isEmpty sq |> should be True } + +[] +let ``TaskSeq-empty multiple times with side effects`` () = task { + let mutable x = 0 + + let sq = taskSeq { + yield! TaskSeq.empty + yield! TaskSeq.empty + x <- x + 1 + yield! TaskSeq.empty + x <- x + 1 + yield! TaskSeq.empty + x <- x + 1 + yield! TaskSeq.empty + x <- x + 1 + x <- x + 1 + } + + // executing side effects once + (TaskSeq.toArray >> Array.isEmpty) sq |> should be True + x |> should equal 5 + + // twice + (TaskSeq.toArray >> Array.isEmpty) sq |> should be True + x |> should equal 10 +} diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs index dfecb542..db53e1ae 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs @@ -60,6 +60,21 @@ module SideEffects = |> Task.map (should be True) |> Task.map (fun () -> i |> should equal 2) + [] + let ``TaskSeq-isEmpty executes side effects each time`` () = + let mutable i = 0 + + taskSeq { + i <- i + 1 + i <- i + 1 + } + |>> TaskSeq.isEmpty + |>> TaskSeq.isEmpty + |>> TaskSeq.isEmpty + |> TaskSeq.isEmpty // 4th time: 8 + |> Task.map (should be True) + |> Task.map (fun () -> i |> should equal 8) + [)>] let ``TaskSeq-isEmpty returns false for non-empty`` variant = Gen.getSeqWithSideEffect variant diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index d6060df2..494e5bb8 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -168,6 +168,7 @@ module TaskSeq = Internal.checkNonNull (nameof sources) sources for ts in sources do + // no null-check of inner taskseqs, similar to seq yield! (ts :> taskSeq<'T>) } From 4f762c28855f98d1522a27837f18f740044ff734 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 22:50:52 +0100 Subject: [PATCH 07/11] Add null-source and related tests for all other TaskSeq functions --- .../TaskSeq.ExactlyOne.Tests.fs | 5 +++ .../TaskSeq.Except.Tests.fs | 14 ++++++++ .../TaskSeq.Exists.Tests.fs | 8 +++++ .../TaskSeq.Filter.Tests.fs | 9 +++++ .../TaskSeq.Find.Tests.fs | 14 ++++++++ .../TaskSeq.FindIndex.Tests.fs | 14 ++++++++ .../TaskSeq.Fold.Tests.fs | 8 +++++ .../TaskSeq.Head.Tests.fs | 4 +++ .../TaskSeq.Indexed.Tests.fs | 3 ++ .../TaskSeq.IsEmpty.fs | 6 ++++ .../TaskSeq.Item.Tests.fs | 5 +++ .../TaskSeq.Iter.Tests.fs | 13 ++++++++ .../TaskSeq.Last.Tests.fs | 4 +++ .../TaskSeq.Length.Tests.fs | 10 ++++++ .../TaskSeq.Map.Tests.fs | 11 +++++++ .../TaskSeq.OfXXX.Tests.fs | 11 +++++++ .../TaskSeq.Pick.Tests.fs | 33 +++++++++++-------- .../TaskSeq.Tail.Tests.fs | 4 +++ .../TaskSeq.Tests.CE.fs | 13 ++++++++ .../TaskSeq.Zip.Tests.fs | 6 ++++ src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 4 +++ 21 files changed, 186 insertions(+), 13 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.ExactlyOne.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.ExactlyOne.Tests.fs index 319a5849..0438086c 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.ExactlyOne.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.ExactlyOne.Tests.fs @@ -13,6 +13,11 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.exactlyOne null + assertNullArg <| fun () -> TaskSeq.tryExactlyOne null + [)>] let ``TaskSeq-exactlyOne throws`` variant = task { fun () -> diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Except.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Except.Tests.fs index 4a946df7..b0cda375 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Except.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Except.Tests.fs @@ -14,6 +14,20 @@ open FSharp.Control module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.except null TaskSeq.empty + assertNullArg <| fun () -> TaskSeq.except TaskSeq.empty null + assertNullArg <| fun () -> TaskSeq.except null null + + assertNullArg + <| fun () -> TaskSeq.exceptOfSeq null TaskSeq.empty + + assertNullArg + <| fun () -> TaskSeq.exceptOfSeq Seq.empty null + + assertNullArg <| fun () -> TaskSeq.exceptOfSeq null null + [)>] let ``TaskSeq-except`` variant = Gen.getEmptyVariant variant diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Exists.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Exists.Tests.fs index d4b6f736..1fe3541c 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Exists.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Exists.Tests.fs @@ -12,6 +12,14 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg + <| fun () -> TaskSeq.exists (fun _ -> false) null + + assertNullArg + <| fun () -> TaskSeq.existsAsync (fun _ -> Task.fromResult false) null + [)>] let ``TaskSeq-exists returns false`` variant = Gen.getEmptyVariant variant diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Filter.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Filter.Tests.fs index 04a0352d..0dfe5963 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Filter.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Filter.Tests.fs @@ -14,6 +14,15 @@ open FSharp.Control module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg + <| fun () -> TaskSeq.filter (fun _ -> false) null + + assertNullArg + <| fun () -> TaskSeq.filterAsync (fun _ -> Task.fromResult false) null + + [)>] let ``TaskSeq-filter has no effect`` variant = Gen.getEmptyVariant variant diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Find.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Find.Tests.fs index 512b5222..61a6e02e 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Find.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Find.Tests.fs @@ -15,6 +15,20 @@ open System.Collections.Generic // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg + <| fun () -> TaskSeq.find (fun _ -> false) null + + assertNullArg + <| fun () -> TaskSeq.findAsync (fun _ -> Task.fromResult false) null + + assertNullArg + <| fun () -> TaskSeq.tryFind (fun _ -> false) null + + assertNullArg + <| fun () -> TaskSeq.tryFindAsync (fun _ -> Task.fromResult false) null + [)>] let ``TaskSeq-find raises KeyNotFoundException`` variant = fun () -> diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.FindIndex.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.FindIndex.Tests.fs index 616dab0c..f65302b0 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.FindIndex.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.FindIndex.Tests.fs @@ -15,6 +15,20 @@ open System.Collections.Generic // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg + <| fun () -> TaskSeq.findIndex (fun _ -> false) null + + assertNullArg + <| fun () -> TaskSeq.findIndexAsync (fun _ -> Task.fromResult false) null + + assertNullArg + <| fun () -> TaskSeq.tryFindIndex (fun _ -> false) null + + assertNullArg + <| fun () -> TaskSeq.tryFindIndexAsync (fun _ -> Task.fromResult false) null + [)>] let ``TaskSeq-findIndex raises KeyNotFoundException`` variant = fun () -> diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Fold.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Fold.Tests.fs index 7c28acf5..1b0791b4 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Fold.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Fold.Tests.fs @@ -13,6 +13,14 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg + <| fun () -> TaskSeq.fold (fun _ _ -> 42) 0 null + + assertNullArg + <| fun () -> TaskSeq.foldAsync (fun _ _ -> Task.fromResult 42) 0 null + [)>] let ``TaskSeq-fold takes state when empty`` variant = task { let! empty = diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Head.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Head.Tests.fs index a27b980a..a1dc1993 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Head.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Head.Tests.fs @@ -13,6 +13,10 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.head null + assertNullArg <| fun () -> TaskSeq.tryHead null [)>] let ``TaskSeq-head throws`` variant = task { diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Indexed.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Indexed.Tests.fs index 1a09b4b5..4ebab7b6 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Indexed.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Indexed.Tests.fs @@ -11,6 +11,9 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = assertNullArg <| fun () -> TaskSeq.indexed null + [)>] let ``TaskSeq-indexed on empty`` variant = Gen.getEmptyVariant variant diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs index db53e1ae..2d9de691 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.IsEmpty.fs @@ -7,7 +7,13 @@ open FsToolkit.ErrorHandling open FSharp.Control +// +// TaskSeq.isEmpty +// + module EmptySeq = + [] + let ``Null source is invalid`` () = assertNullArg <| fun () -> TaskSeq.head null [)>] let ``TaskSeq-isEmpty returns true for empty`` variant = diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Item.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Item.Tests.fs index 09b08461..5e922f90 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Item.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Item.Tests.fs @@ -13,6 +13,11 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.item 42 null + assertNullArg <| fun () -> TaskSeq.tryItem 42 null + [)>] let ``TaskSeq-item throws on empty sequences`` variant = task { fun () -> Gen.getEmptyVariant variant |> TaskSeq.item 0 |> Task.ignore diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Iter.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Iter.Tests.fs index b1561fe0..597f775f 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Iter.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Iter.Tests.fs @@ -13,6 +13,19 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.iter (fun _ -> ()) null + + assertNullArg + <| fun () -> TaskSeq.iteri (fun _ _ -> ()) null + + assertNullArg + <| fun () -> TaskSeq.iterAsync (fun _ -> Task.fromResult ()) null + + assertNullArg + <| fun () -> TaskSeq.iteriAsync (fun _ _ -> Task.fromResult ()) null + [)>] let ``TaskSeq-iteri does nothing on empty sequences`` variant = task { let tq = Gen.getEmptyVariant variant diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Last.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Last.Tests.fs index 31085217..82febc02 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Last.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Last.Tests.fs @@ -13,6 +13,10 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.last null + assertNullArg <| fun () -> TaskSeq.tryLast null [)>] let ``TaskSeq-last throws`` variant = task { diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Length.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Length.Tests.fs index ff2fc343..900aa539 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Length.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Length.Tests.fs @@ -14,6 +14,16 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.length null + + assertNullArg + <| fun () -> TaskSeq.lengthBy (fun _ -> false) null + + assertNullArg + <| fun () -> TaskSeq.lengthByAsync (fun _ -> Task.fromResult false) null + [)>] let ``TaskSeq-length returns zero on empty sequences`` variant = task { let! len = Gen.getEmptyVariant variant |> TaskSeq.length diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Map.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Map.Tests.fs index 4793684e..1f77c1f5 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Map.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Map.Tests.fs @@ -35,6 +35,17 @@ let validateSequenceWithOffset offset ts = |> Task.map (should equal expected) module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.map (fun _ -> ()) null + assertNullArg <| fun () -> TaskSeq.mapi (fun _ _ -> ()) null + + assertNullArg + <| fun () -> TaskSeq.mapAsync (fun _ -> Task.fromResult ()) null + + assertNullArg + <| fun () -> TaskSeq.mapiAsync (fun _ _ -> Task.fromResult ()) null + [)>] let ``TaskSeq-map empty`` variant = Gen.getEmptyVariant variant diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.OfXXX.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.OfXXX.Tests.fs index 035ae7ce..c32ad822 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.OfXXX.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.OfXXX.Tests.fs @@ -11,6 +11,17 @@ let validateSequence sq = |> Task.map (Seq.toArray >> should equal [| 0..9 |]) module EmptySeq = + [] + let ``Null source is invalid`` () = + // note: ofList and its variants do not have null as proper value + assertNullArg <| fun () -> TaskSeq.ofAsyncArray null + assertNullArg <| fun () -> TaskSeq.ofAsyncSeq null + assertNullArg <| fun () -> TaskSeq.ofTaskArray null + assertNullArg <| fun () -> TaskSeq.ofTaskSeq null + assertNullArg <| fun () -> TaskSeq.ofResizeArray null + assertNullArg <| fun () -> TaskSeq.ofArray null + assertNullArg <| fun () -> TaskSeq.ofSeq null + [] let ``TaskSeq-ofAsyncArray with empty set`` () = Array.init 0 (fun x -> async { return x }) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Pick.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Pick.Tests.fs index a5dc0354..9fd7ac5c 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Pick.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Pick.Tests.fs @@ -21,11 +21,22 @@ let pickerAsync equalTo x = task { return if x = equalTo then Some x else None } module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.pick (picker 0) null + assertNullArg <| fun () -> TaskSeq.tryPick (picker 0) null + + assertNullArg + <| fun () -> TaskSeq.pickAsync (pickerAsync 0) null + + assertNullArg + <| fun () -> TaskSeq.tryPickAsync (pickerAsync 0) null + [)>] let ``TaskSeq-pick on an empty sequence raises KeyNotFoundException`` variant = task { fun () -> Gen.getEmptyVariant variant - |> TaskSeq.pick (fun x -> if x = 12 then Some x else None) + |> TaskSeq.pick (picker 12) |> Task.ignore |> should throwAsyncExact typeof } @@ -34,16 +45,14 @@ module EmptySeq = let ``TaskSeq-pickAsync on an empty sequence raises KeyNotFoundException`` variant = task { fun () -> Gen.getEmptyVariant variant - |> TaskSeq.pickAsync (fun x -> task { return if x = 12 then Some x else None }) + |> TaskSeq.pickAsync (pickerAsync 12) |> Task.ignore |> should throwAsyncExact typeof } [)>] let ``TaskSeq-tryPick on an empty sequence returns None`` variant = task { - let! nothing = - Gen.getEmptyVariant variant - |> TaskSeq.tryPick (fun x -> if x = 12 then Some x else None) + let! nothing = Gen.getEmptyVariant variant |> TaskSeq.tryPick (picker 12) nothing |> should be None' } @@ -52,7 +61,7 @@ module EmptySeq = let ``TaskSeq-tryPickAsync on an empty sequence returns None`` variant = task { let! nothing = Gen.getEmptyVariant variant - |> TaskSeq.tryPickAsync (fun x -> task { return if x = 12 then Some x else None }) + |> TaskSeq.tryPickAsync (pickerAsync 12) nothing |> should be None' } @@ -63,7 +72,7 @@ module Immutable = let ``TaskSeq-pick sad path raises KeyNotFoundException`` variant = task { fun () -> Gen.getSeqImmutable variant - |> TaskSeq.pick (fun x -> if x = 0 then Some x else None) // dummy tasks sequence starts at 1 + |> TaskSeq.pick (picker 0) // dummy tasks sequence starts at 1 |> Task.ignore |> should throwAsyncExact typeof @@ -83,7 +92,7 @@ module Immutable = let ``TaskSeq-pick sad path raises KeyNotFoundException variant`` variant = task { fun () -> Gen.getSeqImmutable variant - |> TaskSeq.pick (fun x -> if x = 11 then Some x else None) // dummy tasks sequence ends at 50 + |> TaskSeq.pick (picker 11) // dummy tasks sequence ends at 50 |> Task.ignore |> should throwAsyncExact typeof @@ -93,7 +102,7 @@ module Immutable = let ``TaskSeq-pickAsync sad path raises KeyNotFoundException variant`` variant = task { fun () -> Gen.getSeqImmutable variant - |> TaskSeq.pickAsync (fun x -> task { return if x = 11 then Some x else None }) // dummy tasks sequence ends at 50 + |> TaskSeq.pickAsync (pickerAsync 11) // dummy tasks sequence ends at 50 |> Task.ignore |> should throwAsyncExact typeof @@ -164,9 +173,7 @@ module Immutable = [)>] let ``TaskSeq-tryPick sad path returns None`` variant = task { - let! nothing = - Gen.getSeqImmutable variant - |> TaskSeq.tryPick (fun x -> if x = 0 then Some x else None) // dummy tasks sequence starts at 1 + let! nothing = Gen.getSeqImmutable variant |> TaskSeq.tryPick (picker 0) // dummy tasks sequence starts at 1 nothing |> should be None' } @@ -175,7 +182,7 @@ module Immutable = let ``TaskSeq-tryPickAsync sad path return None`` variant = task { let! nothing = Gen.getSeqImmutable variant - |> TaskSeq.tryPickAsync (fun x -> task { return if x = 0 then Some x else None }) // dummy tasks sequence starts at 1 + |> TaskSeq.tryPickAsync (pickerAsync 0) // dummy tasks sequence starts at 1 nothing |> should be None' } diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tail.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tail.Tests.fs index ee38b1ad..9ba6a8af 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tail.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tail.Tests.fs @@ -13,6 +13,10 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.tail null + assertNullArg <| fun () -> TaskSeq.tryTail null [)>] let ``TaskSeq-tail throws`` variant = task { diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tests.CE.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tests.CE.fs index 33bce7bd..cd7be9b1 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tests.CE.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Tests.CE.fs @@ -1,11 +1,24 @@ module TaskSeq.Tests.``taskSeq Computation Expression`` +open System + open Xunit open FsUnit.Xunit open FsToolkit.ErrorHandling open FSharp.Control +[] +let ``CE taskSeq using yield! with null`` () = task { + let ts = taskSeq { + yield! Gen.sideEffectTaskSeq 10 + yield! (null: taskSeq) + } + + fun () -> TaskSeq.toList ts + |> assertThrows typeof +} + [] let ``CE taskSeq with several yield!`` () = task { let tskSeq = taskSeq { diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Zip.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Zip.Tests.fs index 1206d74c..2ead6106 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Zip.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Zip.Tests.fs @@ -12,6 +12,12 @@ open FSharp.Control // module EmptySeq = + [] + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.zip null TaskSeq.empty + assertNullArg <| fun () -> TaskSeq.zip TaskSeq.empty null + assertNullArg <| fun () -> TaskSeq.zip null null + [)>] let ``TaskSeq-zip can zip empty sequences v1`` variant = TaskSeq.zip (Gen.getEmptyVariant variant) (Gen.getEmptyVariant variant) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 9549d0fe..2070b21f 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -679,6 +679,8 @@ module internal TaskSeqInternal = let except itemsToExclude (source: taskSeq<_>) = taskSeq { checkNonNull (nameof source) source + checkNonNull (nameof itemsToExclude) itemsToExclude + use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let! step = e.MoveNextAsync() @@ -704,6 +706,8 @@ module internal TaskSeqInternal = let exceptOfSeq itemsToExclude (source: taskSeq<_>) = taskSeq { checkNonNull (nameof source) source + checkNonNull (nameof itemsToExclude) itemsToExclude + use e = source.GetAsyncEnumerator(CancellationToken()) let mutable go = true let! step = e.MoveNextAsync() From f88816dac7b4a7ad01b02412571e25ebfddab9fd Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 23:01:13 +0100 Subject: [PATCH 08/11] Elide the null-check in the returned task or taskSeq, instead, fail-fast --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 145 ++-- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 778 +++++++++--------- 2 files changed, 465 insertions(+), 458 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index 494e5bb8..7c8a4b69 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -51,16 +51,19 @@ module TaskSeq = e.DisposeAsync().AsTask().Wait() |] - let toSeq (source: taskSeq<'T>) = seq { + let toSeq (source: taskSeq<'T>) = Internal.checkNonNull (nameof source) source - let e = source.GetAsyncEnumerator(CancellationToken()) - try - while (let vt = e.MoveNextAsync() in if vt.IsCompleted then vt.Result else vt.AsTask().Result) do - yield e.Current - finally - e.DisposeAsync().AsTask().Wait() - } + seq { + + let e = source.GetAsyncEnumerator(CancellationToken()) + + try + while (let vt = e.MoveNextAsync() in if vt.IsCompleted then vt.Result else vt.AsTask().Result) do + yield e.Current + finally + e.DisposeAsync().AsTask().Wait() + } let toArrayAsync source = Internal.toResizeArrayAsync source @@ -76,39 +79,43 @@ module TaskSeq = // Convert 'OfXXX' functions // - let ofArray (source: 'T[]) = taskSeq { + let ofArray (source: 'T[]) = Internal.checkNonNull (nameof source) source - for c in source do - yield c - } + taskSeq { + for c in source do + yield c + } let ofList (source: 'T list) = taskSeq { for c in source do yield c } - let ofSeq (source: 'T seq) = taskSeq { + let ofSeq (source: 'T seq) = Internal.checkNonNull (nameof source) source - for c in source do - yield c - } + taskSeq { + for c in source do + yield c + } - let ofResizeArray (source: 'T ResizeArray) = taskSeq { + let ofResizeArray (source: 'T ResizeArray) = Internal.checkNonNull (nameof source) source - for c in source do - yield c - } + taskSeq { + for c in source do + yield c + } - let ofTaskSeq (source: #Task<'T> seq) = taskSeq { + let ofTaskSeq (source: #Task<'T> seq) = Internal.checkNonNull (nameof source) source - for c in source do - let! c = c - yield c - } + taskSeq { + for c in source do + let! c = c + yield c + } let ofTaskList (source: #Task<'T> list) = taskSeq { for c in source do @@ -116,21 +123,23 @@ module TaskSeq = yield c } - let ofTaskArray (source: #Task<'T> array) = taskSeq { + let ofTaskArray (source: #Task<'T> array) = Internal.checkNonNull (nameof source) source - for c in source do - let! c = c - yield c - } + taskSeq { + for c in source do + let! c = c + yield c + } - let ofAsyncSeq (source: Async<'T> seq) = taskSeq { + let ofAsyncSeq (source: Async<'T> seq) = Internal.checkNonNull (nameof source) source - for c in source do - let! c = task { return! c } - yield c - } + taskSeq { + for c in source do + let! c = task { return! c } + yield c + } let ofAsyncList (source: Async<'T> list) = taskSeq { for c in source do @@ -138,13 +147,14 @@ module TaskSeq = yield c } - let ofAsyncArray (source: Async<'T> array) = taskSeq { + let ofAsyncArray (source: Async<'T> array) = Internal.checkNonNull (nameof source) source - for c in source do - let! c = Async.toTask c - yield c - } + taskSeq { + for c in source do + let! c = Async.toTask c + yield c + } // // Utility functions @@ -164,34 +174,41 @@ module TaskSeq = member _.GetAsyncEnumerator(ct) = generator().GetAsyncEnumerator(ct) } - let concat (sources: taskSeq<#taskSeq<'T>>) = taskSeq { + let concat (sources: taskSeq<#taskSeq<'T>>) = Internal.checkNonNull (nameof sources) sources - for ts in sources do - // no null-check of inner taskseqs, similar to seq - yield! (ts :> taskSeq<'T>) - } + taskSeq { + for ts in sources do + // no null-check of inner taskseqs, similar to seq + yield! (ts :> taskSeq<'T>) + } - let append (source1: taskSeq<'T>) (source2: taskSeq<'T>) = taskSeq { + let append (source1: taskSeq<'T>) (source2: taskSeq<'T>) = Internal.checkNonNull (nameof source1) source1 Internal.checkNonNull (nameof source2) source2 - yield! source1 - yield! source2 - } - let appendSeq (source1: taskSeq<'T>) (source2: seq<'T>) = taskSeq { + taskSeq { + yield! source1 + yield! source2 + } + + let appendSeq (source1: taskSeq<'T>) (source2: seq<'T>) = Internal.checkNonNull (nameof source1) source1 Internal.checkNonNull (nameof source2) source2 - yield! source1 - yield! source2 - } - let prependSeq (source1: seq<'T>) (source2: taskSeq<'T>) = taskSeq { + taskSeq { + yield! source1 + yield! source2 + } + + let prependSeq (source1: seq<'T>) (source2: taskSeq<'T>) = Internal.checkNonNull (nameof source1) source1 Internal.checkNonNull (nameof source2) source2 - yield! source1 - yield! source2 - } + + taskSeq { + yield! source1 + yield! source2 + } // // iter/map/collect functions @@ -261,14 +278,16 @@ module TaskSeq = | None -> return invalidArg (nameof source) "The input sequence contains more than one element." } - let indexed (source: taskSeq<'T>) = taskSeq { + let indexed (source: taskSeq<'T>) = Internal.checkNonNull (nameof source) source - let mutable i = 0 - for x in source do - yield i, x - i <- i + 1 - } + taskSeq { + let mutable i = 0 + + for x in source do + yield i, x + i <- i + 1 + } let choose chooser source = Internal.choose (TryPick chooser) source let chooseAsync chooser source = Internal.choose (TryPickAsync chooser) source diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 2070b21f..35dd7a1b 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -65,12 +65,14 @@ module internal TaskSeqInternal = KeyNotFoundException("The predicate function or index did not satisfy any item in the async sequence.") |> raise - let isEmpty (source: taskSeq<_>) = task { + let isEmpty (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - let! step = e.MoveNextAsync() - return not step - } + + task { + use e = source.GetAsyncEnumerator(CancellationToken()) + let! step = e.MoveNextAsync() + return not step + } let singleton (source: 'T) = { new IAsyncEnumerable<'T> with @@ -98,75 +100,83 @@ module internal TaskSeqInternal = } /// Returns length unconditionally, or based on a predicate - let lengthBy predicate (source: taskSeq<_>) = task { + let lengthBy predicate (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let mutable i = 0 - let! step = e.MoveNextAsync() - go <- step - match predicate with - | None -> - while go do - let! step = e.MoveNextAsync() - i <- i + 1 // update before moving: we are counting, not indexing - go <- step + task { - | Some (Predicate predicate) -> - while go do - if predicate e.Current then - i <- i + 1 + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let mutable i = 0 + let! step = e.MoveNextAsync() + go <- step - let! step = e.MoveNextAsync() - go <- step + match predicate with + | None -> + while go do + let! step = e.MoveNextAsync() + i <- i + 1 // update before moving: we are counting, not indexing + go <- step - | Some (PredicateAsync predicate) -> - while go do - match! predicate e.Current with - | true -> i <- i + 1 - | false -> () + | Some (Predicate predicate) -> + while go do + if predicate e.Current then + i <- i + 1 - let! step = e.MoveNextAsync() - go <- step + let! step = e.MoveNextAsync() + go <- step - return i - } + | Some (PredicateAsync predicate) -> + while go do + match! predicate e.Current with + | true -> i <- i + 1 + | false -> () + + let! step = e.MoveNextAsync() + go <- step + + return i + } /// Returns length unconditionally, or based on a predicate - let lengthBeforeMax max (source: taskSeq<_>) = task { + let lengthBeforeMax max (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let mutable i = 0 - let! step = e.MoveNextAsync() - go <- step - while go && i < max do - i <- i + 1 // update before moving: we are counting, not indexing + task { + + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let mutable i = 0 let! step = e.MoveNextAsync() go <- step - return i - } + while go && i < max do + i <- i + 1 // update before moving: we are counting, not indexing + let! step = e.MoveNextAsync() + go <- step + + return i + } - let tryExactlyOne (source: taskSeq<_>) = task { + let tryExactlyOne (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - match! e.MoveNextAsync() with - | true -> - // grab first item and test if there's a second item - let current = e.Current + task { + use e = source.GetAsyncEnumerator(CancellationToken()) match! e.MoveNextAsync() with - | true -> return None // 2 or more items - | false -> return Some current // exactly one + | true -> + // grab first item and test if there's a second item + let current = e.Current - | false -> - // zero items - return None - } + match! e.MoveNextAsync() with + | true -> return None // 2 or more items + | false -> return Some current // exactly one + + | false -> + // zero items + return None + } let init count initializer = taskSeq { @@ -207,79 +217,85 @@ module internal TaskSeqInternal = } - let iter action (source: taskSeq<_>) = task { + let iter action (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let! step = e.MoveNextAsync() - go <- step - - // this ensures that the inner loop is optimized for the closure - // though perhaps we need to split into individual functions after all to use - // InlineIfLambda? - match action with - | CountableAction action -> - let mutable i = 0 - while go do - do action i e.Current - let! step = e.MoveNextAsync() - i <- i + 1 - go <- step + task { + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let! step = e.MoveNextAsync() + go <- step - | SimpleAction action -> - while go do - do action e.Current - let! step = e.MoveNextAsync() - go <- step + // this ensures that the inner loop is optimized for the closure + // though perhaps we need to split into individual functions after all to use + // InlineIfLambda? + match action with + | CountableAction action -> + let mutable i = 0 - | AsyncCountableAction action -> - let mutable i = 0 + while go do + do action i e.Current + let! step = e.MoveNextAsync() + i <- i + 1 + go <- step - while go do - do! action i e.Current - let! step = e.MoveNextAsync() - i <- i + 1 - go <- step + | SimpleAction action -> + while go do + do action e.Current + let! step = e.MoveNextAsync() + go <- step - | AsyncSimpleAction action -> - while go do - do! action e.Current - let! step = e.MoveNextAsync() - go <- step - } + | AsyncCountableAction action -> + let mutable i = 0 + + while go do + do! action i e.Current + let! step = e.MoveNextAsync() + i <- i + 1 + go <- step + + | AsyncSimpleAction action -> + while go do + do! action e.Current + let! step = e.MoveNextAsync() + go <- step + } - let fold folder initial (source: taskSeq<_>) = task { + let fold folder initial (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let mutable result = initial - let! step = e.MoveNextAsync() - go <- step - - match folder with - | FolderAction folder -> - while go do - result <- folder result e.Current - let! step = e.MoveNextAsync() - go <- step - | AsyncFolderAction folder -> - while go do - let! tempResult = folder result e.Current - result <- tempResult - let! step = e.MoveNextAsync() - go <- step + task { + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let mutable result = initial + let! step = e.MoveNextAsync() + go <- step - return result - } + match folder with + | FolderAction folder -> + while go do + result <- folder result e.Current + let! step = e.MoveNextAsync() + go <- step - let toResizeArrayAsync source = task { + | AsyncFolderAction folder -> + while go do + let! tempResult = folder result e.Current + result <- tempResult + let! step = e.MoveNextAsync() + go <- step + + return result + } + + let toResizeArrayAsync source = checkNonNull (nameof source) source - let res = ResizeArray() - do! source |> iter (SimpleAction(fun item -> res.Add item)) - return res - } + + task { + let res = ResizeArray() + do! source |> iter (SimpleAction(fun item -> res.Add item)) + return res + } let toResizeArrayAndMapAsync mapper source = (toResizeArrayAsync >> Task.map mapper) source @@ -315,315 +331,285 @@ module internal TaskSeqInternal = yield result } - let zip (source1: taskSeq<_>) (source2: taskSeq<_>) = taskSeq { + let zip (source1: taskSeq<_>) (source2: taskSeq<_>) = checkNonNull (nameof source1) source1 checkNonNull (nameof source2) source2 - use e1 = source1.GetAsyncEnumerator(CancellationToken()) - use e2 = source2.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let! step1 = e1.MoveNextAsync() - let! step2 = e2.MoveNextAsync() - go <- step1 && step2 - - while go do - yield e1.Current, e2.Current + + taskSeq { + use e1 = source1.GetAsyncEnumerator(CancellationToken()) + use e2 = source2.GetAsyncEnumerator(CancellationToken()) + let mutable go = true let! step1 = e1.MoveNextAsync() let! step2 = e2.MoveNextAsync() go <- step1 && step2 - } - let collect (binder: _ -> #IAsyncEnumerable<_>) (source: taskSeq<_>) = taskSeq { + while go do + yield e1.Current, e2.Current + let! step1 = e1.MoveNextAsync() + let! step2 = e2.MoveNextAsync() + go <- step1 && step2 + } + + let collect (binder: _ -> #IAsyncEnumerable<_>) (source: taskSeq<_>) = checkNonNull (nameof source) source - for c in source do - yield! binder c :> IAsyncEnumerable<_> - } + taskSeq { + for c in source do + yield! binder c :> IAsyncEnumerable<_> + } - let collectSeq (binder: _ -> #seq<_>) (source: taskSeq<_>) = taskSeq { + let collectSeq (binder: _ -> #seq<_>) (source: taskSeq<_>) = checkNonNull (nameof source) source - for c in source do - yield! binder c :> seq<_> - } + taskSeq { + for c in source do + yield! binder c :> seq<_> + } - let collectAsync (binder: _ -> #Task<#IAsyncEnumerable<_>>) (source: taskSeq<_>) = taskSeq { + let collectAsync (binder: _ -> #Task<#IAsyncEnumerable<_>>) (source: taskSeq<_>) = checkNonNull (nameof source) source - for c in source do - let! result = binder c - yield! result :> IAsyncEnumerable<_> - } + taskSeq { + for c in source do + let! result = binder c + yield! result :> IAsyncEnumerable<_> + } - let collectSeqAsync (binder: _ -> #Task<#seq<_>>) (source: taskSeq<_>) = taskSeq { + let collectSeqAsync (binder: _ -> #Task<#seq<_>>) (source: taskSeq<_>) = checkNonNull (nameof source) source - for c in source do - let! result = binder c - yield! result :> seq<_> - } + taskSeq { + for c in source do + let! result = binder c + yield! result :> seq<_> + } - let tryLast (source: taskSeq<_>) = task { + let tryLast (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let mutable last = ValueNone - let! step = e.MoveNextAsync() - go <- step - - while go do - last <- ValueSome e.Current + + task { + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let mutable last = ValueNone let! step = e.MoveNextAsync() go <- step - match last with - | ValueSome value -> return Some value - | ValueNone -> return None - } + while go do + last <- ValueSome e.Current + let! step = e.MoveNextAsync() + go <- step + + match last with + | ValueSome value -> return Some value + | ValueNone -> return None + } - let tryHead (source: taskSeq<_>) = task { + let tryHead (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - match! e.MoveNextAsync() with - | true -> return Some e.Current - | false -> return None - } + task { + use e = source.GetAsyncEnumerator(CancellationToken()) + + match! e.MoveNextAsync() with + | true -> return Some e.Current + | false -> return None + } - let tryTail (source: taskSeq<_>) = task { + let tryTail (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - - match! e.MoveNextAsync() with - | false -> return None - | true -> - return - taskSeq { - let mutable go = true - let! step = e.MoveNextAsync() - go <- step - while go do - yield e.Current + task { + use e = source.GetAsyncEnumerator(CancellationToken()) + + match! e.MoveNextAsync() with + | false -> return None + | true -> + return + taskSeq { + let mutable go = true let! step = e.MoveNextAsync() go <- step - } - |> Some - } - let tryItem index (source: taskSeq<_>) = task { + while go do + yield e.Current + let! step = e.MoveNextAsync() + go <- step + } + |> Some + } + + let tryItem index (source: taskSeq<_>) = checkNonNull (nameof source) source - if index < 0 then - // while the loop below wouldn't run anyway, we don't want to call MoveNext in this case - // to prevent side effects hitting unnecessarily - return None - else + task { + if index < 0 then + // while the loop below wouldn't run anyway, we don't want to call MoveNext in this case + // to prevent side effects hitting unnecessarily + return None + else + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let mutable idx = 0 + let mutable foundItem = None + let! step = e.MoveNextAsync() + go <- step + + while go && idx <= index do + if idx = index then + foundItem <- Some e.Current + go <- false + else + let! step = e.MoveNextAsync() + go <- step + idx <- idx + 1 + + return foundItem + } + + let tryPick chooser (source: taskSeq<_>) = + checkNonNull (nameof source) source + + task { use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true - let mutable idx = 0 let mutable foundItem = None let! step = e.MoveNextAsync() go <- step - while go && idx <= index do - if idx = index then - foundItem <- Some e.Current - go <- false - else - let! step = e.MoveNextAsync() - go <- step - idx <- idx + 1 + match chooser with + | TryPick picker -> + while go do + match picker e.Current with + | Some value -> + foundItem <- Some value + go <- false + | None -> + let! step = e.MoveNextAsync() + go <- step + + | TryPickAsync picker -> + while go do + match! picker e.Current with + | Some value -> + foundItem <- Some value + go <- false + | None -> + let! step = e.MoveNextAsync() + go <- step return foundItem - } + } - let tryPick chooser (source: taskSeq<_>) = task { + let tryFind predicate (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - - let mutable go = true - let mutable foundItem = None - let! step = e.MoveNextAsync() - go <- step - - match chooser with - | TryPick picker -> - while go do - match picker e.Current with - | Some value -> - foundItem <- Some value - go <- false - | None -> - let! step = e.MoveNextAsync() - go <- step - - | TryPickAsync picker -> - while go do - match! picker e.Current with - | Some value -> - foundItem <- Some value - go <- false - | None -> - let! step = e.MoveNextAsync() - go <- step - return foundItem - } + task { + use e = source.GetAsyncEnumerator(CancellationToken()) - let tryFind predicate (source: taskSeq<_>) = task { - checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let mutable foundItem = None + let! step = e.MoveNextAsync() + go <- step - let mutable go = true - let mutable foundItem = None - let! step = e.MoveNextAsync() - go <- step + match predicate with + | Predicate predicate -> + while go do + let current = e.Current - match predicate with - | Predicate predicate -> - while go do - let current = e.Current - - match predicate current with - | true -> - foundItem <- Some current - go <- false - | false -> - let! step = e.MoveNextAsync() - go <- step + match predicate current with + | true -> + foundItem <- Some current + go <- false + | false -> + let! step = e.MoveNextAsync() + go <- step - | PredicateAsync predicate -> - while go do - let current = e.Current + | PredicateAsync predicate -> + while go do + let current = e.Current - match! predicate current with - | true -> - foundItem <- Some current - go <- false - | false -> - let! step = e.MoveNextAsync() - go <- step + match! predicate current with + | true -> + foundItem <- Some current + go <- false + | false -> + let! step = e.MoveNextAsync() + go <- step - return foundItem - } + return foundItem + } - let tryFindIndex predicate (source: taskSeq<_>) = task { + let tryFindIndex predicate (source: taskSeq<_>) = checkNonNull (nameof source) source - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let mutable isFound = false - let mutable index = -1 - let! step = e.MoveNextAsync() - go <- step + task { + use e = source.GetAsyncEnumerator(CancellationToken()) + + let mutable go = true + let mutable isFound = false + let mutable index = -1 + let! step = e.MoveNextAsync() + go <- step - match predicate with - | Predicate predicate -> - while go && not isFound do - index <- index + 1 - isFound <- predicate e.Current + match predicate with + | Predicate predicate -> + while go && not isFound do + index <- index + 1 + isFound <- predicate e.Current - if not isFound then - let! step = e.MoveNextAsync() - go <- step + if not isFound then + let! step = e.MoveNextAsync() + go <- step - | PredicateAsync predicate -> - while go && not isFound do - index <- index + 1 - let! predicateResult = predicate e.Current - isFound <- predicateResult + | PredicateAsync predicate -> + while go && not isFound do + index <- index + 1 + let! predicateResult = predicate e.Current + isFound <- predicateResult - if not isFound then - let! step = e.MoveNextAsync() - go <- step + if not isFound then + let! step = e.MoveNextAsync() + go <- step - if isFound then return Some index else return None - } + if isFound then return Some index else return None + } - let choose chooser (source: taskSeq<_>) = taskSeq { + let choose chooser (source: taskSeq<_>) = checkNonNull (nameof source) source - match chooser with - | TryPick picker -> - for item in source do - match picker item with - | Some value -> yield value - | None -> () - - | TryPickAsync picker -> - for item in source do - match! picker item with - | Some value -> yield value - | None -> () - } + taskSeq { - let filter predicate (source: taskSeq<_>) = taskSeq { - checkNonNull (nameof source) source + match chooser with + | TryPick picker -> + for item in source do + match picker item with + | Some value -> yield value + | None -> () - match predicate with - | Predicate predicate -> - for item in source do - if predicate item then - yield item - - | PredicateAsync predicate -> - for item in source do - match! predicate item with - | true -> yield item - | false -> () - } + | TryPickAsync picker -> + for item in source do + match! picker item with + | Some value -> yield value + | None -> () + } - let takeWhile whileKind predicate (source: taskSeq<_>) = taskSeq { - use e = source.GetAsyncEnumerator(CancellationToken()) - let! step = e.MoveNextAsync() - let mutable more = step - - match whileKind, predicate with - | Exclusive, Predicate predicate -> - while more do - let value = e.Current - more <- predicate value - - if more then - yield value - let! ok = e.MoveNextAsync() - more <- ok - - | Inclusive, Predicate predicate -> - while more do - let value = e.Current - more <- predicate value - - yield value - - if more then - let! ok = e.MoveNextAsync() - more <- ok - - | Exclusive, PredicateAsync predicate -> - while more do - let value = e.Current - let! passed = predicate value - more <- passed - - if more then - yield value - let! ok = e.MoveNextAsync() - more <- ok - - | Inclusive, PredicateAsync predicate -> - while more do - let value = e.Current - let! passed = predicate value - more <- passed - - yield value - - if more then - let! ok = e.MoveNextAsync() - more <- ok - } + let filter predicate (source: taskSeq<_>) = + checkNonNull (nameof source) source + taskSeq { + match predicate with + | Predicate predicate -> + for item in source do + if predicate item then + yield item + + | PredicateAsync predicate -> + for item in source do + match! predicate item with + | true -> yield item + | false -> () + } // Consider turning using an F# version of this instead? // https://github.com/i3arnon/ConcurrentHashSet type ConcurrentHashSet<'T when 'T: equality>(ct) = @@ -677,56 +663,58 @@ module internal TaskSeqInternal = ValueTask.CompletedTask - let except itemsToExclude (source: taskSeq<_>) = taskSeq { + let except itemsToExclude (source: taskSeq<_>) = checkNonNull (nameof source) source checkNonNull (nameof itemsToExclude) itemsToExclude - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let! step = e.MoveNextAsync() - go <- step + taskSeq { + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let! step = e.MoveNextAsync() + go <- step - if step then - // only create hashset by the time we actually start iterating - use hashSet = new ConcurrentHashSet<_>(CancellationToken()) - do! hashSet.AddManyAsync itemsToExclude + if step then + // only create hashset by the time we actually start iterating + use hashSet = new ConcurrentHashSet<_>(CancellationToken()) + do! hashSet.AddManyAsync itemsToExclude - while go do - let current = e.Current + while go do + let current = e.Current - // if true, it was added, and therefore unique, so we return it - // if false, it existed, and therefore a duplicate, and we skip - if hashSet.Add current then - yield current + // if true, it was added, and therefore unique, so we return it + // if false, it existed, and therefore a duplicate, and we skip + if hashSet.Add current then + yield current - let! step = e.MoveNextAsync() - go <- step + let! step = e.MoveNextAsync() + go <- step - } + } - let exceptOfSeq itemsToExclude (source: taskSeq<_>) = taskSeq { + let exceptOfSeq itemsToExclude (source: taskSeq<_>) = checkNonNull (nameof source) source checkNonNull (nameof itemsToExclude) itemsToExclude - use e = source.GetAsyncEnumerator(CancellationToken()) - let mutable go = true - let! step = e.MoveNextAsync() - go <- step + taskSeq { + use e = source.GetAsyncEnumerator(CancellationToken()) + let mutable go = true + let! step = e.MoveNextAsync() + go <- step - if step then - // only create hashset by the time we actually start iterating - use hashSet = new ConcurrentHashSet<_>(CancellationToken()) - do hashSet.AddMany itemsToExclude + if step then + // only create hashset by the time we actually start iterating + use hashSet = new ConcurrentHashSet<_>(CancellationToken()) + do hashSet.AddMany itemsToExclude - while go do - let current = e.Current + while go do + let current = e.Current - // if true, it was added, and therefore unique, so we return it - // if false, it existed, and therefore a duplicate, and we skip - if hashSet.Add current then - yield current + // if true, it was added, and therefore unique, so we return it + // if false, it existed, and therefore a duplicate, and we skip + if hashSet.Add current then + yield current - let! step = e.MoveNextAsync() - go <- step + let! step = e.MoveNextAsync() + go <- step - } + } From eba56b5714f5bc2f15d5aded4280bd4127088cdf Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sun, 11 Dec 2022 23:18:38 +0100 Subject: [PATCH 09/11] Fix tests by always lifting null-checking internal function outside of `taskSeq` or `task` CE, or it may not fire in time --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 101 ++++++++++--------------- src/FSharp.Control.TaskSeq/TaskSeq.fsi | 2 +- 2 files changed, 41 insertions(+), 62 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index 7c8a4b69..689deda0 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -236,47 +236,39 @@ module TaskSeq = let tryHead source = Internal.tryHead source - let head source = task { - match! Internal.tryHead source with - | Some head -> return head - | None -> return Internal.raiseEmptySeq () - } + let head source = + Internal.tryHead source + |> Task.map (Option.defaultWith Internal.raiseEmptySeq) let tryLast source = Internal.tryLast source - let last source = task { - match! Internal.tryLast source with - | Some last -> return last - | None -> return Internal.raiseEmptySeq () - } + let last source = + Internal.tryLast source + |> Task.map (Option.defaultWith Internal.raiseEmptySeq) let tryTail source = Internal.tryTail source - let tail source = task { - match! Internal.tryTail source with - | Some result -> return result - | None -> return Internal.raiseEmptySeq () - } + let tail source = + Internal.tryTail source + |> Task.map (Option.defaultWith Internal.raiseEmptySeq) let tryItem index source = Internal.tryItem index source - let item index source = task { - match! Internal.tryItem index source with - | Some item -> return item - | None -> - if index < 0 then - return invalidArg (nameof index) "The input must be non-negative." - else - return Internal.raiseInsufficient () - } + let item index source = + if index < 0 then + invalidArg (nameof index) "The input must be non-negative." + + Internal.tryItem index source + |> Task.map (Option.defaultWith Internal.raiseInsufficient) let tryExactlyOne source = Internal.tryExactlyOne source - let exactlyOne source = task { - match! Internal.tryExactlyOne source with - | Some item -> return item - | None -> return invalidArg (nameof source) "The input sequence contains more than one element." - } + let exactlyOne source = + Internal.tryExactlyOne source + |> Task.map ( + Option.defaultWith (fun () -> + invalidArg (nameof source) "The input sequence contains more than one element.") + ) let indexed (source: taskSeq<'T>) = Internal.checkNonNull (nameof source) source @@ -318,47 +310,34 @@ module TaskSeq = Internal.tryFind (Predicate((=) value)) source |> Task.map (Option.isSome) - let pick chooser source = task { - match! Internal.tryPick (TryPick chooser) source with - | Some item -> return item - | None -> return Internal.raiseNotFound () - } + let pick chooser source = + Internal.tryPick (TryPick chooser) source + |> Task.map (Option.defaultWith Internal.raiseNotFound) - let pickAsync chooser source = task { - match! Internal.tryPick (TryPickAsync chooser) source with - | Some item -> return item - | None -> return Internal.raiseNotFound () - } - - let find predicate source = task { - match! Internal.tryFind (Predicate predicate) source with - | Some item -> return item - | None -> return Internal.raiseNotFound () - } + let pickAsync chooser source = + Internal.tryPick (TryPickAsync chooser) source + |> Task.map (Option.defaultWith Internal.raiseNotFound) + let find predicate source = + Internal.tryFind (Predicate predicate) source + |> Task.map (Option.defaultWith Internal.raiseNotFound) - let findAsync predicate source = task { - match! Internal.tryFind (PredicateAsync predicate) source with - | Some item -> return item - | None -> return Internal.raiseNotFound () - } + let findAsync predicate source = + Internal.tryFind (PredicateAsync predicate) source + |> Task.map (Option.defaultWith Internal.raiseNotFound) - let findIndex predicate source = task { - match! Internal.tryFindIndex (Predicate predicate) source with - | Some item -> return item - | None -> return Internal.raiseNotFound () - } + let findIndex predicate source = + Internal.tryFindIndex (Predicate predicate) source + |> Task.map (Option.defaultWith Internal.raiseNotFound) - let findIndexAsync predicate source = task { - match! Internal.tryFindIndex (PredicateAsync predicate) source with - | Some item -> return item - | None -> return Internal.raiseNotFound () - } + let findIndexAsync predicate source = + Internal.tryFindIndex (PredicateAsync predicate) source + |> Task.map (Option.defaultWith Internal.raiseNotFound) // - // zip/unzip etc functions + // zip/unzip/fold etc functions // let zip source1 source2 = Internal.zip source1 source2 diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fsi b/src/FSharp.Control.TaskSeq/TaskSeq.fsi index dc51941f..ddae68c2 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fsi +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fsi @@ -323,7 +323,7 @@ module TaskSeq = val tryItem: index: int -> source: taskSeq<'T> -> Task<'T option> /// - /// Returns the nth element of the , or if the sequence + /// Returns the nth element of the , or raises an exception if the sequence /// does not contain enough elements, or if is negative. /// /// Thrown when the sequence has insufficient length or From bee1d58150b15e62a4b2d248cdc2c25f9fc6b12f Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Mon, 12 Dec 2022 03:49:45 +0100 Subject: [PATCH 10/11] Add some more tests --- .../TaskSeq.Cast.Tests.fs | 5 ++- .../TaskSeq.Singleton.Tests.fs | 31 ++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs index 7006d523..f63e9e9d 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Cast.Tests.fs @@ -24,7 +24,10 @@ let validateSequence ts = module EmptySeq = [] - let ``Null source is invalid`` () = assertNullArg <| fun () -> TaskSeq.box null + let ``Null source is invalid`` () = + assertNullArg <| fun () -> TaskSeq.box null + assertNullArg <| fun () -> TaskSeq.unbox null + assertNullArg <| fun () -> TaskSeq.cast null [)>] let ``TaskSeq-box empty`` variant = Gen.getEmptyVariant variant |> TaskSeq.box |> verifyEmpty diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Singleton.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Singleton.Tests.fs index a916b800..aec6fbbf 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Singleton.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Singleton.Tests.fs @@ -1,12 +1,15 @@ module TaskSeq.Tests.Singleton -open System.Threading.Tasks open Xunit open FsUnit.Xunit open FsToolkit.ErrorHandling open FSharp.Control +// +// TaskSeq.singleton +// + module EmptySeq = [)>] @@ -18,6 +21,26 @@ module EmptySeq = |> TaskSeq.exactlyOne |> Task.map (should equal 10) +module SideEffects = + [] + let ``TaskSeq-singleton with a mutable value`` () = + let mutable x = 0 + let ts = TaskSeq.singleton x + x <- x + 1 + + // mutable value is dereferenced when passed to a function + ts |> TaskSeq.exactlyOne |> Task.map (should equal 0) + + [] + let ``TaskSeq-singleton with a ref cell`` () = + let x = ref 0 + let ts = TaskSeq.singleton x + x.Value <- x.Value + 1 + + ts + |> TaskSeq.exactlyOne + |> Task.map (fun x -> x.Value |> should equal 1) + module Other = [] let ``TaskSeq-singleton creates a sequence of one`` () = @@ -25,6 +48,12 @@ module Other = |> TaskSeq.exactlyOne |> Task.map (should equal 42) + [] + let ``TaskSeq-singleton with null as value`` () = + TaskSeq.singleton null + |> TaskSeq.exactlyOne + |> Task.map (should be Null) + [] let ``TaskSeq-singleton can be yielded multiple times`` () = let singleton = TaskSeq.singleton 42 From 7bbc282af9b063d0f213639ce7f42cfadc7ce915 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Mon, 19 Dec 2022 03:51:21 +0100 Subject: [PATCH 11/11] Fix by adding back `takeWhileXXX` after rebasing faux pas --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 35dd7a1b..87fc44af 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -13,7 +13,9 @@ type internal AsyncEnumStatus = [] type internal WhileKind = + /// The item under test is included even if false | Inclusive + /// The item under test is always excluded | Exclusive [] @@ -610,6 +612,61 @@ module internal TaskSeqInternal = | true -> yield item | false -> () } + + let takeWhile whileKind predicate (source: taskSeq<_>) = + checkNonNull (nameof source) source + + taskSeq { + use e = source.GetAsyncEnumerator(CancellationToken()) + let! step = e.MoveNextAsync() + let mutable more = step + + match whileKind, predicate with + | Exclusive, Predicate predicate -> + while more do + let value = e.Current + more <- predicate value + + if more then + yield value + let! ok = e.MoveNextAsync() + more <- ok + + | Inclusive, Predicate predicate -> + while more do + let value = e.Current + more <- predicate value + + yield value + + if more then + let! ok = e.MoveNextAsync() + more <- ok + + | Exclusive, PredicateAsync predicate -> + while more do + let value = e.Current + let! passed = predicate value + more <- passed + + if more then + yield value + let! ok = e.MoveNextAsync() + more <- ok + + | Inclusive, PredicateAsync predicate -> + while more do + let value = e.Current + let! passed = predicate value + more <- passed + + yield value + + if more then + let! ok = e.MoveNextAsync() + more <- ok + } + // Consider turning using an F# version of this instead? // https://github.com/i3arnon/ConcurrentHashSet type ConcurrentHashSet<'T when 'T: equality>(ct) =