Skip to content

Commit 377bf02

Browse files
xperiandriclaude
andcommitted
fixup! feat: FlatList module with the vocabulary of the FSharp.Core Array module
A default flat list given as a sequence is rejected with `ArgumentNullException`, as it is when it is given as a flat list (D6). It converts to a sequence without a check and fails with `InvalidOperationException` when a function enumerates it, so every function that takes a sequence calls the new `checkNotDefaultSeq` of `Helper.fs` next to its `null` check: - `concat` and `transpose` for the outer sequence; - `except` for the items to exclude, also when the input is empty, and before the input is checked, as FSharp.Core checks them; - `insertManyAt` for the values, after the check of the index, because FSharp.Core enumerates the values only then; - `FlatListBuilder.addRange`, and `for` of the `flatList` expression; `yield!` of a sequence uses the helper instead of its own copy of the check. The signatures document the exceptions. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 parent 047ee9b commit 377bf02

7 files changed

Lines changed: 36 additions & 8 deletions

File tree

‎src/FSharp.Collections.Immutable/FlatList/FlatList.fs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ let collect (mapping : 'T -> FlatList<'U>) (array : FlatList<'T>) =
360360
[<CompiledName "Concat">]
361361
let concat (arrays : seq<FlatList<'T>>) =
362362
checkNotNull (nameof arrays) arrays
363+
checkNotDefaultSeq (nameof arrays) arrays
363364

364365
arrays
365366
|> Seq.map (fun array -> unwrapArray (nameof arrays) array)
@@ -383,8 +384,13 @@ let insertAt index (value : 'T) (source : FlatList<'T>) =
383384

384385
[<CompiledName "InsertManyAt">]
385386
let insertManyAt index (values : seq<'T>) (source : FlatList<'T>) =
386-
Array.insertManyAt index values (unwrapArray (nameof source) source)
387-
|> wrapArray
387+
let array = unwrapArray (nameof source) source
388+
389+
// Array.insertManyAt checks the index before it enumerates the values, so the check of the values comes after it
390+
if index >= 0 && index <= array.Length then
391+
checkNotDefaultSeq (nameof values) values
392+
393+
Array.insertManyAt index values array |> wrapArray
388394

389395
[<CompiledName "Map">]
390396
let map (mapping : 'T -> 'U) (array : FlatList<'T>) =
@@ -441,6 +447,7 @@ let rev (array : FlatList<'T>) = Array.rev (unwrapArray (nameof array) array) |>
441447
[<CompiledName "Transpose">]
442448
let transpose (arrays : seq<FlatList<'T>>) =
443449
checkNotNull (nameof arrays) arrays
450+
checkNotDefaultSeq (nameof arrays) arrays
444451

445452
arrays
446453
|> Seq.map (fun array -> unwrapArray (nameof arrays) array)
@@ -643,6 +650,7 @@ let distinctBy (projection : 'T -> 'Key) (array : FlatList<'T>) =
643650
let except (itemsToExclude : seq<'T>) (array : FlatList<'T>) =
644651
// FSharp.Core checks the items to exclude first
645652
checkNotNull (nameof itemsToExclude) itemsToExclude
653+
checkNotDefaultSeq (nameof itemsToExclude) itemsToExclude
646654
Array.except itemsToExclude (unwrapArray (nameof array) array)
647655
|> wrapArray
648656

‎src/FSharp.Collections.Immutable/FlatList/FlatList.fsi‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ val collect : mapping : ('T -> FlatList<'U>) -> array : FlatList<'T> -> FlatList
648648
/// <summary>Concatenates a sequence of flat lists.</summary>
649649
/// <param name="arrays">The flat lists.</param>
650650
/// <returns>The concatenation.</returns>
651-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="arrays"/> is <see langword="null"/>.</exception>
651+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="arrays"/> is <see langword="null"/> or a default flat list, or contains a default flat list.</exception>
652652
/// <remarks>O(the total length).</remarks>
653653
[<CompiledName "Concat">]
654654
val concat : arrays : seq<FlatList<'T>> -> FlatList<'T>
@@ -683,6 +683,7 @@ val insertAt : index : int -> value : 'T -> source : FlatList<'T> -> FlatList<'T
683683
/// <param name="values">The values to insert.</param>
684684
/// <param name="source">The input flat list.</param>
685685
/// <returns>The flat list with the values.</returns>
686+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="values"/> is <see langword="null"/> or a default flat list.</exception>
686687
/// <exception cref="T:System.ArgumentException">Thrown when the index is negative or greater than the length.</exception>
687688
/// <remarks>O(n + m).</remarks>
688689
[<CompiledName "InsertManyAt">]
@@ -807,6 +808,7 @@ val rev : array : FlatList<'T> -> FlatList<'T>
807808
/// </summary>
808809
/// <param name="arrays">The flat lists.</param>
809810
/// <returns>A flat list whose element i holds the element i of every input flat list.</returns>
811+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="arrays"/> is <see langword="null"/> or a default flat list, or contains a default flat list.</exception>
810812
/// <exception cref="T:System.ArgumentException">Thrown when the flat lists have different lengths.</exception>
811813
/// <remarks>
812814
/// O(the total length).
@@ -1214,7 +1216,7 @@ val distinctBy : projection : ('T -> 'Key) -> array : FlatList<'T> -> FlatList<'
12141216
/// <param name="itemsToExclude">The items to remove.</param>
12151217
/// <param name="array">The input flat list.</param>
12161218
/// <returns>The distinct remaining elements, in their order.</returns>
1217-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="itemsToExclude"/> is <see langword="null"/>.</exception>
1219+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="itemsToExclude"/> is <see langword="null"/> or a default flat list.</exception>
12181220
/// <remarks>
12191221
/// O(n + m).
12201222
/// </remarks>

‎src/FSharp.Collections.Immutable/FlatList/FlatListBuilder.fs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ let add (value : 'T) (builder : FlatList<'T>.Builder) =
3838

3939
[<CompiledName "AddRange">]
4040
let addRange (values : seq<'T>) (builder : FlatList<'T>.Builder) =
41+
checkNotDefaultSeq (nameof values) values
4142
builder.AddRange values
4243
builder
4344

‎src/FSharp.Collections.Immutable/FlatList/FlatListBuilder.fsi‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ val add : value : 'T -> builder : FlatList<'T>.Builder -> FlatList<'T>.Builder
8080
/// <param name="values">The values to add.</param>
8181
/// <param name="builder">The builder.</param>
8282
/// <returns>The same builder.</returns>
83+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="values"/> is <see langword="null"/> or a default flat list.</exception>
8384
[<CompiledName "AddRange">]
8485
val addRange : values : seq<'T> -> builder : FlatList<'T>.Builder -> FlatList<'T>.Builder
8586

‎src/FSharp.Collections.Immutable/FlatList/FlatListCE.fs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,7 @@ type FlatListCE () =
1414

1515
member inline _.YieldFrom (items : seq<'T>) : FlatListBuffer<'T> =
1616
// A default flat list given as a sequence is still a default flat list, as in FlatList.ofSeq (ADR 0001, D6)
17-
match items with
18-
| :? ImmutableArray<'T> as array -> checkNotDefault (nameof items) array
19-
| _ -> ()
20-
17+
checkNotDefaultSeq (nameof items) items
2118
fun builder -> builder.AddRange items
2219

2320
member inline _.YieldFrom (items : 'T array) : FlatListBuffer<'T> = fun builder -> builder.AddRange items
@@ -39,6 +36,9 @@ type FlatListCE () =
3936
fun builder -> delayed () builder
4037

4138
member inline _.For (source : seq<'U>, [<InlineIfLambda>] body : 'U -> FlatListBuffer<'T>) : FlatListBuffer<'T> =
39+
// A default flat list is a sequence that fails when it is enumerated; the expression rejects it as yield! does
40+
checkNotDefaultSeq (nameof source) source
41+
4242
fun builder ->
4343
for item in source do
4444
body item builder

‎src/FSharp.Collections.Immutable/FlatList/FlatListCE.fsi‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type FlatListCE =
5050
member inline Delay : [<InlineIfLambda>] delayed : (unit -> FlatListBuffer<'T>) -> FlatListBuffer<'T>
5151

5252
/// <summary>Adds the elements of the body for each item of a sequence.</summary>
53+
/// <exception cref="T:System.ArgumentNullException">Thrown when the sequence is a default flat list.</exception>
5354
member inline For : source : seq<'U> * [<InlineIfLambda>] body : ('U -> FlatListBuffer<'T>) -> FlatListBuffer<'T>
5455

5556
/// <summary>Adds the elements of the body while the guard holds.</summary>

‎src/FSharp.Collections.Immutable/Helper.fs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,21 @@ module internal ImmutableCollectionUtil =
3232
if array.IsDefault then
3333
nullArg name
3434

35+
/// <summary>
36+
/// Throws <see cref="T:System.ArgumentNullException"/> for a default <see cref="T:System.Collections.Immutable.ImmutableArray`1"/>
37+
/// that is given as a sequence.
38+
/// </summary>
39+
/// <remarks>
40+
/// A default immutable array converts to a sequence without a check, and the sequence fails with
41+
/// <see cref="T:System.InvalidOperationException"/> when a function enumerates it, so a function that takes a sequence
42+
/// calls this check next to its <see langword="null"/> check (ADR 0001, D6). Any other sequence passes, a
43+
/// <see langword="null"/> sequence included.
44+
/// </remarks>
45+
let inline checkNotDefaultSeq name (source : seq<'T>) =
46+
match source with
47+
| :? ImmutableArray<'T> as array -> checkNotDefault name array
48+
| _ -> ()
49+
3550
/// <summary>
3651
/// The array under an immutable array, without a copy (ADR 0001, D5).
3752
/// </summary>

0 commit comments

Comments
 (0)