fixup! feat: FlatList module with the vocabulary of the FSharp.Core Array module

Review of #26:

- `ofSeq` rejects a default flat list passed as a sequence with
  `ArgumentNullException`, as every function rejects a default flat
  list (D6); a valid flat list is still returned without a copy.
- `collect` rejects a default flat list that the mapping returns with
  `ArgumentNullException`, as `concat` does.
- `countBy` keeps one counter per key instead of every element: a
  dictionary over keys wrapped in a struct, which admits `null` keys,
  with F# structural equality and the order of first occurrence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: xperiandri

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

@@ -36,8 +36,10 @@ module FlatList =
     [<CompiledName "OfSeq">]
     let ofSeq (source : seq<'T>) =
         match source with
-        // An immutable array needs no copy
-        | :? ImmutableArray<'T> as array when not array.IsDefault -> array
+        // An immutable array needs no copy; a default one is a default FlatList, which no function accepts (ADR 0001, D6)
+        | :? ImmutableArray<'T> as array ->
+            checkNotDefault (nameof source) array
+            array
         | _ -> Array.ofSeq source |> wrapArray
 
     [<CompiledName "OfSpan">]
@@ -351,7 +353,10 @@ module FlatList =
         let builder = ImmutableArray.CreateBuilder<'U>()
 
         for item in source do
-            builder.AddRange (mapping item)
+            let items = mapping item
+            // Array.collect dereferences a null result; a default result is a default FlatList, rejected as in concat
+            checkNotDefault (nameof mapping) items
+            builder.AddRange items
 
         builder.DrainToImmutable ()
 

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

@@ -84,8 +84,8 @@ module FlatList =
     /// <summary>Creates a flat list from a sequence.</summary>
     /// <param name="source">The input sequence.</param>
     /// <returns>The flat list of the elements of the sequence.</returns>
-    /// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="source"/> is <see langword="null"/>.</exception>
-    /// <remarks>O(n).</remarks>
+    /// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="source"/> is <see langword="null"/> or a default flat list.</exception>
+    /// <remarks>O(n). A flat list is returned as it is, without a copy.</remarks>
     [<CompiledName "OfSeq">]
     val ofSeq : source : seq<'T> -> FlatList<'T>
 
@@ -502,6 +502,7 @@ module FlatList =
     /// <param name="mapping">The function that returns a flat list for each element.</param>
     /// <param name="array">The input flat list.</param>
     /// <returns>The concatenation of the returned flat lists.</returns>
+    /// <exception cref="T:System.ArgumentNullException">Thrown when the mapping returns a default flat list.</exception>
     /// <remarks>O(n + the total length of the returned flat lists).</remarks>
     [<CompiledName "Collect">]
     val collect : mapping : ('T -> FlatList<'U>) -> array : FlatList<'T> -> FlatList<'U>

src/FSharp.Collections.Immutable/Helper.fs

@@ -229,20 +229,46 @@ module internal Arrays =
 
         result
 
-    /// The keys of the projection in the order of their first occurrence, each with the number of its elements.
+    /// A key that may be null, for a dictionary, which rejects null keys.
+    [<Struct>]
+    type NullableKey<'Key> = { Key : 'Key }
+
+    /// Compares wrapped keys in F# structural equality, which handles null keys, as FSharp.Core does for its keys.
+    let nullableKeyComparer<'Key when 'Key : equality> () =
+        let structural = HashIdentity.Structural<'Key>
+
+        {
+            new IEqualityComparer<NullableKey<'Key>> with
+                member _.Equals (left, right) = structural.Equals (left.Key, right.Key)
+                member _.GetHashCode key = structural.GetHashCode key.Key
+        }
+
+    /// <summary>The keys of the projection in the order of their first occurrence, each with the number of its elements.</summary>
+    /// <remarks>The counting keeps one counter per key, not the elements, so it needs memory for the keys only.</remarks>
     let inline countBy
         ([<InlineIfLambda>] tuple : 'Key -> int -> 'R)
         (projection : 'T -> 'Key)
         (array : 'T array)
         : 'R array when 'Key : equality =
-        // ToLookup keeps the order of the first occurrence of each key and accepts null keys, as FSharp.Core does
-        let lookup = array.ToLookup ((fun item -> projection item), HashIdentity.Structural<'Key>)
-        let result = Array.zeroCreate lookup.Count
-        let mutable index = 0
+        let indices = Dictionary<NullableKey<'Key>, int>(nullableKeyComparer<'Key>())
+        let keys = ResizeArray<'Key>()
+        let counts = ResizeArray<int>()
 
-        for grouping in lookup do
-            result[index] <- tuple grouping.Key (grouping.Count ())
-            index <- index + 1
+        for item in array do
+            let key = projection item
+            let mutable index = 0
+
+            if indices.TryGetValue ({ Key = key }, &index) then
+                counts[index] <- counts[index] + 1
+            else
+                indices.Add ({ Key = key }, keys.Count)
+                keys.Add key
+                counts.Add 1
+
+        let result = Array.zeroCreate keys.Count
+
+        for index = 0 to result.Length - 1 do
+            result[index] <- tuple keys[index] counts[index]
 
         result
 

tests/FSharp.Collections.Immutable.Tests/FlatList/FlatListTests.fs

@@ -192,6 +192,15 @@ type FlatListTests () =
             // Array.concat dereferences a null element array without a check; a default element is a default flat list (D6)
             struct (nameof FlatList.concat, "arrays", (fun () -> FlatList.concat [ defaultArray ] |> ignore))
             struct (nameof FlatList.transpose, "arrays", (fun () -> FlatList.transpose [ defaultArray ] |> ignore))
+            // Array.collect dereferences a null result of the mapping; a default result is rejected as in concat
+            struct (nameof FlatList.collect,
+                    "mapping",
+                    (fun () ->
+                        FlatList.collect (fun _ -> defaultArray) (FlatList.singleton 1)
+                        |> ignore
+                    ))
+            // A default flat list passed as a sequence is still a default flat list
+            struct (nameof FlatList.ofSeq, "source", (fun () -> FlatList.ofSeq (defaultArray :> seq<int>) |> ignore))
             struct (nameof FlatList.indexOf, "array", (fun () -> FlatList.indexOf 1 defaultArray |> ignore))
             struct (nameof FlatList.indexOfWith,
                     "array",
@@ -230,6 +239,18 @@ type FlatListTests () =
                 $"FlatList.%s{name} must name the parameter of the default flat list"
             )
 
+    [<TestMethod; TestCategory(nameof FlatList.countBy)>]
+    member _.``countBy counts null keys like any other key`` () =
+        let array = FlatList.ofList Inputs.several
+        let keyOf (item : int) = if item > 0 then Some (item % 2) else None
+
+        Assert.AreEqual<struct (int option * int) list>(
+            List.countBy keyOf Inputs.several
+            |> List.map (fun (key, count) -> struct (key, count)),
+            FlatList.countBy keyOf array |> FlatList.toList,
+            "FlatList.countBy must count the key None, which is null, as List.countBy does"
+        )
+
     [<TestMethod; TestCategory(nameof FlatList.asSpan)>]
     member _.``asSpan views the elements`` () =
         let array = FlatList.ofList Inputs.several