From 382db5775eaf7b3f08cd7ae5f85d721067f27c6d Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Mon, 18 Dec 2023 03:13:38 +0100 Subject: [PATCH 01/11] Implement `take`, `truncate`, `skip` and `drop` --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 26 ++-- src/FSharp.Control.TaskSeq/TaskSeq.fsi | 59 ++++++++ src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 129 ++++++++++++++++-- 3 files changed, 193 insertions(+), 21 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index fb8d67b8..42c00ee5 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -6,22 +6,15 @@ open System.Threading.Tasks #nowarn "57" +// Just for convenience +module Internal = TaskSeqInternal + [] module TaskSeqExtensions = + // these need to be in a module, not a type for proper auto-initialization of generic values module TaskSeq = + let empty<'T> = Internal.empty<'T> - let empty<'T> = - { new IAsyncEnumerable<'T> with - member _.GetAsyncEnumerator(_) = - { new IAsyncEnumerator<'T> with - member _.MoveNextAsync() = ValueTask.False - member _.Current = Unchecked.defaultof<'T> - member _.DisposeAsync() = ValueTask.CompletedTask - } - } - -// Just for convenience -module Internal = TaskSeqInternal [] type TaskSeq private () = @@ -289,18 +282,27 @@ type TaskSeq private () = static member choose chooser source = Internal.choose (TryPick chooser) source static member chooseAsync chooser source = Internal.choose (TryPickAsync chooser) source + static member filter predicate source = Internal.filter (Predicate predicate) source static member filterAsync predicate source = Internal.filter (PredicateAsync predicate) source + + static member skip count source = Internal.skipOrTake Skip count source + static member drop count source = Internal.skipOrTake Drop count source + static member take count source = Internal.skipOrTake Take count source + static member truncate count source = Internal.skipOrTake Truncate count source + static member takeWhile predicate source = Internal.takeWhile Exclusive (Predicate predicate) source static member takeWhileAsync predicate source = Internal.takeWhile Exclusive (PredicateAsync predicate) source static member takeWhileInclusive predicate source = Internal.takeWhile Inclusive (Predicate predicate) source static member takeWhileInclusiveAsync predicate source = Internal.takeWhile Inclusive (PredicateAsync predicate) source + static member tryPick chooser source = Internal.tryPick (TryPick chooser) source static member tryPickAsync chooser source = Internal.tryPick (TryPickAsync chooser) source static member tryFind predicate source = Internal.tryFind (Predicate predicate) source static member tryFindAsync predicate source = Internal.tryFind (PredicateAsync predicate) source static member tryFindIndex predicate source = Internal.tryFindIndex (Predicate predicate) source static member tryFindIndexAsync predicate source = Internal.tryFindIndex (PredicateAsync predicate) source + static member except itemsToExclude source = Internal.except itemsToExclude source static member exceptOfSeq itemsToExclude source = Internal.exceptOfSeq itemsToExclude source diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fsi b/src/FSharp.Control.TaskSeq/TaskSeq.fsi index 27d27a25..a58775df 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fsi +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fsi @@ -725,6 +725,65 @@ type TaskSeq = /// Thrown when the input task sequence is null. static member filterAsync: predicate: ('T -> #Task) -> source: TaskSeq<'T> -> TaskSeq<'T> + /// + /// Returns a task sequence that, when iterated, skips elements of the + /// underlying sequence, and then returns the remainder of the elements. Raises an exception if there are not enough + /// elements in the sequence. See for a version that does not raise an exception. + /// See also for the inverse of this operation. + /// + /// + /// The number of items to skip. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + /// Thrown when is less than zero. + /// Thrown when count exceeds the number of elements in the sequence. + static member skip: count: int -> source: TaskSeq<'T> -> TaskSeq<'T> + + + /// + /// Returns a task sequence that, when iterated, drops at most elements of the + /// underlying sequence, and then returns the remainder of the elements, if any. + /// See for a version that raises an exception if there + /// are not enough elements. See also for the inverse of this operation. + /// + /// + /// The number of items to drop. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + /// Thrown when is less than zero. + static member drop: count: int -> source: TaskSeq<'T> -> TaskSeq<'T> + + /// + /// Returns a task sequence that, when iterated, yields elements of the + /// underlying sequence, and then returns no further elements. Raises an exception if there are not enough + /// elements in the sequence. See for a version that does not raise an exception. + /// See also for the inverse of this operation. + /// + /// + /// The number of items to take. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + /// Thrown when is less than zero. + /// Thrown when count exceeds the number of elements in the sequence. + static member take: count: int -> source: TaskSeq<'T> -> TaskSeq<'T> + + /// + /// Returns a task sequence that, when iterated, yields at most elements of the underlying + /// sequence, truncating the remainder, if any. + /// See for a version that raises an exception if there are not enough elements in the + /// sequence. See also for the inverse of this operation. + /// + /// + /// The maximum number of items to enumerate. + /// The input task sequence. + /// The resulting task sequence. + /// Thrown when the input task sequence is null. + /// Thrown when is less than zero. + static member truncate: count: int -> source: TaskSeq<'T> -> TaskSeq<'T> + /// /// Returns a task sequence that, when iterated, yields elements of the underlying sequence while the /// given function returns , and then returns no further elements. diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 7ea572a5..9d963b9b 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -18,6 +18,17 @@ type internal WhileKind = /// The item under test is always excluded | Exclusive +[] +type internal TakeOrSkipKind = + /// use the Seq.take semantics, raises exception if not enough elements + | Take + /// use the Seq.skip semantics, raises exception if not enough elements + | Skip + /// use the Seq.truncate semantics, safe operation, returns all if count exceeds the seq + | Truncate + /// no Seq equiv, but like Stream.drop in Scala: safe operation, return empty if not enough elements + | Drop + [] type internal Action<'T, 'U, 'TaskU when 'TaskU :> Task<'U>> = | CountableAction of countable_action: (int -> 'T -> 'U) @@ -51,20 +62,15 @@ module internal TaskSeqInternal = if isNull arg then nullArg argName - let inline raiseEmptySeq () = - ArgumentException("The asynchronous input sequence was empty.", "source") - |> raise + let inline raiseEmptySeq () = invalidArg "source" "The input task sequence was empty." - let inline raiseCannotBeNegative (name: string) = - ArgumentException("The value cannot be negative", name) - |> raise + let inline raiseCannotBeNegative name = invalidArg name "The value must be non-negative" let inline raiseInsufficient () = - ArgumentException("The asynchronous input sequence was has an insufficient number of elements.", "source") - |> raise + invalidArg "source" "The input task sequence was has an insufficient number of elements." let inline raiseNotFound () = - KeyNotFoundException("The predicate function or index did not satisfy any item in the async sequence.") + KeyNotFoundException("The predicate function or index did not satisfy any item in the task sequence.") |> raise let isEmpty (source: TaskSeq<_>) = @@ -76,6 +82,16 @@ module internal TaskSeqInternal = return not step } + let empty<'T> = + { new IAsyncEnumerable<'T> with + member _.GetAsyncEnumerator(_) = + { new IAsyncEnumerator<'T> with + member _.MoveNextAsync() = ValueTask.False + member _.Current = Unchecked.defaultof<'T> + member _.DisposeAsync() = ValueTask.CompletedTask + } + } + let singleton (value: 'T) = { new IAsyncEnumerable<'T> with member _.GetAsyncEnumerator(_) = @@ -613,6 +629,101 @@ module internal TaskSeqInternal = | false -> () } + + let skipOrTake skipOrTake count (source: TaskSeq<_>) = + checkNonNull (nameof source) source + + if count < 0 then + raiseCannotBeNegative (nameof count) + + match skipOrTake with + | Skip -> + // don't create a new sequence if count = 0 + if count = 0 then + source + else + taskSeq { + use e = source.GetAsyncEnumerator CancellationToken.None + + for _ in 1..count do + let! step = e.MoveNextAsync() + + if not step then + raiseInsufficient () + + let mutable cont = true + + while cont do + yield e.Current + let! moveNext = e.MoveNextAsync() + cont <- moveNext + + } + | Drop -> + // don't create a new sequence if count = 0 + if count = 0 then + source + else + taskSeq { + use e = source.GetAsyncEnumerator CancellationToken.None + + let! step = e.MoveNextAsync() + let mutable cont = step + let mutable pos = 0 + + // skip, or stop looping if we reached the end + while cont do + pos <- pos + 1 + let! moveNext = e.MoveNextAsync() + cont <- moveNext && pos <= count + + // return the rest + while cont do + yield e.Current + let! moveNext = e.MoveNextAsync() + cont <- moveNext + + } + | Take -> + // don't initialize an empty task sequence + if count = 0 then + empty + else + taskSeq { + use e = source.GetAsyncEnumerator CancellationToken.None + + for _ in count .. - 1 .. 1 do + let! step = e.MoveNextAsync() + + if not step then + raiseInsufficient () + + yield e.Current + } + + | Truncate -> + // don't create a new sequence if count = 0 + if count = 0 then + empty + else + taskSeq { + use e = source.GetAsyncEnumerator CancellationToken.None + + let! step = e.MoveNextAsync() + let mutable cont = step + let mutable pos = 0 + + // return items until we've exhausted the seq + // report this line, weird error: + //while! e.MoveNextAsync() && pos < 1 do + while cont do + yield e.Current + pos <- pos + 1 + let! moveNext = e.MoveNextAsync() + cont <- moveNext && pos <= count + + } + let takeWhile whileKind predicate (source: TaskSeq<_>) = checkNonNull (nameof source) source From f959fe503f2a2c91d373da57361cceab3f378899 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 03:11:32 +0100 Subject: [PATCH 02/11] Add tests for `TaskSeq.take` and `truncate` --- .../FSharp.Control.TaskSeq.Test.fsproj | 5 +- .../Nunit.Extensions.fs | 3 + .../TaskSeq.Take.fs | 248 ++++++++++++++++++ .../TaskSeq.TakeWhile.Tests.fs | 5 +- src/FSharp.Control.TaskSeq.Test/TestUtils.fs | 3 + 5 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.fs diff --git a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj index e6287c42..a40ff912 100644 --- a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj +++ b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj @@ -1,4 +1,4 @@ - + net6.0 @@ -35,8 +35,9 @@ - + + diff --git a/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs b/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs index eb67fecf..c43d459c 100644 --- a/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs +++ b/src/FSharp.Control.TaskSeq.Test/Nunit.Extensions.fs @@ -138,11 +138,14 @@ module ExtraCustomMatchers = ) /// + /// This makes a test BLOCKING!!! (TODO: get a better test framework?) + /// /// Asserts any exception that exactly matches the given exception . /// Async exceptions are almost always nested in an , however, in an /// async try/catch in F#, the exception is typically unwrapped. But this is not foolproof, and /// in cases where we just call , and will be raised regardless. /// This assertion will go over all nested exceptions and 'self', to find a matching exception. + /// /// Function to evaluate MUST return a , not a generic /// . /// Calls of xUnit to ensure proper evaluation of async. diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.fs new file mode 100644 index 00000000..13297e4c --- /dev/null +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.fs @@ -0,0 +1,248 @@ +module TaskSeq.Tests.Take + +open System + +open Xunit +open FsUnit.Xunit + +open FSharp.Control + +// +// TaskSeq.take +// TaskSeq.truncate +// + +exception SideEffectPastEnd of string + +[] +module With = + /// Turns a sequence of numbers into a string, starting with A for '1' + let verifyAsString expected = + TaskSeq.map char + >> TaskSeq.map ((+) '@') + >> TaskSeq.toArrayAsync + >> Task.map (String >> should equal expected) + +module EmptySeq = + [)>] + let ``TaskSeq-take(0) has no effect on empty input`` variant = + // no `task` block needed + Gen.getEmptyVariant variant |> TaskSeq.take 0 |> verifyEmpty + + [)>] + let ``TaskSeq-take(1) on empty input should throw InvalidOperation`` variant = + fun () -> + Gen.getEmptyVariant variant + |> TaskSeq.take 1 + |> consumeTaskSeq + + |> should throwAsyncExact typeof + + [] + let ``TaskSeq-take(-1) should throw ArgumentException on any input`` () = + fun () -> TaskSeq.empty |> TaskSeq.take -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + fun () -> TaskSeq.init 10 id |> TaskSeq.take -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + [] + let ``TaskSeq-take(-1) should throw ArgumentException before awaiting`` () = + fun () -> + taskSeq { + do! longDelay () + + if false then + yield 0 // type inference + } + |> TaskSeq.take -1 + |> ignore // throws even without running the async. Bad coding, don't ignore a task! + + |> should throw typeof + + [)>] + let ``TaskSeq-truncate(0) has no effect on empty input`` variant = + Gen.getEmptyVariant variant + |> TaskSeq.truncate 0 + |> verifyEmpty + + [)>] + let ``TaskSeq-truncate(99) does not throw on empty input`` variant = + Gen.getEmptyVariant variant + |> TaskSeq.truncate 99 + |> verifyEmpty + + + [] + let ``TaskSeq-truncate(-1) should throw ArgumentException on any input`` () = + fun () -> TaskSeq.empty |> TaskSeq.truncate -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + fun () -> TaskSeq.init 10 id |> TaskSeq.truncate -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + [] + let ``TaskSeq-truncate(-1) should throw ArgumentException before awaiting`` () = + fun () -> + taskSeq { + do! longDelay () + + if false then + yield 0 // type inference + } + |> TaskSeq.truncate -1 + |> ignore // throws even without running the async. Bad coding, don't ignore a task! + + |> should throw typeof + +module Immutable = + + [)>] + let ``TaskSeq-take returns exactly 'count' items`` variant = task { + + do! Gen.getSeqImmutable variant |> TaskSeq.take 0 |> verifyEmpty + + do! + Gen.getSeqImmutable variant + |> TaskSeq.take 1 + |> verifyAsString "A" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.take 5 + |> verifyAsString "ABCDE" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.take 10 + |> verifyAsString "ABCDEFGHIJ" + } + + [)>] + let ``TaskSeq-take throws when there are not enough elements`` variant = + fun () -> TaskSeq.init 1 id |> TaskSeq.take 2 |> consumeTaskSeq + + |> should throwAsyncExact typeof + + fun () -> + Gen.getSeqImmutable variant + |> TaskSeq.take 11 + |> consumeTaskSeq + + |> should throwAsyncExact typeof + + fun () -> + Gen.getSeqImmutable variant + |> TaskSeq.take 10_000_000 + |> consumeTaskSeq + + |> should throwAsyncExact typeof + + [)>] + let ``TaskSeq-truncate returns at least 'count' items`` variant = task { + do! + Gen.getSeqImmutable variant + |> TaskSeq.truncate 0 + |> verifyEmpty + + do! + Gen.getSeqImmutable variant + |> TaskSeq.truncate 1 + |> verifyAsString "A" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.truncate 5 + |> verifyAsString "ABCDE" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.truncate 10 + |> verifyAsString "ABCDEFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.truncate 11 + |> verifyAsString "ABCDEFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.truncate 10_000_000 + |> verifyAsString "ABCDEFGHIJ" + } + +module SideEffects = + [)>] + let ``TaskSeq-take gets enough items`` variant = + Gen.getSeqWithSideEffect variant + |> TaskSeq.take 5 + |> verifyAsString "ABCDE" + + [)>] + let ``TaskSeq-truncate gets enough items`` variant = + Gen.getSeqWithSideEffect variant + |> TaskSeq.truncate 5 + |> verifyAsString "ABCDE" + + [] + let ``TaskSeq-take prove it does not read beyond the last yield`` () = task { + let mutable x = 42 // for this test, the potential mutation should not actually occur + + let items = taskSeq { + yield x + yield x * 2 + x <- x + 1 // we are proving we never get here + } + + let expected = [| 42; 84 |] + + let! first = items |> TaskSeq.take 2 |> TaskSeq.toArrayAsync + let! repeat = items |> TaskSeq.take 2 |> TaskSeq.toArrayAsync + + first |> should equal expected + repeat |> should equal expected // if we read too far, this is now [|43, 86|] + x |> should equal 42 // expect: side-effect at end of taskseq not executed + } + + [] + let ``TaskSeq-take prove that an exception that is not consumed, is not raised`` () = + let items = taskSeq { + yield 1 + yield! [ 2; 3 ] + do SideEffectPastEnd "at the end" |> raise // we SHOULD NOT get here + } + + items |> TaskSeq.take 3 |> verifyAsString "ABC" + + + [] + let ``TaskSeq-take prove that an exception from the taskseq is thrown instead of exception from function`` () = + let items = taskSeq { + yield 42 + yield! [ 1; 2 ] + do SideEffectPastEnd "at the end" |> raise // we SHOULD get here before ArgumentException is raised + } + + fun () -> items |> TaskSeq.take 4 |> consumeTaskSeq // this would raise ArgumentException normally + |> should throwAsyncExact typeof + + + [] + let ``TaskSeq-truncate prove it does not read beyond the last yield`` () = task { + let mutable x = 42 // for this test, the potential mutation should not actually occur + + let items = taskSeq { + yield x + yield x * 2 + x <- x + 1 // we are proving we never get here + } + + let expected = [| 42; 84 |] + + let! first = items |> TaskSeq.truncate 2 |> TaskSeq.toArrayAsync + let! repeat = items |> TaskSeq.truncate 2 |> TaskSeq.toArrayAsync + + first |> should equal expected + repeat |> should equal expected // if we read too far, this is now [|43, 86|] + x |> should equal 42 // expect: side-effect at end of taskseq not executed + } diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs index 30c15924..de1ac77d 100644 --- a/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.TakeWhile.Tests.fs @@ -25,7 +25,7 @@ module With = | true, false -> TaskSeq.takeWhileInclusive | true, true -> fun pred -> TaskSeq.takeWhileInclusiveAsync (pred >> Task.fromResult) - /// adds '@' to each number and concatenates the chars before calling 'should equal' + /// Turns a sequence of numbers into a string, starting with A for '1' let verifyAsString expected = TaskSeq.map char >> TaskSeq.map ((+) '@') @@ -74,6 +74,9 @@ module EmptySeq = module Immutable = + // TaskSeq-takeWhile+A stands for: + // takeWhile + takeWhileAsync etc. + [)>] let ``TaskSeq-takeWhile+A filters correctly`` variant = task { do! diff --git a/src/FSharp.Control.TaskSeq.Test/TestUtils.fs b/src/FSharp.Control.TaskSeq.Test/TestUtils.fs index 59c8dbb9..d8afe1b6 100644 --- a/src/FSharp.Control.TaskSeq.Test/TestUtils.fs +++ b/src/FSharp.Control.TaskSeq.Test/TestUtils.fs @@ -147,6 +147,9 @@ module TestUtils = /// Spin-waits, occasionally normal delay, between 50µs - 18,000µs let microDelay () = task { do! DelayHelper.delayTask 50L<µs> 18_000L<µs> (fun _ -> ()) } + /// Consumes and returns a Task (not a Task!!!) + let consumeTaskSeq ts = TaskSeq.iter ignore ts |> Task.ignore + module Assert = /// Call MoveNextAsync() and check if return value is the expected value let moveNextAndCheck expected (enumerator: IAsyncEnumerator<_>) = task { From 140f82b39cf016f6dd7668ee182af7721f56e0db Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 03:11:47 +0100 Subject: [PATCH 03/11] Fix `TaskSeq.truncate` reading one too many from the input --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 9d963b9b..6704d868 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -67,6 +67,8 @@ module internal TaskSeqInternal = let inline raiseCannotBeNegative name = invalidArg name "The value must be non-negative" let inline raiseInsufficient () = + // this is correct, it is NOT an InvalidOperationException (see Seq.fs in F# Core) + // but instead, it's an ArgumentException... FWIW lol invalidArg "source" "The input task sequence was has an insufficient number of elements." let inline raiseNotFound () = @@ -136,7 +138,7 @@ module internal TaskSeqInternal = i <- i + 1 // update before moving: we are counting, not indexing go <- step - | Some(Predicate predicate) -> + | Some (Predicate predicate) -> while go do if predicate e.Current then i <- i + 1 @@ -144,7 +146,7 @@ module internal TaskSeqInternal = let! step = e.MoveNextAsync() go <- step - | Some(PredicateAsync predicate) -> + | Some (PredicateAsync predicate) -> while go do match! predicate e.Current with | true -> i <- i + 1 @@ -213,7 +215,7 @@ module internal TaskSeqInternal = // multiple threads access the same item through the same enumerator (which is // bad practice, but hey, who're we to judge). if isNull value then - value <- Lazy<_>.Create(fun () -> init i) + value <- Lazy<_>.Create (fun () -> init i) yield value.Force() value <- Unchecked.defaultof<_> @@ -720,7 +722,7 @@ module internal TaskSeqInternal = yield e.Current pos <- pos + 1 let! moveNext = e.MoveNextAsync() - cont <- moveNext && pos <= count + cont <- moveNext && pos < count } From bd6dcf8bfb4173a3bfb43301f42a2bcf36ebcc50 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 03:20:42 +0100 Subject: [PATCH 04/11] Fix `truncate`, do not `MoveNext` before we know we have enough elements --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 6704d868..9915f358 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -721,8 +721,12 @@ module internal TaskSeqInternal = while cont do yield e.Current pos <- pos + 1 - let! moveNext = e.MoveNextAsync() - cont <- moveNext && pos < count + + if pos < count then + let! moveNext = e.MoveNextAsync() + cont <- moveNext + else + cont <- false } From 15f1e09b123aec1b8025660364b16b90ce306d10 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 03:25:17 +0100 Subject: [PATCH 05/11] Rename source file and formatting --- .../FSharp.Control.TaskSeq.Test.fsproj | 2 +- .../{TaskSeq.Take.fs => TaskSeq.Take.Tests.fs} | 0 src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 6 +++--- 3 files changed, 4 insertions(+), 4 deletions(-) rename src/FSharp.Control.TaskSeq.Test/{TaskSeq.Take.fs => TaskSeq.Take.Tests.fs} (100%) diff --git a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj index a40ff912..dd43eaa3 100644 --- a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj +++ b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj @@ -36,7 +36,7 @@ - + diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.Tests.fs similarity index 100% rename from src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.fs rename to src/FSharp.Control.TaskSeq.Test/TaskSeq.Take.Tests.fs diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 9915f358..3294275e 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -138,7 +138,7 @@ module internal TaskSeqInternal = i <- i + 1 // update before moving: we are counting, not indexing go <- step - | Some (Predicate predicate) -> + | Some(Predicate predicate) -> while go do if predicate e.Current then i <- i + 1 @@ -146,7 +146,7 @@ module internal TaskSeqInternal = let! step = e.MoveNextAsync() go <- step - | Some (PredicateAsync predicate) -> + | Some(PredicateAsync predicate) -> while go do match! predicate e.Current with | true -> i <- i + 1 @@ -215,7 +215,7 @@ module internal TaskSeqInternal = // multiple threads access the same item through the same enumerator (which is // bad practice, but hey, who're we to judge). if isNull value then - value <- Lazy<_>.Create (fun () -> init i) + value <- Lazy<_>.Create(fun () -> init i) yield value.Force() value <- Unchecked.defaultof<_> From 54956465da7a1b84f9b76f220ae7a06dc7d0e2ba Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 03:51:51 +0100 Subject: [PATCH 06/11] Add tests for `TaskSeq.skip` and `TaskSeq.drop` --- .../FSharp.Control.TaskSeq.Test.fsproj | 1 + .../TaskSeq.Skip.Tests.fs | 233 ++++++++++++++++++ 2 files changed, 234 insertions(+) create mode 100644 src/FSharp.Control.TaskSeq.Test/TaskSeq.Skip.Tests.fs diff --git a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj index dd43eaa3..16ba1b70 100644 --- a/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj +++ b/src/FSharp.Control.TaskSeq.Test/FSharp.Control.TaskSeq.Test.fsproj @@ -35,6 +35,7 @@ + diff --git a/src/FSharp.Control.TaskSeq.Test/TaskSeq.Skip.Tests.fs b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Skip.Tests.fs new file mode 100644 index 00000000..dc0cfa1e --- /dev/null +++ b/src/FSharp.Control.TaskSeq.Test/TaskSeq.Skip.Tests.fs @@ -0,0 +1,233 @@ +module TaskSeq.Tests.Skip + +open System + +open Xunit +open FsUnit.Xunit + +open FSharp.Control + +// +// TaskSeq.skip +// TaskSeq.drop +// + +exception SideEffectPastEnd of string + +[] +module With = + /// Turns a sequence of numbers into a string, starting with A for '1' + let verifyAsString expected = + TaskSeq.map char + >> TaskSeq.map ((+) '@') + >> TaskSeq.toArrayAsync + >> Task.map (String >> should equal expected) + +module EmptySeq = + [)>] + let ``TaskSeq-skip(0) has no effect on empty input`` variant = + // no `task` block needed + Gen.getEmptyVariant variant |> TaskSeq.skip 0 |> verifyEmpty + + [)>] + let ``TaskSeq-skip(1) on empty input should throw InvalidOperation`` variant = + fun () -> + Gen.getEmptyVariant variant + |> TaskSeq.skip 1 + |> consumeTaskSeq + + |> should throwAsyncExact typeof + + [] + let ``TaskSeq-skip(-1) should throw ArgumentException on any input`` () = + fun () -> TaskSeq.empty |> TaskSeq.skip -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + fun () -> TaskSeq.init 10 id |> TaskSeq.skip -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + [] + let ``TaskSeq-skip(-1) should throw ArgumentException before awaiting`` () = + fun () -> + taskSeq { + do! longDelay () + + if false then + yield 0 // type inference + } + |> TaskSeq.skip -1 + |> ignore // throws even without running the async. Bad coding, don't ignore a task! + + |> should throw typeof + + [)>] + let ``TaskSeq-drop(0) has no effect on empty input`` variant = Gen.getEmptyVariant variant |> TaskSeq.drop 0 |> verifyEmpty + + [)>] + let ``TaskSeq-drop(99) does not throw on empty input`` variant = + Gen.getEmptyVariant variant + |> TaskSeq.drop 99 + |> verifyEmpty + + + [] + let ``TaskSeq-drop(-1) should throw ArgumentException on any input`` () = + fun () -> TaskSeq.empty |> TaskSeq.drop -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + fun () -> TaskSeq.init 10 id |> TaskSeq.drop -1 |> consumeTaskSeq + |> should throwAsyncExact typeof + + [] + let ``TaskSeq-drop(-1) should throw ArgumentException before awaiting`` () = + fun () -> + taskSeq { + do! longDelay () + + if false then + yield 0 // type inference + } + |> TaskSeq.drop -1 + |> ignore // throws even without running the async. Bad coding, don't ignore a task! + + |> should throw typeof + +module Immutable = + + [)>] + let ``TaskSeq-skip skips over exactly 'count' items`` variant = task { + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skip 0 + |> verifyAsString "ABCDEFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skip 1 + |> verifyAsString "BCDEFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skip 5 + |> verifyAsString "FGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.skip 10 + |> verifyEmpty + } + + [)>] + let ``TaskSeq-skip throws when there are not enough elements`` variant = + fun () -> TaskSeq.init 1 id |> TaskSeq.skip 2 |> consumeTaskSeq + + |> should throwAsyncExact typeof + + fun () -> + Gen.getSeqImmutable variant + |> TaskSeq.skip 11 + |> consumeTaskSeq + + |> should throwAsyncExact typeof + + fun () -> + Gen.getSeqImmutable variant + |> TaskSeq.skip 10_000_000 + |> consumeTaskSeq + + |> should throwAsyncExact typeof + + [)>] + let ``TaskSeq-drop skips over at least 'count' items`` variant = task { + do! + Gen.getSeqImmutable variant + |> TaskSeq.drop 0 + |> verifyAsString "ABCDEFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.drop 1 + |> verifyAsString "BCDEFGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.drop 5 + |> verifyAsString "FGHIJ" + + do! + Gen.getSeqImmutable variant + |> TaskSeq.drop 10 + |> verifyEmpty + + do! + Gen.getSeqImmutable variant + |> TaskSeq.drop 11 // no exception + |> verifyEmpty + + do! + Gen.getSeqImmutable variant + |> TaskSeq.drop 10_000_000 // no exception + |> verifyEmpty + } + +module SideEffects = + [)>] + let ``TaskSeq-skip skips over enough items`` variant = + Gen.getSeqWithSideEffect variant + |> TaskSeq.skip 5 + |> verifyAsString "FGHIJ" + + [)>] + let ``TaskSeq-drop skips over enough items`` variant = + Gen.getSeqWithSideEffect variant + |> TaskSeq.drop 5 + |> verifyAsString "FGHIJ" + + [] + let ``TaskSeq-skip prove we do not skip side effects`` () = task { + let mutable x = 42 // for this test, the potential mutation should not actually occur + + let items = taskSeq { + yield x + yield x * 2 + x <- x + 1 // we are proving we never get here + } + + let! first = items |> TaskSeq.skip 2 |> TaskSeq.toArrayAsync + let! repeat = items |> TaskSeq.skip 2 |> TaskSeq.toArrayAsync + + first |> should equal Array.empty + repeat |> should equal Array.empty + x |> should equal 44 // expect: side-effect is executed twice by now + } + + [] + let ``TaskSeq-skip prove that an exception from the taskseq is thrown instead of exception from function`` () = + let items = taskSeq { + yield 42 + yield! [ 1; 2 ] + do SideEffectPastEnd "at the end" |> raise // we SHOULD get here before ArgumentException is raised + } + + fun () -> items |> TaskSeq.skip 4 |> consumeTaskSeq // this would raise ArgumentException normally + |> should throwAsyncExact typeof + + + [] + let ``TaskSeq-drop prove we do not skip side effects at the end`` () = task { + let mutable x = 42 // for this test, the potential mutation should not actually occur + + let items = taskSeq { + yield x + yield x * 2 + x <- x + 1 // we are proving we never get here + } + + let! first = items |> TaskSeq.drop 2 |> TaskSeq.toArrayAsync + let! repeat = items |> TaskSeq.drop 2 |> TaskSeq.toArrayAsync + + first |> should equal Array.empty + repeat |> should equal Array.empty + x |> should equal 44 // expect: side-effect at end is executed twice by now + } From 6e6821ee6c6836705832861b6222cf122404ad1a Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 03:54:21 +0100 Subject: [PATCH 07/11] Fix `TaskSeq.skip` skipped one too few, and simplify --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 3294275e..9e396530 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -653,12 +653,8 @@ module internal TaskSeqInternal = if not step then raiseInsufficient () - let mutable cont = true - - while cont do + while! e.MoveNextAsync() do yield e.Current - let! moveNext = e.MoveNextAsync() - cont <- moveNext } | Drop -> From dd5b08583afb9c241a755d4dfaf0baa32c6342d9 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 03:58:30 +0100 Subject: [PATCH 08/11] Fix and simplify code for `TaskSeq.drop` --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 9e396530..0165ba65 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -672,14 +672,16 @@ module internal TaskSeqInternal = // skip, or stop looping if we reached the end while cont do pos <- pos + 1 - let! moveNext = e.MoveNextAsync() - cont <- moveNext && pos <= count + + if pos < count then + let! moveNext = e.MoveNextAsync() + cont <- moveNext + else + cont <- false // return the rest - while cont do + while! e.MoveNextAsync() do yield e.Current - let! moveNext = e.MoveNextAsync() - cont <- moveNext } | Take -> From 9acc527b4ee3b893e07dc3bfbf9469fd7b9c7d60 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 04:21:10 +0100 Subject: [PATCH 09/11] Temporarily ignore formatting for some files, see https://github.com/fsprojects/fantomas/issues/3019 --- .fantomasignore | 1 + src/.fantomasignore | 1 + 2 files changed, 2 insertions(+) create mode 100644 .fantomasignore create mode 100644 src/.fantomasignore diff --git a/.fantomasignore b/.fantomasignore new file mode 100644 index 00000000..cfebbd44 --- /dev/null +++ b/.fantomasignore @@ -0,0 +1 @@ +TaskSeqInternal.fs \ No newline at end of file diff --git a/src/.fantomasignore b/src/.fantomasignore new file mode 100644 index 00000000..cfebbd44 --- /dev/null +++ b/src/.fantomasignore @@ -0,0 +1 @@ +TaskSeqInternal.fs \ No newline at end of file From 79f0bc48f8436c02ea8d111d81b6209a288f29f8 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 15:34:48 +0100 Subject: [PATCH 10/11] Apply review comments, thanks @bartelink, fix naming and doc comments --- src/FSharp.Control.TaskSeq/TaskSeq.fsi | 20 +++++++++++-------- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 6 ++---- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fsi b/src/FSharp.Control.TaskSeq/TaskSeq.fsi index a58775df..3c14581d 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fsi +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fsi @@ -726,9 +726,9 @@ type TaskSeq = static member filterAsync: predicate: ('T -> #Task) -> source: TaskSeq<'T> -> TaskSeq<'T> /// - /// Returns a task sequence that, when iterated, skips elements of the - /// underlying sequence, and then returns the remainder of the elements. Raises an exception if there are not enough - /// elements in the sequence. See for a version that does not raise an exception. + /// Returns a task sequence that, when iterated, skips elements of the underlying + /// sequence, and then yields the remainder. Raises an exception if there are not + /// items. See for a version that does not raise an exception. /// See also for the inverse of this operation. /// /// @@ -736,8 +736,10 @@ type TaskSeq = /// The input task sequence. /// The resulting task sequence. /// Thrown when the input task sequence is null. - /// Thrown when is less than zero. - /// Thrown when count exceeds the number of elements in the sequence. + /// + /// Thrown when is less than zero or when + /// it exceeds the number of elements in the sequence. + /// static member skip: count: int -> source: TaskSeq<'T> -> TaskSeq<'T> @@ -748,7 +750,7 @@ type TaskSeq = /// are not enough elements. See also for the inverse of this operation. /// /// - /// The number of items to drop. + /// The maximum number of items to drop. /// The input task sequence. /// The resulting task sequence. /// Thrown when the input task sequence is null. @@ -766,8 +768,10 @@ type TaskSeq = /// The input task sequence. /// The resulting task sequence. /// Thrown when the input task sequence is null. - /// Thrown when is less than zero. - /// Thrown when count exceeds the number of elements in the sequence. + /// + /// Thrown when is less than zero or when + /// it exceeds the number of elements in the sequence. + /// static member take: count: int -> source: TaskSeq<'T> -> TaskSeq<'T> /// diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 0165ba65..d6a33421 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -648,9 +648,9 @@ module internal TaskSeqInternal = use e = source.GetAsyncEnumerator CancellationToken.None for _ in 1..count do - let! step = e.MoveNextAsync() + let! ok = e.MoveNextAsync() - if not step then + if not ok then raiseInsufficient () while! e.MoveNextAsync() do @@ -714,8 +714,6 @@ module internal TaskSeqInternal = let mutable pos = 0 // return items until we've exhausted the seq - // report this line, weird error: - //while! e.MoveNextAsync() && pos < 1 do while cont do yield e.Current pos <- pos + 1 From d1994343688caf38acff8046e32af7dac5fc87b2 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Tue, 19 Dec 2023 17:57:32 +0100 Subject: [PATCH 11/11] Revert "Temporarily ignore formatting for some files, see https://github.com/fsprojects/fantomas/issues/3019" This reverts commit 9acc527b4ee3b893e07dc3bfbf9469fd7b9c7d60. --- .fantomasignore | 1 - src/.fantomasignore | 1 - 2 files changed, 2 deletions(-) delete mode 100644 .fantomasignore delete mode 100644 src/.fantomasignore diff --git a/.fantomasignore b/.fantomasignore deleted file mode 100644 index cfebbd44..00000000 --- a/.fantomasignore +++ /dev/null @@ -1 +0,0 @@ -TaskSeqInternal.fs \ No newline at end of file diff --git a/src/.fantomasignore b/src/.fantomasignore deleted file mode 100644 index cfebbd44..00000000 --- a/src/.fantomasignore +++ /dev/null @@ -1 +0,0 @@ -TaskSeqInternal.fs \ No newline at end of file