From ca48b2be4c5bde008216f6997b6fc68b871852c9 Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Sun, 25 Oct 2026 08:31:44 -0500 Subject: [PATCH 1/5] fix(js/ts): add FSharp.Core's argument checks to collection functions Collection functions that FSharp.Core rejects arguments for returned a value (often a wrong one), and a few raised where FSharp.Core does not. They now behave like FSharp.Core, raising its exception types with its ParamNames: - List.fold2, map2, mapi2, map3, zip, zip3, iter2, iteri2 and forall2 raise ArgumentException for lists of different lengths, after calling the function on the common items. forall2 still stops at the first false. List.foldBack2 names the lists like FSharp.Core, and List.exists2 now gives the same "The lists had different lengths." message as the others. - List.init, List.replicate, Seq.init and Seq.take raise ArgumentException for negative counts, Seq.replicate ArgumentOutOfRangeException. The Seq functions check when called, like Seq.chunkBySize, Seq.windowed and Seq.splitInto now do. - List.average and List.averageBy raise ArgumentException on empty lists. - Array.sub and Array.fill check their ranges. Array.blit and System.Array.Copy, which now compiles to Array.copyTo instead of Util.copyToArray, raise System.Array.Copy's exceptions and leave the target untouched. The Int64 overloads of System.Array.Copy convert their arguments (they failed before), and a copy within one array handles overlapping ranges like System.Array.Copy. - List, Array and Seq.removeManyAt raise ArgumentException when FSharp.Core does, and a count of 0 removes nothing. A negative count removes nothing from an array, where FSharp.Core gives odd results. - Seq.transpose groups jagged input by index like FSharp.Core instead of raising. Array.transpose and List.transpose still raise. - Array.chunkBySize and Array.splitInto (and so List's) use FSharp.Core's ParamNames. --- src/Fable.Transforms/Replacements.fs | 18 ++++- src/fable-library-ts/Array.fs | 117 +++++++++++++++++---------- src/fable-library-ts/List.fs | 88 ++++++++++++-------- src/fable-library-ts/Seq.fs | 83 +++++++++++++------ tests/Js/Main/ArrayTests.fs | 85 +++++++++++++++++++ tests/Js/Main/ListTests.fs | 59 ++++++++++++++ tests/Js/Main/SeqTests.fs | 37 +++++++++ tests/Js/Main/Util/Util.fs | 1 + 8 files changed, 384 insertions(+), 104 deletions(-) diff --git a/src/Fable.Transforms/Replacements.fs b/src/Fable.Transforms/Replacements.fs index 80c7a1de2d..f3e0cd8d38 100644 --- a/src/Fable.Transforms/Replacements.fs +++ b/src/Fable.Transforms/Replacements.fs @@ -2339,14 +2339,28 @@ let tuples (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (thisArg: E | "ToTuple", _ -> changeKind false args | _ -> None +/// System.Array.Copy compiles to Array.copyTo, which checks the arguments like it (Util.copyToArray doesn't). +/// Array.copyTo takes ints, so the Int64 overloads convert their arguments first. +let arrayCopy (com: ICompiler) (ctx: Context) r t source sourceIndex target targetIndex count = + let toInt32 (arg: Expr) = + match arg.Type with + | Number(Int32, _) -> arg + | _ -> toInt com ctx r Int32.Number [ arg ] + + let args = + [ source; toInt32 sourceIndex; target; toInt32 targetIndex; toInt32 count ] + + Helper.LibCall(com, "Array", "copyTo", t, args, ?loc = r) |> Some + let arrays (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (thisArg: Expr option) (args: Expr list) = match i.CompiledName, thisArg, args with | "get_Length", Some arg, _ -> getFieldWith r t arg "length" |> Some | "get_Item", Some arg, [ idx ] -> getExpr r t arg idx |> Some | "set_Item", Some arg, [ idx; value ] -> setExpr r arg idx value |> Some - | "Copy", None, [ _source; _sourceIndex; _target; _targetIndex; _count ] -> copyToArray com r t i args + | "Copy", None, [ source; sourceIndex; target; targetIndex; count ] -> + arrayCopy com ctx r t source sourceIndex target targetIndex count | "Copy", None, [ source; target; count ] -> - copyToArray com r t i [ source; makeIntConst 0; target; makeIntConst 0; count ] + arrayCopy com ctx r t source (makeIntConst 0) target (makeIntConst 0) count | "ConvertAll", None, [ source; mapping ] -> Helper.LibCall(com, "Array", "map", t, [ mapping; source ], ?loc = r) |> Some | "IndexOf", None, args -> diff --git a/src/fable-library-ts/Array.fs b/src/fable-library-ts/Array.fs index fd7cbe5a3a..35bd0bb11c 100644 --- a/src/fable-library-ts/Array.fs +++ b/src/fable-library-ts/Array.fs @@ -36,9 +36,30 @@ let append (array1: 'T[]) (array2: 'T[]) ([] cons: Con let filter (predicate: 'T -> bool) (array: 'T[]) = filterImpl predicate array // intentionally returns target instead of unit -let fill (target: 'T[]) (targetIndex: int) (count: int) (value: 'T) : 'T[] = fillImpl target value targetIndex count +let fill (target: 'T[]) (targetIndex: int) (count: int) (value: 'T) : 'T[] = + if targetIndex < 0 then + invalidArg "targetIndex" SR.inputMustBeNonNegative -let getSubArray (array: 'T[]) (start: int) (count: int) : 'T[] = subArrayImpl array start count + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + + // .NET fills up to the end of the array before raising, this checks first + if count > 0 && targetIndex + count > target.Length then + raise (System.IndexOutOfRangeException()) + + fillImpl target value targetIndex count + +let getSubArray (array: 'T[]) (start: int) (count: int) : 'T[] = + if start < 0 then + invalidArg "startIndex" SR.inputMustBeNonNegative + + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + + if start + count > array.Length then + raise (System.ArgumentOutOfRangeException("count", "The index is outside the legal range.")) + + subArrayImpl array start count let last (array: 'T[]) = if array.Length = 0 then @@ -338,9 +359,46 @@ let copy (array: 'T[]) = // else copyImpl array +// Not inline: TypeScript accepts the typed array methods only on the obj (any) parameters +let private copyTypedArray (source: obj) (sourceIndex: int) (target: obj) (targetIndex: int) (count: int) : unit = + target?set (source?subarray (sourceIndex, sourceIndex + count), targetIndex) + +let private argumentOutOfRange (paramName: string) (value: int) (requirement: string) = + raise ( + System.ArgumentOutOfRangeException(paramName, paramName + " ('" + string value + "') must be " + requirement) + ) + +// System.Array.Copy calls compile to this function too, so it checks the arguments like +// System.Array.Copy, in the same order, and copies the items itself like Util.copyToArray let copyTo (source: 'T[]) (sourceIndex: int) (target: 'T[]) (targetIndex: int) (count: int) = - // TODO: Check array lengths - System.Array.Copy(source, sourceIndex, target, targetIndex, count) + if count < 0 then + argumentOutOfRange "length" count "a non-negative value." + + if sourceIndex < 0 then + argumentOutOfRange "sourceIndex" sourceIndex "greater than or equal to '0'." + + if sourceIndex + count > source.Length then + invalidArg + "sourceArray" + "Source array was not long enough. Check the source index, length, and the array's lower bounds." + + if targetIndex < 0 then + argumentOutOfRange "destinationIndex" targetIndex "greater than or equal to '0'." + + if targetIndex + count > target.Length then + invalidArg + "destinationArray" + "Destination array was not long enough. Check the destination index, length, and the array's lower bounds." + + if isTypedArrayImpl source && isTypedArrayImpl target then + copyTypedArray (source :> obj) sourceIndex (target :> obj) targetIndex count + elif sourceIndex < targetIndex then + // Backwards, so a copy within one array doesn't overwrite items before reading them + for i = count - 1 downto 0 do + arraySetItemImpl target (targetIndex + i) (arrayAccessImpl (sourceIndex + i) source) + else + for i = 0 to count - 1 do + arraySetItemImpl target (targetIndex + i) (arrayAccessImpl (sourceIndex + i) source) let reverse (array: 'T[]) = // if isTypedArrayImpl array then @@ -788,7 +846,7 @@ let zip3 (array1: 'T[]) (array2: 'U[]) (array3: 'V[]) = let chunkBySize (chunkSize: int) (array: 'T[]) : 'T[][] = if chunkSize < 1 then - invalidArg "size" "The input must be positive." + invalidArg "chunkSize" "The input must be positive." let result = ResizeArray<'T[]>() @@ -1133,7 +1191,7 @@ let windowed (windowSize: int) (source: 'T[]) : 'T[][] = let splitInto (chunks: int) (array: 'T[]) : 'T[][] = if chunks < 1 then - invalidArg "chunks" "The input must be positive." + invalidArg "count" "The input must be positive." let result = ResizeArray<'T[]>() @@ -1354,45 +1412,18 @@ let removeAt (index: int) (xs: 'T[]) : 'T[] = ) let removeManyAt (index: int) (count: int) (xs: 'T[]) : 'T[] = - let mutable i = -1 - // incomplete -1, in-progress 0, complete 1 - let mutable status = -1 + // FSharp.Core's check. Negative counts, which FSharp.Core handles oddly, remove nothing + // but the index can't be past the end. + if index < 0 || index > xs.Length - count || index > xs.Length then + invalidArg "index" "index must be within bounds of the array" - let ys = - xs - |> filter (fun _ -> - i <- i + 1 - - if i = index then - status <- 0 - false - elif i > index then - if i < index + count then - false - else - status <- 1 - true - else - true - ) - - let status = - if status = 0 && i + 1 = index + count then - 1 - else - status - - if status < 1 then - // F# always says the wrong parameter is index but the problem may be count - let arg = - if status < 0 then - "index" - else - "count" - - invalidArg arg SR.indexOutOfBounds + let mutable i = -1 - ys + xs + |> filter (fun _ -> + i <- i + 1 + i < index || i >= index + count + ) let updateAt (index: int) (y: 'T) (xs: 'T[]) ([] cons: Cons<'T>) : 'T[] = let len = xs.Length diff --git a/src/fable-library-ts/List.fs b/src/fable-library-ts/List.fs index 3cf4d74945..5dda298daf 100644 --- a/src/fable-library-ts/List.fs +++ b/src/fable-library-ts/List.fs @@ -176,6 +176,9 @@ let inline private newRoot<'T> () : 'T list = let indexNotFound () = raise (System.Collections.Generic.KeyNotFoundException(SR.keyNotFoundAlt)) +let private differentLengths (paramName: string) = + invalidArg paramName "The lists had different lengths." + let cons (x: 'T) (xs: 'T list) = List.Cons(x, xs) let singleton x = List.Cons(x, empty ()) @@ -288,11 +291,30 @@ let fold2<'T1, 'T2, 'State> (folder: 'State -> 'T1 -> 'T2 -> 'State) (state: 'St xs <- xs.Tail ys <- ys.Tail + // Like FSharp.Core, name the list that ran out + if not xs.IsEmpty then + differentLengths "list2" + elif not ys.IsEmpty then + differentLengths "list1" + acc let foldBack2<'T1, 'T2, 'State> (folder: 'T1 -> 'T2 -> 'State -> 'State) (xs: 'T1 list) (ys: 'T2 list) (state: 'State) = // fold2 (fun acc x y -> folder x y acc) state (reverse xs) (reverse ys) - Array.foldBack2 folder (toArray xs) (toArray ys) state + let xs = toArray xs + let ys = toArray ys + + if xs.Length <> ys.Length then + differentLengths ( + if xs.Length = 0 then + "list1" + elif ys.Length = 0 then + "list2" + else + "list1, list2" + ) + + Array.foldBack2 folder xs ys state let unfold (generator: 'State -> ('T * 'State) option) (state: 'State) = let rec loop acc (node: 'T list) = @@ -451,6 +473,11 @@ let map2 (mapping: 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) = let mapIndexed2 (mapping: int -> 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) = let rec loop i (acc: 'U list) (xs: 'T1 list) (ys: 'T2 list) = if xs.IsEmpty || ys.IsEmpty then + if not xs.IsEmpty then + differentLengths "list2" + elif not ys.IsEmpty then + differentLengths "list1" + acc else let node = acc.AppendConsNoTail(mapping i xs.Head ys.Head) @@ -464,6 +491,9 @@ let mapIndexed2 (mapping: int -> 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) let map3 (mapping: 'T1 -> 'T2 -> 'T3 -> 'U) (xs: 'T1 list) (ys: 'T2 list) (zs: 'T3 list) = let rec loop (acc: 'U list) (xs: 'T1 list) (ys: 'T2 list) (zs: 'T3 list) = if xs.IsEmpty || ys.IsEmpty || zs.IsEmpty then + if not (xs.IsEmpty && ys.IsEmpty && zs.IsEmpty) then + differentLengths "list1, list2, list3" + acc else let node = acc.AppendConsNoTail(mapping xs.Head ys.Head zs.Head) @@ -609,6 +639,9 @@ let contains (value: 'T) (xs: 'T list) ([] eq: System.Collections.Generi tryFindIndex (fun v -> eq.Equals(value, v)) xs |> Option.isSome let initialize n (f: int -> 'T) = + if n < 0 then + invalidArg "count" SR.inputMustBeNonNegative + let root = newRoot () let mutable node = root @@ -634,8 +667,12 @@ let reduceBack f (xs: 'T list) = let forAll f xs = fold (fun acc x -> acc && f x) true xs -let forAll2 f xs ys = - fold2 (fun acc x y -> acc && f x y) true xs ys +let rec forAll2 (f: 'T1 -> 'T2 -> bool) (xs: 'T1 list) (ys: 'T2 list) = + match xs.IsEmpty, ys.IsEmpty with + | true, true -> true + | false, false -> f xs.Head ys.Head && forAll2 f xs.Tail ys.Tail + | true, false -> differentLengths "list1" + | false, true -> differentLengths "list2" let exists f xs = tryFindIndex f xs |> Option.isSome @@ -643,7 +680,7 @@ let rec exists2 (f: 'T1 -> 'T2 -> bool) (xs: 'T1 list) (ys: 'T2 list) = match xs.IsEmpty, ys.IsEmpty with | true, true -> false | false, false -> f xs.Head ys.Head || exists2 f xs.Tail ys.Tail - | _ -> invalidArg "list2" SR.differentLengths + | _ -> differentLengths "list2" let unzip xs = foldBack (fun (x, y) (lacc, racc) -> List.Cons(x, lacc), List.Cons(y, racc)) xs (empty (), empty ()) @@ -726,6 +763,9 @@ let min (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer xs let average (xs: 'T list) ([] averager: IGenericAverager<'T>) : 'T = + if xs.IsEmpty then + invalidArg "source" SR.inputSequenceEmpty + let mutable count = 0 let folder acc x = @@ -736,6 +776,9 @@ let average (xs: 'T list) ([] averager: IGenericAverager<'T>) : 'T = averager.DivideByInt(total, count) let averageBy (f: 'T -> 'U) (xs: 'T list) ([] averager: IGenericAverager<'U>) : 'U = + if xs.IsEmpty then + invalidArg "source" SR.inputSequenceEmpty + let mutable count = 0 let inline folder acc x = @@ -972,43 +1015,22 @@ let removeAt (index: int) (xs: 'T list) : 'T list = ys let removeManyAt (index: int) (count: int) (xs: 'T list) : 'T list = + if index < 0 then + invalidArg "index" "index must be within bounds of the list" + + let endIndex = index + count let mutable i = -1 - // incomplete -1, in-progress 0, complete 1 - let mutable status = -1 let ys = xs |> filter (fun _ -> i <- i + 1 - - if i = index then - status <- 0 - false - elif i > index then - if i < index + count then - false - else - status <- 1 - true - else - true + i < index || i >= endIndex ) - let status = - if status = 0 && i + 1 = index + count then - 1 - else - status - - if status < 1 then - // F# always says the wrong parameter is index but the problem may be count - let arg = - if status < 0 then - "index" - else - "count" - - invalidArg arg SR.indexOutOfBounds + // i + 1 is now the length of the list + if endIndex > i + 1 then + invalidArg "index" "index must be within bounds of the list" ys diff --git a/src/fable-library-ts/Seq.fs b/src/fable-library-ts/Seq.fs index cf14a251fd..c37cf6da0f 100644 --- a/src/fable-library-ts/Seq.fs +++ b/src/fable-library-ts/Seq.fs @@ -13,6 +13,7 @@ type IEnumerable<'T> = System.Collections.Generic.IEnumerable<'T> module SR = let enumerationAlreadyFinished = "Enumeration already finished." let enumerationNotStarted = "Enumeration has not started. Call MoveNext." + let inputMustBePositive = "The input must be positive." let inputSequenceEmpty = "The input sequence was empty." let inputSequenceTooLong = "The input sequence contains more than one element." @@ -689,6 +690,9 @@ let head (xs: seq<'T>) = | None -> invalidArg "source" SR.inputSequenceEmpty let initialize count f = + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + unfold (fun i -> if (i < count) then @@ -1008,7 +1012,12 @@ let reduceBack folder (xs: seq<'T>) = else invalidArg "source" SR.inputSequenceEmpty -let replicate n x = initialize n (fun _ -> x) +let replicate n x = + // FSharp.Core uses Enumerable.Repeat, which raises ArgumentOutOfRangeException + if n < 0 then + raise (System.ArgumentOutOfRangeException("count")) + + initialize n (fun _ -> x) let reverse (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.rev |> ofArray) @@ -1080,6 +1089,9 @@ let tail (xs: seq<'T>) = ) let take count (xs: seq<'T>) = + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + generateIndexed (fun () -> ofSeq xs) (fun i e -> @@ -1128,19 +1140,33 @@ let pairwise (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.pairwise |> ofArray) let splitInto (chunks: int) (xs: seq<'T>) : 'T[] seq = + if chunks < 1 then + invalidArg "count" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.splitInto chunks |> ofArray) let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = + if windowSize < 1 then + invalidArg "windowSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) +// Groups the items by index like FSharp.Core, so jagged input gives shorter rows instead of raising let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> - xss - |> toArray - |> Array.map toArray - |> Array.transpose - |> Array.map ofArray - |> ofArray + let rows = ResizeArray>() + + for xs in xss do + let mutable i = 0 + + for x in xs do + if i = rows.Count then + rows.Add(ResizeArray<'T>()) + + rows.[i].Add(x) + i <- i + 1 + + rows |> map (fun row -> row :> seq<'T>) ) let sortWith (comparer: 'T -> 'T -> int) (xs: seq<'T>) = @@ -1244,6 +1270,9 @@ let permute f (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.permute f |> ofArray) let chunkBySize (chunkSize: int) (xs: seq<'T>) : seq<'T[]> = + if chunkSize < 1 then + invalidArg "chunkSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.chunkBySize chunkSize |> ofArray) // let init = initialize @@ -1346,30 +1375,32 @@ let removeAt (index: int) (xs: seq<'T>) : seq<'T> = ) (fun e -> e.Dispose()) +// Like FSharp.Core: a count past the end (or a negative one) isn't an error, but the input +// must be longer than index, which can only be checked once it has been enumerated let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then - invalidArg "index" SR.indexOutOfBounds + invalidArg "index" "index must be within bounds of the array" - generateIndexed - (fun () -> ofSeq xs) - (fun i e -> + let endIndex = index + count + + delay (fun () -> + let mutable i = -1 + + let rest () = if i < index then - if e.MoveNext() then - Some e.Current - else - invalidArg "index" SR.indexOutOfBounds - else - if i = index then - for _ = 1 to count do - if not (e.MoveNext()) then - invalidArg "count" SR.indexOutOfBounds + invalidArg "index" "index must be within bounds of the array" - if e.MoveNext() then - Some e.Current - else - None - ) - (fun e -> e.Dispose()) + empty () + + let ys = + xs + |> filter (fun _ -> + i <- i + 1 + i < index || i >= endIndex + ) + + append ys (delay rest) + ) let updateAt (index: int) (y: 'T) (xs: seq<'T>) : seq<'T> = let mutable isDone = false diff --git a/tests/Js/Main/ArrayTests.fs b/tests/Js/Main/ArrayTests.fs index 33b77aebc7..dad65d4280 100644 --- a/tests/Js/Main/ArrayTests.fs +++ b/tests/Js/Main/ArrayTests.fs @@ -412,6 +412,14 @@ let tests = Array.blit xs 3 ys 5 4 ys.[5] + ys.[6] + ys.[7] + ys.[8] |> equal "defg" + testCase "Array.blit works with overlapping ranges in the same array" <| fun () -> + let xs = [| "a"; "b"; "c"; "d"; "e" |] + Array.blit xs 0 xs 1 3 + xs |> equal [| "a"; "a"; "b"; "c"; "e" |] + let ys = [| "a"; "b"; "c"; "d"; "e" |] + Array.blit ys 1 ys 0 3 + ys |> equal [| "b"; "c"; "d"; "d"; "e" |] + #if FABLE_COMPILER testCase "Array.blit works with deceiving typed arrays" <| fun () -> let xs = Fable.Core.JsInterop.emitJsExpr () "[1,2,3,4,5,6,7,8,9,10]" @@ -1392,6 +1400,14 @@ let tests = System.Array.Copy(source, 1, destination, 1, 2) equal [| "a"; "xx"; "xyz" |] destination + testCase "System.Array.Copy works with Int64 indexes and length" <| fun () -> + let source = [| "xy"; "xx"; "xyz" |] + let destination = [| "a"; "b"; "c"; "d" |] + System.Array.Copy(source, destination, 3L) + equal [| "xy"; "xx"; "xyz"; "d" |] destination + System.Array.Copy(source, 1L, destination, 2L, 2L) + equal [| "xy"; "xx"; "xx"; "xyz" |] destination + testCase "Array.splitInto works" <| fun () -> [|1..10|] |> Array.splitInto 3 |> equal [| [|1..4|]; [|5..7|]; [|8..10|] |] [|1..11|] |> Array.splitInto 3 |> equal [| [|1..4|]; [|5..8|]; [|9..11|] |] @@ -1772,4 +1788,73 @@ let tests = raisedExceptionType (fun () -> Array.randomChoicesBy (fun () -> 1.0) 2 [|1; 2|]) |> equal "ArgumentOutOfRangeException" raisedExceptionType (fun () -> Array.randomSampleBy (fun () -> -0.1) 1 [|1; 2|]) |> equal "ArgumentOutOfRangeException" + testCase "Array.sub raises FSharp.Core's exception types for out of range arguments" <| fun () -> + let xs = [|1; 2; 3|] + raisedExceptionType (fun () -> Array.sub xs -1 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.sub xs 1 -1) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.sub xs 2 5) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.sub xs 4 0) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.sub [|"a"; "b"; "c"|] 3 1) |> equal "ArgumentOutOfRangeException" + Array.sub xs 3 0 |> equal [||] + + testCase "Array.fill raises FSharp.Core's exception types for out of range arguments" <| fun () -> + let xs = [|1; 2; 3|] + raisedExceptionType (fun () -> Array.fill xs -1 2 0) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.fill xs 0 -1 0) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.fill xs 3 1 0) |> equal "IndexOutOfRangeException" + raisedExceptionType (fun () -> Array.fill [|"a"; "b"; "c"|] 2 5 "z") |> equal "IndexOutOfRangeException" + xs |> equal [|1; 2; 3|] + Array.fill xs 4 0 0 + xs |> equal [|1; 2; 3|] + + testCase "Array.blit raises System.Array.Copy's exception types for out of range arguments" <| fun () -> + let xs = [|1; 2; 3|] + let ys: int[] = Array.zeroCreate 5 + raisedExceptionType (fun () -> Array.blit xs 0 ys 0 -1) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit xs -1 ys 0 2) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit xs 0 ys -1 2) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit xs 2 ys 0 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.blit xs 0 ys 4 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.blit xs 4 ys 0 0) |> equal "ArgumentException" + ys |> equal [|0; 0; 0; 0; 0|] + let zs = [|"a"; "b"; "c"|] + let ws = Array.create 5 "_" + raisedExceptionType (fun () -> Array.blit zs 0 ws 0 -1) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit zs -1 ws 0 2) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit zs 0 ws -1 2) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit zs 2 ws 0 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.blit zs 0 ws 4 2) |> equal "ArgumentException" + ws |> equal [|"_"; "_"; "_"; "_"; "_"|] + Array.blit zs 3 ws 5 0 + Array.blit zs 1 ws 3 2 + ws |> equal [|"_"; "_"; "_"; "b"; "c"|] + + testCase "System.Array.Copy raises .NET's exception types for out of range arguments" <| fun () -> + let xs = [|1; 2; 3|] + let ys: int[] = Array.zeroCreate 5 + raisedExceptionType (fun () -> System.Array.Copy(xs, 0, ys, 0, -1)) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> System.Array.Copy(xs, -1, ys, 0, 2)) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> System.Array.Copy(xs, 0, ys, -1, 2)) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> System.Array.Copy(xs, 2, ys, 0, 2)) |> equal "ArgumentException" + raisedExceptionType (fun () -> System.Array.Copy(xs, 0, ys, 4, 2)) |> equal "ArgumentException" + raisedExceptionType (fun () -> System.Array.Copy(xs, ys, 4)) |> equal "ArgumentException" + raisedExceptionType (fun () -> System.Array.Copy([|"a"; "b"|], [|"c"|], 2)) |> equal "ArgumentException" + ys |> equal [|0; 0; 0; 0; 0|] + + testCase "Array.removeManyAt raises ArgumentException only when FSharp.Core does" <| fun () -> + let xs = [|1; 2; 3|] + let empty: int[] = [||] + raisedExceptionType (fun () -> Array.removeManyAt -1 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt -1 1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt 1 3 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt 3 1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt 4 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt 0 1 empty) |> equal "ArgumentException" + Array.removeManyAt 0 0 xs |> equal [|1; 2; 3|] + Array.removeManyAt 1 0 xs |> equal [|1; 2; 3|] + Array.removeManyAt 3 0 xs |> equal [|1; 2; 3|] + Array.removeManyAt 0 0 empty |> equal [||] + Array.removeManyAt 1 2 xs |> equal [|1|] + Array.removeManyAt 0 2 [|"a"; "b"; "c"|] |> equal [|"c"|] + ] diff --git a/tests/Js/Main/ListTests.fs b/tests/Js/Main/ListTests.fs index 4df3113faf..8082564c81 100644 --- a/tests/Js/Main/ListTests.fs +++ b/tests/Js/Main/ListTests.fs @@ -1112,4 +1112,63 @@ let tests = raisedExceptionType (fun () -> List.randomChoiceBy (fun () -> 1.0) [1; 2]) |> equal "ArgumentOutOfRangeException" raisedExceptionType (fun () -> List.randomChoicesBy (fun () -> 1.0) 2 [1; 2]) |> equal "ArgumentOutOfRangeException" raisedExceptionType (fun () -> List.randomSampleBy (fun () -> -0.1) 1 [1; 2]) |> equal "ArgumentOutOfRangeException" + + testCase "List functions raise ArgumentException for lists of different lengths" <| fun () -> + let xs = [1; 2] + let ys = [1; 2; 3] + let empty: int list = [] + raisedExceptionType (fun () -> List.fold2 (fun acc x y -> acc + x + y) 0 xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.fold2 (fun acc x y -> acc + x + y) 0 ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.foldBack2 (fun x y acc -> x + y + acc) empty xs 0) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map2 (+) xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.mapi2 (fun i x y -> i + x + y) ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map3 (fun x y z -> x + y + z) xs ys ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map3 (fun x y z -> x + y + z) ys ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip empty xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip xs empty) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip3 xs ys ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.iter2 (fun _ _ -> ()) xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.iteri2 (fun _ _ _ -> ()) ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.forall2 (fun x y -> x = y) xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.forall2 (fun x y -> x = y) empty xs) |> equal "ArgumentException" + + testCase "List functions call the function on the common items of lists of different lengths before raising" <| fun () -> + let mutable calls = 0 + raisedExceptionType (fun () -> List.iter2 (fun _ _ -> calls <- calls + 1) [1; 2] [1; 2; 3]) |> equal "ArgumentException" + equal 2 calls + raisedExceptionType (fun () -> List.map3 (fun x _ _ -> calls <- calls + 1; x) [1; 2; 3] [1; 2] [1; 2; 3]) |> equal "ArgumentException" + equal 4 calls + + testCase "List.forall2 returns false before reaching the end of lists of different lengths" <| fun () -> + List.forall2 (fun x y -> x + y < 3) [1; 2] [1; 2; 3] |> equal false + List.forall2 (fun _ _ -> false) [1; 2; 3] [1; 2] |> equal false + + testCase "List.init and List.replicate raise ArgumentException for negative counts" <| fun () -> + raisedExceptionType (fun () -> List.init -1 id) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.replicate -1 "a") |> equal "ArgumentException" + List.init 0 id |> equal [] + + testCase "List.average and List.averageBy raise ArgumentException on empty lists" <| fun () -> + let xs: float list = [] + let ys: decimal list = [] + raisedExceptionType (fun () -> List.average xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.averageBy (fun x -> x * 2.) xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.average ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.averageBy id ys) |> equal "ArgumentException" + + testCase "List.removeManyAt raises ArgumentException only when FSharp.Core does" <| fun () -> + let xs = [1; 2; 3] + let empty: int list = [] + raisedExceptionType (fun () -> List.removeManyAt -1 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt -1 1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 2 2 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 3 1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 4 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 0 1 empty) |> equal "ArgumentException" + List.removeManyAt 0 0 xs |> equal [1; 2; 3] + List.removeManyAt 1 0 xs |> equal [1; 2; 3] + List.removeManyAt 3 0 xs |> equal [1; 2; 3] + List.removeManyAt 1 -1 xs |> equal [1; 2; 3] + List.removeManyAt 0 0 empty |> equal [] + List.removeManyAt 1 2 xs |> equal [1] ] diff --git a/tests/Js/Main/SeqTests.fs b/tests/Js/Main/SeqTests.fs index fc19f494ba..418eec38c9 100644 --- a/tests/Js/Main/SeqTests.fs +++ b/tests/Js/Main/SeqTests.fs @@ -1190,4 +1190,41 @@ let tests = raisedExceptionType (fun () -> Seq.permute (fun _ -> 0) [1; 2] |> Seq.toList) |> equal "ArgumentException" raisedExceptionType (fun () -> Seq.randomChoiceBy (fun () -> 1.0) [1; 2]) |> equal "ArgumentOutOfRangeException" raisedExceptionType (fun () -> Seq.randomShuffleBy (fun () -> 1.0) [1; 2] |> Seq.toList) |> equal "ArgumentOutOfRangeException" + + testCase "Seq functions check their arguments when called, before enumerating" <| fun () -> + let xs = seq { for x in [1; 2; 3] do yield x } + raisedExceptionType (fun () -> Seq.init -1 id) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.replicate -1 "a") |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Seq.take -1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.take -1 [1; 2]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.chunkBySize 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.windowed 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.splitInto 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt -1 0 xs) |> equal "ArgumentException" + Seq.init 0 id |> Seq.toList |> equal [] + Seq.take 0 xs |> Seq.toList |> equal [] + + testCase "Seq.removeManyAt raises ArgumentException only when FSharp.Core does" <| fun () -> + let xs = seq { for x in [1; 2; 3] do yield x } + let empty: int seq = Seq.empty + raisedExceptionType (fun () -> Seq.removeManyAt 3 0 xs |> Seq.toList) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 3 0 [|1; 2; 3|] |> Seq.toList) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 4 1 xs |> Seq.toList) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 0 0 empty |> Seq.toList) |> equal "ArgumentException" + Seq.removeManyAt 0 4 xs |> Seq.toList |> equal [] + Seq.removeManyAt 1 3 xs |> Seq.toList |> equal [1] + Seq.removeManyAt 1 0 xs |> Seq.toList |> equal [1; 2; 3] + Seq.removeManyAt 1 -1 [1; 2; 3] |> Seq.toList |> equal [1; 2; 3] + Seq.removeManyAt 2 2 [|1; 2; 3|] |> Seq.toList |> equal [1; 2] + + testCase "Seq.transpose groups the items of jagged sequences by index" <| fun () -> + let transpose (xss: seq>) = Seq.transpose xss |> Seq.map Seq.toList |> Seq.toList + transpose [seq [1; 2]; seq [3]] |> equal [[1; 3]; [2]] + transpose [seq [1]; seq [2; 3]] |> equal [[1; 2]; [3]] + transpose [Seq.empty; seq [1]] |> equal [[1]] + transpose [seq [1]; Seq.empty] |> equal [[1]] + transpose [seq [1; 2; 3]; seq [4]; seq [5; 6]] |> equal [[1; 4; 5]; [2; 6]; [3]] + transpose (seq { for xs in [[1; 2]; [3]] do yield seq { for x in xs do yield x } }) |> equal [[1; 3]; [2]] + Seq.transpose [[1; 2]; [3]] |> Seq.map Seq.toList |> Seq.toList |> equal [[1; 3]; [2]] + Seq.transpose [| [|1|]; [|2; 3|] |] |> Seq.map Seq.toList |> Seq.toList |> equal [[1; 2]; [3]] ] diff --git a/tests/Js/Main/Util/Util.fs b/tests/Js/Main/Util/Util.fs index e0c24a8220..647f6c0c39 100644 --- a/tests/Js/Main/Util/Util.fs +++ b/tests/Js/Main/Util/Util.fs @@ -119,6 +119,7 @@ let raisedExceptionType (f: unit -> 'a): string = | :? InvalidOperationException -> "InvalidOperationException" | :? Collections.Generic.KeyNotFoundException -> "KeyNotFoundException" | :? NotSupportedException -> "NotSupportedException" + | :? IndexOutOfRangeException -> "IndexOutOfRangeException" | e -> "Unexpected exception: " + e.Message let rec sumFirstSeq (zs: seq) (n: int): float = From bb1131e9bf84d384814892011f78796ac78a3c59 Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Sun, 25 Oct 2026 08:18:13 -0500 Subject: [PATCH 2/5] fix(python): add FSharp.Core's argument checks to collection functions Several List and Seq functions returned a value, often a wrong one, where FSharp.Core rejects the argument, and a few raised where FSharp.Core does not. They now behave like FSharp.Core: - List.fold2, map2, mapi2, map3, zip, zip3, iter2, iteri2 and forall2 raise ArgumentException when the lists have different lengths, after calling back on the common prefix. forall2 still stops at the first false. List.foldBack2, which raised the Rust core's ValueError, checks the lengths before calling back. Every List length check, exists2's included, now uses FSharp.Core's message. - List.init, List.replicate, Seq.init and Seq.take raise ArgumentException for a negative count, Seq.replicate ArgumentOutOfRangeException. The Seq checks run when the function is called. - List.average and List.averageBy raise ArgumentException on an empty list. - List.removeManyAt raises exactly when FSharp.Core does and removes nothing for a count <= 0. Seq.removeManyAt raises after enumeration only when the source is too short for index, and no longer when the count runs past the end. Both use FSharp.Core's messages. - Seq.chunkBySize, Seq.windowed and Seq.splitInto check the size when called, with FSharp.Core's parameter names. - Seq.transpose groups jagged input by index like FSharp.Core instead of raising through Array.transpose. --- src/fable-library-py/fable_library/List.fs | 88 ++++++++++++------- src/fable-library-py/fable_library/Seq.fs | 85 ++++++++++++------ .../fable_library/seq_native.py | 4 + tests/Python/TestList.fs | 70 +++++++++++++++ tests/Python/TestSeq.fs | 52 +++++++++++ 5 files changed, 239 insertions(+), 60 deletions(-) diff --git a/src/fable-library-py/fable_library/List.fs b/src/fable-library-py/fable_library/List.fs index 9859354847..63a973162c 100644 --- a/src/fable-library-py/fable_library/List.fs +++ b/src/fable-library-py/fable_library/List.fs @@ -167,6 +167,9 @@ and List<'T> = LinkedList<'T> let indexNotFound () = raise (System.Collections.Generic.KeyNotFoundException(SR.keyNotFoundAlt)) +let private differentLengths (paramName: string) = + invalidArg paramName "The lists had different lengths." + let empty () = List.Empty let cons (x: 'T) (xs: 'T list) = List.Cons(x, xs) @@ -281,11 +284,30 @@ let fold2<'T1, 'T2, 'State> (folder: 'State -> 'T1 -> 'T2 -> 'State) (state: 'St xs <- xs.Tail ys <- ys.Tail + // FSharp.Core names the list that ran out first + if not xs.IsEmpty then + differentLengths "list2" + elif not ys.IsEmpty then + differentLengths "list1" + acc let foldBack2<'T1, 'T2, 'State> (folder: 'T1 -> 'T2 -> 'State -> 'State) (xs: 'T1 list) (ys: 'T2 list) (state: 'State) = // fold2 (fun acc x y -> folder x y acc) state (reverse xs) (reverse ys) - Array.foldBack2 folder (toArray xs) (toArray ys) state + let xs = toArray xs + let ys = toArray ys + + if xs.Length <> ys.Length then + differentLengths ( + if xs.Length = 0 then + "list1" + elif ys.Length = 0 then + "list2" + else + "list1, list2" + ) + + Array.foldBack2 folder xs ys state let unfold (generator: 'State -> ('T * 'State) option) (state: 'State) = let rec loop acc (node: 'T list) = @@ -431,6 +453,11 @@ let map2 (mapping: 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) = let mapIndexed2 (mapping: int -> 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) = let rec loop i (acc: 'U list) (xs: 'T1 list) (ys: 'T2 list) = if xs.IsEmpty || ys.IsEmpty then + if not xs.IsEmpty then + differentLengths "list2" + elif not ys.IsEmpty then + differentLengths "list1" + acc else let node = acc.AppendConsNoTail(mapping i xs.Head ys.Head) @@ -444,6 +471,9 @@ let mapIndexed2 (mapping: int -> 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) let map3 (mapping: 'T1 -> 'T2 -> 'T3 -> 'U) (xs: 'T1 list) (ys: 'T2 list) (zs: 'T3 list) = let rec loop (acc: 'U list) (xs: 'T1 list) (ys: 'T2 list) (zs: 'T3 list) = if xs.IsEmpty || ys.IsEmpty || zs.IsEmpty then + if not (xs.IsEmpty && ys.IsEmpty && zs.IsEmpty) then + differentLengths "list1, list2, list3" + acc else let node = acc.AppendConsNoTail(mapping xs.Head ys.Head zs.Head) @@ -589,6 +619,9 @@ let contains (value: 'T) (xs: 'T list) ([] eq: System.Collections.Generi tryFindIndex (fun v -> eq.Equals(value, v)) xs |> Option.isSome let initialize n (f: int -> 'T) = + if n < 0 then + invalidArg "count" SR.inputMustBeNonNegative + let root = List.Empty let mutable node = root @@ -614,8 +647,12 @@ let reduceBack f (xs: 'T list) = let forAll f xs = fold (fun acc x -> acc && f x) true xs -let forAll2 f xs ys = - fold2 (fun acc x y -> acc && f x y) true xs ys +let rec forAll2 (f: 'T1 -> 'T2 -> bool) (xs: 'T1 list) (ys: 'T2 list) = + match xs.IsEmpty, ys.IsEmpty with + | true, true -> true + | false, false -> f xs.Head ys.Head && forAll2 f xs.Tail ys.Tail + | true, false -> differentLengths "list1" + | false, true -> differentLengths "list2" let exists f xs = tryFindIndex f xs |> Option.isSome @@ -623,7 +660,7 @@ let rec exists2 (f: 'T1 -> 'T2 -> bool) (xs: 'T1 list) (ys: 'T2 list) = match xs.IsEmpty, ys.IsEmpty with | true, true -> false | false, false -> f xs.Head ys.Head || exists2 f xs.Tail ys.Tail - | _ -> invalidArg "list2" SR.differentLengths + | _ -> differentLengths "list2" let unzip xs = foldBack (fun (x, y) (lacc, racc) -> List.Cons(x, lacc), List.Cons(y, racc)) xs (List.Empty, List.Empty) @@ -706,6 +743,9 @@ let min (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer xs let average (xs: 'T list) ([] averager: IGenericAverager<'T>) : 'T = + if xs.IsEmpty then + invalidArg "source" SR.inputSequenceEmpty + let mutable count = 0 let folder acc x = @@ -716,6 +756,9 @@ let average (xs: 'T list) ([] averager: IGenericAverager<'T>) : 'T = averager.DivideByInt(total, count) let averageBy (f: 'T -> 'U) (xs: 'T list) ([] averager: IGenericAverager<'U>) : 'U = + if xs.IsEmpty then + invalidArg "source" SR.inputSequenceEmpty + let mutable count = 0 let inline folder acc x = @@ -952,43 +995,22 @@ let removeAt (index: int) (xs: 'T list) : 'T list = ys let removeManyAt (index: int) (count: int) (xs: 'T list) : 'T list = + if index < 0 then + invalidArg "index" "index must be within bounds of the list" + + let endIndex = index + count let mutable i = -1 - // incomplete -1, in-progress 0, complete 1 - let mutable status = -1 let ys = xs |> filter (fun _ -> i <- i + 1 - - if i = index then - status <- 0 - false - elif i > index then - if i < index + count then - false - else - status <- 1 - true - else - true + i < index || i >= endIndex ) - let status = - if status = 0 && i + 1 = index + count then - 1 - else - status - - if status < 1 then - // F# always says the wrong parameter is index but the problem may be count - let arg = - if status < 0 then - "index" - else - "count" - - invalidArg arg SR.indexOutOfBounds + // i + 1 is now the length of the list + if endIndex > i + 1 then + invalidArg "index" "index must be within bounds of the list" ys diff --git a/src/fable-library-py/fable_library/Seq.fs b/src/fable-library-py/fable_library/Seq.fs index e15b92cd11..7693beed62 100644 --- a/src/fable-library-py/fable_library/Seq.fs +++ b/src/fable-library-py/fable_library/Seq.fs @@ -14,6 +14,7 @@ type IEnumerable<'T> = System.Collections.Generic.IEnumerable<'T> module SR = let enumerationAlreadyFinished = "Enumeration already finished." let enumerationNotStarted = "Enumeration has not started. Call MoveNext." + let inputMustBePositive = "The input must be positive." let inputSequenceEmpty = "The input sequence was empty." let inputSequenceTooLong = "The input sequence contains more than one element." @@ -650,6 +651,9 @@ let forAll2 predicate xs ys = // `tryHead`/`head`: no F# impl - Replacements.fs redirects both straight to `seq_native`. let initialize count f = + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + unfold (fun i -> if (i < count) then @@ -890,7 +894,12 @@ let reduceBack folder (xs: seq<'T>) = else invalidArg "source" SR.inputSequenceEmpty -let replicate n x = initialize n (fun _ -> x) +let replicate n x = + // FSharp.Core uses Enumerable.Repeat, which raises ArgumentOutOfRangeException + if n < 0 then + raise (System.ArgumentOutOfRangeException("count")) + + initialize n (fun _ -> x) let reverse (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.rev |> ofArray) @@ -975,19 +984,33 @@ let pairwise (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.pairwise |> ofArray) let splitInto (chunks: int) (xs: seq<'T>) : 'T[] seq = + if chunks < 1 then + invalidArg "count" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.splitInto chunks |> ofArray) let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = + if windowSize < 1 then + invalidArg "windowSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) +// Groups the items by index like FSharp.Core, so jagged input gives shorter rows instead of raising let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> - xss - |> toArray - |> Array.map toArray - |> Array.transpose - |> Array.map ofArray - |> ofArray + let rows = ResizeArray>() + + for xs in xss do + let mutable i = 0 + + for x in xs do + if i = rows.Count then + rows.Add(ResizeArray<'T>()) + + rows.[i].Add(x) + i <- i + 1 + + rows |> map (fun row -> row :> seq<'T>) ) let sortWith (comparer: 'T -> 'T -> int) (xs: seq<'T>) = @@ -1091,6 +1114,9 @@ let permute f (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.permute f |> ofArray) let chunkBySize (chunkSize: int) (xs: seq<'T>) : seq<'T[]> = + if chunkSize < 1 then + invalidArg "chunkSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.chunkBySize chunkSize |> ofArray) // let init = initialize @@ -1195,28 +1221,33 @@ let removeAt (index: int) (xs: seq<'T>) : seq<'T> = let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then - invalidArg "index" SR.indexOutOfBounds + invalidArg "index" "index must be within bounds of the array" - generateIndexed - (fun () -> ofSeq xs) - (fun i e -> - if i < index then - if e.MoveNext() then - Some e.Current - else - invalidArg "index" SR.indexOutOfBounds - else - if i = index then - for _ = 1 to count do - if not (e.MoveNext()) then - invalidArg "count" SR.indexOutOfBounds + // Like FSharp.Core, count <= 0 removes nothing, and a source too short for index + // raises only once it has been enumerated + let endIndex = index + count - if e.MoveNext() then - Some e.Current - else - None - ) - (fun e -> e.Dispose()) + mkSeq (fun () -> + let mutable i = -1 + + Enumerator.generateWhileSome + (fun () -> ofSeq xs) + (fun e -> + let mutable res = None + + while Option.isNone res && e.MoveNext() do + i <- i + 1 + + if i < index || i >= endIndex then + res <- Some e.Current + + if Option.isNone res && i < index then + invalidArg "index" "index must be within bounds of the array" + + res + ) + (fun e -> e.Dispose()) + ) let updateAt (index: int) (y: 'T) (xs: seq<'T>) : seq<'T> = let mutable isDone = false diff --git a/src/fable-library-py/fable_library/seq_native.py b/src/fable-library-py/fable_library/seq_native.py index f9a9750ce4..c9a3bae59e 100644 --- a/src/fable-library-py/fable_library/seq_native.py +++ b/src/fable-library-py/fable_library/seq_native.py @@ -161,6 +161,10 @@ def gen() -> Iterator[T]: def take[T](count: int, xs: IEnumerable_1[T]) -> NativeSeq[T]: + # Checked when called, not when enumerated, like FSharp.Core + if count < 0: + raise ArgumentException__ctor_Z384F8060("The input must be non-negative.", "count") + def gen() -> Iterator[T]: it = _source_iter(xs) for _ in range(count): diff --git a/tests/Python/TestList.fs b/tests/Python/TestList.fs index c7db8d1859..c8631a6af8 100644 --- a/tests/Python/TestList.fs +++ b/tests/Python/TestList.fs @@ -1286,3 +1286,73 @@ let ``test List.take and List.splitAt raise InvalidOperationException when the l [] let ``test List.skip raises ArgumentOutOfRangeException when the list is too short`` () = raisedExceptionType (fun () -> List.skip 5 [1]) |> equal "ArgumentOutOfRangeException" + +[] +let ``test List functions walking two lists raise ArgumentException when the lengths differ`` () = + let xs = [1; 2] + let ys = [1; 2; 3] + raisedExceptionType (fun () -> List.fold2 (fun acc x y -> acc + x + y) 0 xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.fold2 (fun acc x y -> acc + x + y) 0 ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.foldBack2 (fun x y acc -> x + y + acc) [1] [1; 2] 0) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map2 (+) xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.mapi2 (fun i x y -> i + x + y) ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.iter2 (fun _ _ -> ()) [] [1]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.iteri2 (fun _ _ _ -> ()) [1] []) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip ([]: int list) [1]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.forall2 (fun _ _ -> true) xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.forall2 (fun _ _ -> false) [1] []) |> equal "ArgumentException" + +[] +let ``test List.map3 and List.zip3 raise ArgumentException when the lengths differ`` () = + let add3 x y z = x + y + z + raisedExceptionType (fun () -> List.map3 add3 [1; 2] [1; 2; 3] [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map3 add3 [1; 2; 3] [1; 2; 3] [1; 2]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip3 [1] [] [1]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip3 [1; 2] [1; 2] [1; 2; 3]) |> equal "ArgumentException" + +[] +let ``test List functions walking lists in step call back on the common prefix before raising`` () = + let calls = ResizeArray() + raisedExceptionType (fun () -> List.iter2 (fun x _ -> calls.Add x) [1; 2] [1; 2; 3]) + |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map3 (fun x _ _ -> calls.Add x) [3; 4; 5] [1; 2] [1; 2; 3]) + |> equal "ArgumentException" + List.ofSeq calls |> equal [1; 2; 3; 4] + +[] +let ``test List.forall2 stops at the first false before checking the lengths`` () = + List.forall2 (fun _ _ -> false) [1; 2] [1; 2; 3] |> equal false + List.forall2 (fun x _ -> x < 2) [1; 2; 3] [1; 2] |> equal false + List.forall2 (fun x y -> x = y) [1; 2] [1; 2] |> equal true + +[] +let ``test List.init and List.replicate raise ArgumentException for a negative count`` () = + raisedExceptionType (fun () -> List.init -1 id) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.replicate -1 "a") |> equal "ArgumentException" + List.init 0 id |> equal ([]: int list) + List.replicate 0 "a" |> equal [] + +[] +let ``test List.average and List.averageBy raise ArgumentException on empty input`` () = + let xs: float list = [] + raisedExceptionType (fun () -> List.average xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.averageBy (fun x -> x * 2.) xs) |> equal "ArgumentException" + +[] +let ``test List.removeManyAt raises ArgumentException where FSharp.Core does`` () = + let xs = [1; 2; 3] + raisedExceptionType (fun () -> List.removeManyAt -1 1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt -1 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 2 2 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 4 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 0 1 ([]: int list)) |> equal "ArgumentException" + +[] +let ``test List.removeManyAt works and removes nothing for a count that is not positive`` () = + let xs = [1; 2; 3] + List.removeManyAt 1 2 xs |> equal [1] + List.removeManyAt 1 0 xs |> equal [1; 2; 3] + List.removeManyAt 1 -1 xs |> equal [1; 2; 3] + List.removeManyAt 3 0 xs |> equal [1; 2; 3] + List.removeManyAt 0 0 ([]: int list) |> equal [] diff --git a/tests/Python/TestSeq.fs b/tests/Python/TestSeq.fs index 78314876de..ecacf82993 100644 --- a/tests/Python/TestSeq.fs +++ b/tests/Python/TestSeq.fs @@ -1358,3 +1358,55 @@ let ``test Seq.head and Seq.item raise ArgumentException when the sequence is to raisedExceptionType (fun () -> Seq.item 5 [1]) |> equal "ArgumentException" raisedExceptionType (fun () -> Seq.item 5 [|1|]) |> equal "ArgumentException" raisedExceptionType (fun () -> Seq.item -1 (seq { 1 })) |> equal "ArgumentException" + +[] +let ``test Seq.init, Seq.replicate and Seq.take check for a negative count when called`` () = + raisedExceptionType (fun () -> Seq.init -1 id) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.replicate -1 "a") |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Seq.take -1 (seq { 1; 2 })) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.take -1 [1; 2]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.take -1 [|1; 2|]) |> equal "ArgumentException" + Seq.init 0 id |> Seq.toList |> equal [] + Seq.replicate 0 "a" |> Seq.toList |> equal [] + Seq.take 0 (seq { 1; 2 }) |> Seq.toList |> equal [] + +[] +let ``test Seq.chunkBySize, Seq.windowed and Seq.splitInto check the size when called`` () = + let xs = seq { 1; 2; 3 } + raisedExceptionType (fun () -> Seq.chunkBySize 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.windowed 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.splitInto 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.chunkBySize -1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.windowed -1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.splitInto -1 xs) |> equal "ArgumentException" + +[] +let ``test Seq.transpose groups jagged input by index`` () = + let toLists (xss: seq>) = xss |> Seq.map Seq.toList |> Seq.toList + Seq.transpose [[1; 2]; [3]] |> toLists |> equal [[1; 3]; [2]] + Seq.transpose [[1]; [2; 3]] |> toLists |> equal [[1; 2]; [3]] + Seq.transpose [[]; [1]] |> toLists |> equal [[1]] + Seq.transpose [[1]; []] |> toLists |> equal [[1]] + Seq.transpose [[1; 2; 3]; [4]; [5; 6]] |> toLists |> equal [[1; 4; 5]; [2; 6]; [3]] + Seq.transpose (seq [seq { 1; 2 }; seq { 3 }]) |> toLists |> equal [[1; 3]; [2]] + Seq.transpose [|[|1; 2|]; [|3|]|] |> toLists |> equal [[1; 3]; [2]] + +[] +let ``test Seq.removeManyAt raises ArgumentException where FSharp.Core does`` () = + let xs = seq { 1; 2; 3 } + raisedExceptionType (fun () -> Seq.removeManyAt -1 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 0 0 Seq.empty |> Seq.toList) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 3 0 [1; 2; 3] |> Seq.toList) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 3 0 [|1; 2; 3|] |> Seq.toList) |> equal "ArgumentException" + let yielded = ResizeArray() + raisedExceptionType (fun () -> Seq.removeManyAt 4 1 xs |> Seq.iter yielded.Add) |> equal "ArgumentException" + List.ofSeq yielded |> equal [1; 2; 3] + +[] +let ``test Seq.removeManyAt does not raise when the count runs past the end or is not positive`` () = + let xs = seq { 1; 2; 3 } + Seq.removeManyAt 1 3 xs |> Seq.toList |> equal [1] + Seq.removeManyAt 0 4 xs |> Seq.toList |> equal [] + Seq.removeManyAt 2 2 [|1; 2; 3|] |> Seq.toList |> equal [1; 2] + Seq.removeManyAt 1 0 xs |> Seq.toList |> equal [1; 2; 3] + Seq.removeManyAt 1 -1 [1; 2; 3] |> Seq.toList |> equal [1; 2; 3] From c0d83ea1c2a3df4c9630f7665d5e9f63f4779328 Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Sun, 25 Oct 2026 08:22:59 -0500 Subject: [PATCH 3/5] fix(dart): add FSharp.Core's argument checks to collection functions Collection functions returned a value, often a wrong one, where FSharp.Core rejects the arguments, and a few raised where FSharp.Core does not. They now follow FSharp.Core, with its exception types and ParamNames: - List.fold2, map2, mapi2, map3, zip, zip3, iter2, iteri2 and forall2 raise ArgumentException when the lists have different lengths, after calling the function on the common prefix. forall2 still returns false before reaching the end. List.foldBack2 names the lists like FSharp.Core, and every List length check says "The lists had different lengths.". - List.init and List.replicate with a negative count, and List.average and List.averageBy on an empty list, raise ArgumentException. - Seq.init, Seq.take, Seq.chunkBySize, Seq.windowed and Seq.splitInto check their argument when called, and Seq.replicate raises ArgumentOutOfRangeException, instead of returning an empty sequence or failing on enumeration. Array.chunkBySize and Array.splitInto use FSharp.Core's ParamNames. - Array.sub, Array.fill, Array.blit and System.Array.Copy check their indexes and counts up front, and Array.fill allows a count of 0 past the end. Array.sub gets its own library function so ResizeArray.GetRange keeps using getSubArray. - List, Array and Seq.removeManyAt raise ArgumentException with FSharp.Core's messages when FSharp.Core does (Seq also when the sequence is not longer than index), and a count of 0 or less removes nothing (on arrays FSharp.Core copies odd ranges or fails in System.Array.Copy). Seq.removeManyAt allows a count past the end. - Seq.transpose groups the elements by index like FSharp.Core instead of raising for sequences of different lengths. --- src/Fable.Transforms/Dart/Replacements.fs | 4 + src/fable-library-dart/Array.fs | 101 +++++++++++++--------- src/fable-library-dart/Global.fs | 1 + src/fable-library-dart/List.fs | 88 ++++++++++++------- src/fable-library-dart/Seq.fs | 82 ++++++++++++------ tests/Dart/src/ArrayTests.fs | 42 +++++++++ tests/Dart/src/ListTests.fs | 48 ++++++++++ tests/Dart/src/SeqTests.fs | 38 ++++++++ tests/Dart/src/Tests.Util.fs | 1 + 9 files changed, 307 insertions(+), 98 deletions(-) diff --git a/src/Fable.Transforms/Dart/Replacements.fs b/src/Fable.Transforms/Dart/Replacements.fs index 66b24c4b58..bd6774959d 100644 --- a/src/Fable.Transforms/Dart/Replacements.fs +++ b/src/Fable.Transforms/Dart/Replacements.fs @@ -1877,6 +1877,10 @@ let arrayModule (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (_: Ex | "IsEmpty", [ ar ] -> getFieldWith r t ar "isEmpty" |> Some | "Contains", [ value; ar ] -> Helper.LibCall(com, "Array", "contains", t, [ value; ar ], ?loc = r) |> Some | "CopyTo", args -> copyToArray com r t i args + // Not getSubArray, which ResizeArray.GetRange also uses: Array.sub checks its arguments like FSharp.Core + | "GetSubArray", args -> + Helper.LibCall(com, "Array", "sub", t, args, i.SignatureArgTypes, genArgs = i.GenericArgs, ?loc = r) + |> Some | ("Distinct" | "DistinctBy" | "Except" | "GroupBy" | "CountBy" as meth), args -> let meth = Naming.lowerFirst meth let args = injectArg com ctx r "Seq2" meth i.GenericArgs args diff --git a/src/fable-library-dart/Array.fs b/src/fable-library-dart/Array.fs index db4e3746e8..d38bda594d 100644 --- a/src/fable-library-dart/Array.fs +++ b/src/fable-library-dart/Array.fs @@ -143,12 +143,38 @@ let filter (predicate: 'T -> bool) (array: 'T[]) = Native.where predicate array // intentionally returns target instead of unit let fill (target: 'T[]) (targetIndex: int) (count: int) (value: 'T) : 'T[] = - Native.fillRange target targetIndex (targetIndex + count) value + if targetIndex < 0 then + invalidArg "targetIndex" SR.inputMustBeNonNegative + + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + + // FSharp.Core sets the items one by one: a count of 0 is fine even past the end, and + // it fills up to the end before raising, where this checks first + if count > 0 then + if targetIndex + count > target.Length then + raise (System.IndexOutOfRangeException(SR.Arg_IndexOutOfRangeException)) + + Native.fillRange target targetIndex (targetIndex + count) value + target let getSubArray (array: 'T[]) (start: int) (count: int) : 'T[] = Native.sublist (array, start, start + count) +// Array.sub checks its arguments like FSharp.Core (unlike getSubArray, which ResizeArray.GetRange also uses) +let sub (array: 'T[]) (startIndex: int) (count: int) : 'T[] = + if startIndex < 0 then + invalidArg "startIndex" SR.inputMustBeNonNegative + + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + + if startIndex + count > array.Length then + raise (System.ArgumentOutOfRangeException("count", "The index is outside the legal range.")) + + getSubArray array startIndex count + let last (array: 'T[]) = if Array.isEmpty array then invalidArg "array" LanguagePrimitives.ErrorStrings.InputArrayEmptyString @@ -400,8 +426,27 @@ let removeAllInPlace (predicate: 'T -> bool) (array: 'T[]) : int = Native.removeWhere array predicate len - array.Length -// TODO: Check array lengths +// Array.blit and System.Array.Copy: checks the arguments in the same order as System.Array.Copy let copyTo (source: 'T[]) (sourceIndex: int) (target: 'T[]) (targetIndex: int) (count: int) : unit = + if count < 0 then + raise (System.ArgumentOutOfRangeException("length", SR.Arg_ArgumentOutOfRangeException)) + + if sourceIndex < 0 then + raise (System.ArgumentOutOfRangeException("sourceIndex", SR.Arg_ArgumentOutOfRangeException)) + + if sourceIndex + count > source.Length then + invalidArg + "sourceArray" + "Source array was not long enough. Check the source index, length, and the array's lower bounds." + + if targetIndex < 0 then + raise (System.ArgumentOutOfRangeException("destinationIndex", SR.Arg_ArgumentOutOfRangeException)) + + if targetIndex + count > target.Length then + invalidArg + "destinationArray" + "Destination array was not long enough. Check the destination index, length, and the array's lower bounds." + Native.copyRange (target, targetIndex, source, sourceIndex, sourceIndex + count) let indexOf (array: 'T[]) (item: 'T) (start: int option) (count: int option) : int = @@ -739,7 +784,7 @@ let zip3 (array1: 'T1[]) (array2: 'T2[]) (array3: 'T3[]) : ('T1 * 'T2 * 'T3)[] = let chunkBySize (chunkSize: int) (array: 'T[]) : 'T[][] = if chunkSize < 1 then - invalidArg "size" "The input must be positive." + invalidArg "chunkSize" "The input must be positive." if Array.isEmpty array then [| [||] |] @@ -1030,7 +1075,7 @@ let windowed (windowSize: int) (source: 'T[]) : 'T[][] = let splitInto (chunks: int) (array: 'T[]) : 'T[][] = if chunks < 1 then - invalidArg "chunks" "The input must be positive." + invalidArg "count" "The input must be positive." if Array.isEmpty array then [| [||] |] @@ -1129,45 +1174,23 @@ let removeAt (index: int) (xs: 'T[]) : 'T[] = ) let removeManyAt (index: int) (count: int) (xs: 'T[]) : 'T[] = - let mutable i = -1 - // incomplete -1, in-progress 0, complete 1 - let mutable status = -1 - - let ys = - xs - |> filter (fun _ -> - i <- i + 1 + let len = xs.Length - if i = index then - status <- 0 - false - elif i > index then - if i < index + count then - false - else - status <- 1 - true - else - true - ) + // FSharp.Core's check. A negative count removes nothing, like in List.removeManyAt and + // Seq.removeManyAt, but the index can't be past the end + if index < 0 || index > len - count || index > len then + invalidArg "index" "index must be within bounds of the array" - let status = - if status = 0 && i + 1 = index + count then - 1 - else - status + let count = Operators.max 0 count - if status < 1 then - // F# always says the wrong parameter is index but the problem may be count - let arg = - if status < 0 then - "index" + Native.generate + (len - count) + (fun i -> + if i < index then + xs[i] else - "count" - - invalidArg arg SR.indexOutOfBounds - - ys + xs[i + count] + ) let updateAt (index: int) (y: 'T) (xs: 'T[]) : 'T[] = let len = xs.Length diff --git a/src/fable-library-dart/Global.fs b/src/fable-library-dart/Global.fs index 5da4b222b5..ce62f1f3f6 100644 --- a/src/fable-library-dart/Global.fs +++ b/src/fable-library-dart/Global.fs @@ -18,6 +18,7 @@ module SR = let inputWasEmpty = "The input list was empty." let inputMustBeNonNegative = "The input must be non-negative." + let inputMustBePositive = "The input must be positive." let inputSequenceEmpty = "The input sequence was empty." let inputSequenceTooLong = "The input sequence contains more than one element." diff --git a/src/fable-library-dart/List.fs b/src/fable-library-dart/List.fs index 50c0948e56..0c0c5ad1f6 100644 --- a/src/fable-library-dart/List.fs +++ b/src/fable-library-dart/List.fs @@ -152,6 +152,9 @@ and List<'T> = LinkedList<'T> let inline indexNotFound () = raise (KeyNotFoundException(SR.keyNotFoundAlt)) +let private differentLengths (paramName: string) = + invalidArg paramName "The lists had different lengths." + let empty () = List.Empty let cons (x: 'T) (xs: 'T list) = List.Cons(x, xs) @@ -265,11 +268,30 @@ let fold2<'T1, 'T2, 'State> (folder: 'State -> 'T1 -> 'T2 -> 'State) (state: 'St xs <- xs.Tail ys <- ys.Tail + // Like FSharp.Core, name the list that ran out + if not xs.IsEmpty then + differentLengths "list2" + elif not ys.IsEmpty then + differentLengths "list1" + acc let foldBack2<'T1, 'T2, 'State> (folder: 'T1 -> 'T2 -> 'State -> 'State) (xs: 'T1 list) (ys: 'T2 list) (state: 'State) = // fold2 (fun acc x y -> folder x y acc) state (reverse xs) (reverse ys) - Array.foldBack2 folder (toArray xs) (toArray ys) state + let xs = toArray xs + let ys = toArray ys + + if xs.Length <> ys.Length then + differentLengths ( + if xs.Length = 0 then + "list1" + elif ys.Length = 0 then + "list2" + else + "list1, list2" + ) + + Array.foldBack2 folder xs ys state let unfold<'T, 'State> (generator: 'State -> ('T * 'State) option) (state: 'State) = let rec loop acc (node: 'T list) = @@ -414,6 +436,11 @@ let map2 (mapping: 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) = let mapIndexed2 (mapping: int -> 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) = let rec loop i (acc: 'U list) (xs: 'T1 list) (ys: 'T2 list) = if xs.IsEmpty || ys.IsEmpty then + if not xs.IsEmpty then + differentLengths "list2" + elif not ys.IsEmpty then + differentLengths "list1" + acc else let node = acc.AppendConsNoTail(mapping i xs.Head ys.Head) @@ -427,6 +454,9 @@ let mapIndexed2 (mapping: int -> 'T1 -> 'T2 -> 'U) (xs: 'T1 list) (ys: 'T2 list) let map3 (mapping: 'T1 -> 'T2 -> 'T3 -> 'U) (xs: 'T1 list) (ys: 'T2 list) (zs: 'T3 list) = let rec loop (acc: 'U list) (xs: 'T1 list) (ys: 'T2 list) (zs: 'T3 list) = if xs.IsEmpty || ys.IsEmpty || zs.IsEmpty then + if not (xs.IsEmpty && ys.IsEmpty && zs.IsEmpty) then + differentLengths "list1, list2, list3" + acc else let node = acc.AppendConsNoTail(mapping xs.Head ys.Head zs.Head) @@ -568,6 +598,9 @@ let contains (value: 'T) (xs: 'T list) ([] eq: IEqualityComparer<'T>) = tryFindIndex (fun v -> eq.Equals(value, v)) xs |> Option.isSome let initialize n (f: int -> 'T) = + if n < 0 then + invalidArg "count" SR.inputMustBeNonNegative + let root = List.Empty let mutable node = root @@ -593,8 +626,12 @@ let reduceBack f (xs: 'T list) = let forAll f xs = fold (fun acc x -> acc && f x) true xs -let forAll2 f xs ys = - fold2 (fun acc x y -> acc && f x y) true xs ys +let rec forAll2 (f: 'T1 -> 'T2 -> bool) (xs: 'T1 list) (ys: 'T2 list) = + match xs.IsEmpty, ys.IsEmpty with + | true, true -> true + | false, false -> f xs.Head ys.Head && forAll2 f xs.Tail ys.Tail + | true, false -> differentLengths "list1" + | false, true -> differentLengths "list2" let exists f xs = tryFindIndex f xs |> Option.isSome @@ -602,7 +639,7 @@ let rec exists2 (f: 'T1 -> 'T2 -> bool) (xs: 'T1 list) (ys: 'T2 list) = match xs.IsEmpty, ys.IsEmpty with | true, true -> false | false, false -> f xs.Head ys.Head || exists2 f xs.Tail ys.Tail - | _ -> invalidArg "list2" SR.differentLengths + | _ -> differentLengths "list2" let unzip xs = foldBack (fun (x, y) (lacc, racc) -> List.Cons(x, lacc), List.Cons(y, racc)) xs (List.Empty, List.Empty) @@ -681,6 +718,9 @@ let min (xs: 'T list) ([] comparer: IComparer<'T>) : 'T = xs let average (xs: 'T list) ([] averager: IGenericAverager<'T>) : 'T = + if xs.IsEmpty then + invalidArg "source" SR.inputSequenceEmpty + let mutable count = 0 let folder acc x = @@ -691,6 +731,9 @@ let average (xs: 'T list) ([] averager: IGenericAverager<'T>) : 'T = averager.DivideByInt(total, count) let averageBy (f: 'T -> 'U) (xs: 'T list) ([] averager: IGenericAverager<'U>) : 'U = + if xs.IsEmpty then + invalidArg "source" SR.inputSequenceEmpty + let mutable count = 0 let inline folder acc x = @@ -927,43 +970,22 @@ let removeAt (index: int) (xs: 'T list) : 'T list = ys let removeManyAt (index: int) (count: int) (xs: 'T list) : 'T list = + if index < 0 then + invalidArg "index" "index must be within bounds of the list" + + let endIndex = index + count let mutable i = -1 - // incomplete -1, in-progress 0, complete 1 - let mutable status = -1 let ys = xs |> filter (fun _ -> i <- i + 1 - - if i = index then - status <- 0 - false - elif i > index then - if i < index + count then - false - else - status <- 1 - true - else - true + i < index || i >= endIndex ) - let status = - if status = 0 && i + 1 = index + count then - 1 - else - status - - if status < 1 then - // F# always says the wrong parameter is index but the problem may be count - let arg = - if status < 0 then - "index" - else - "count" - - invalidArg arg SR.indexOutOfBounds + // i + 1 is now the length of the list + if endIndex > i + 1 then + invalidArg "index" "index must be within bounds of the list" ys diff --git a/src/fable-library-dart/Seq.fs b/src/fable-library-dart/Seq.fs index dc22feb87f..1949d71aec 100644 --- a/src/fable-library-dart/Seq.fs +++ b/src/fable-library-dart/Seq.fs @@ -630,6 +630,9 @@ let tryHead (xs: seq<'T>) : 'T option = None let initialize (count: int) (f: int -> 'a) : seq<'a> = + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + unfold (fun i -> if (i < count) then @@ -954,7 +957,12 @@ let reduceBack (folder: 'T -> 'T -> 'T) (xs: seq<'T>) : 'T = Array.reduceBack folder arr -let replicate (n: int) (x: 'a) : seq<'a> = initialize n (fun _ -> x) +let replicate (n: int) (x: 'a) : seq<'a> = + // FSharp.Core uses Enumerable.Repeat, which raises ArgumentOutOfRangeException + if n < 0 then + raise (ArgumentOutOfRangeException("count", SR.Arg_ArgumentOutOfRangeException)) + + initialize n (fun _ -> x) let reverse (xs: seq<'T>) : seq<'T> = delay (fun () -> xs |> toArray |> Array.rev |> ofArray) @@ -1026,6 +1034,9 @@ let tail (xs: seq<'T>) : seq<'T> = ) let take (count: int) (xs: seq<'T>) : seq<'T> = + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + Enumerator.generateIndexed (fun () -> ofSeq xs) (fun i e -> @@ -1074,19 +1085,33 @@ let pairwise (xs: seq<'T>) : seq<'T * 'T> = delay (fun () -> xs |> toArray |> Array.pairwise |> ofArray) let splitInto (chunks: int) (xs: seq<'T>) : 'T[] seq = + if chunks < 1 then + invalidArg "count" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.splitInto chunks |> ofArray) let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = + if windowSize <= 0 then + invalidArg "windowSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) +// Like FSharp.Core, groups the elements by their index, so sequences of different lengths are fine let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> - xss - |> toArray - |> Array.map toArray - |> Array.transpose - |> Array.map ofArray - |> ofArray + let rows = ResizeArray>() + + for xs in xss do + let mutable i = 0 + + for x in xs do + if i = rows.Count then + rows.Add(ResizeArray()) + + rows[i].Add(x) + i <- i + 1 + + rows |> map (fun row -> row :> seq<'T>) ) let sortWith (comparer: 'T -> 'T -> int) (xs: seq<'T>) = @@ -1186,6 +1211,9 @@ let permute f (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.permute f |> ofArray) let chunkBySize (chunkSize: int) (xs: seq<'T>) : seq<'T[]> = + if chunkSize < 1 then + invalidArg "chunkSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.chunkBySize chunkSize |> ofArray) let insertAt (index: int) (y: 'T) (xs: seq<'T>) : seq<'T> = @@ -1275,30 +1303,32 @@ let removeAt (index: int) (xs: seq<'T>) : seq<'T> = ) (fun e -> e.Dispose()) +// Like FSharp.Core: a count past the end (or a negative one) isn't an error, but the input +// must be longer than index, which can only be checked once it has been enumerated let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then - invalidArg "index" SR.indexOutOfBounds + invalidArg "index" "index must be within bounds of the array" - Enumerator.generateIndexed - (fun () -> ofSeq xs) - (fun i e -> + let endIndex = index + count + + delay (fun () -> + let mutable i = -1 + + let rest () = if i < index then - if e.MoveNext() then - Some(e.Current) - else - invalidArg "index" SR.indexOutOfBounds - else - if i = index then - for _ = 1 to count do - if not (e.MoveNext()) then - invalidArg "count" SR.indexOutOfBounds + invalidArg "index" "index must be within bounds of the array" - if e.MoveNext() then - Some(e.Current) - else - None - ) - (fun e -> e.Dispose()) + empty () + + let ys = + xs + |> filter (fun _ -> + i <- i + 1 + i < index || i >= endIndex + ) + + append ys (delay rest) + ) let updateAt (index: int) (y: 'T) (xs: seq<'T>) : seq<'T> = let mutable isDone = false diff --git a/tests/Dart/src/ArrayTests.fs b/tests/Dart/src/ArrayTests.fs index b0284149ab..53ce577dc9 100644 --- a/tests/Dart/src/ArrayTests.fs +++ b/tests/Dart/src/ArrayTests.fs @@ -1505,6 +1505,48 @@ let tests () = raisedExceptionType (fun () -> Array.randomChoicesBy (fun () -> 1.0) 1 [|1|]) |> equal "ArgumentOutOfRangeException" raisedExceptionType (fun () -> Array.randomSampleBy (fun () -> 1.0) 1 [|1; 2|]) |> equal "ArgumentOutOfRangeException" + testCase "Array.sub with an index or count out of range raises" <| fun () -> + let xs = [|1; 2; 3|] + raisedExceptionType (fun () -> Array.sub xs -1 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.sub xs 1 -1) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.sub xs 2 5) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.sub xs 4 0) |> equal "ArgumentOutOfRangeException" + Array.sub xs 3 0 |> equal [||] + + testCase "Array.fill with an index or count out of range raises" <| fun () -> + let xs = [|1; 2; 3|] + raisedExceptionType (fun () -> Array.fill xs -1 2 0) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.fill xs 0 -1 0) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.fill xs 3 1 0) |> equal "IndexOutOfRangeException" + Array.fill xs 4 0 0 + xs |> equal [|1; 2; 3|] + + testCase "Array.blit and System.Array.Copy with an index or count out of range raise" <| fun () -> + let source = [|1; 2; 3|] + let target = Array.zeroCreate 5 + raisedExceptionType (fun () -> Array.blit source 0 target 0 -1) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit source -1 target 0 2) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit source 0 target -1 2) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.blit source 2 target 0 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.blit source 0 target 4 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.blit source 2 target -1 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> System.Array.Copy(source, 0, target, 0, 4)) |> equal "ArgumentException" + raisedExceptionType (fun () -> System.Array.Copy(source, target, -1)) |> equal "ArgumentOutOfRangeException" + target |> equal [|0; 0; 0; 0; 0|] + Array.blit source 3 target 5 0 + target |> equal [|0; 0; 0; 0; 0|] + + testCase "Array.removeManyAt raises ArgumentException only when FSharp.Core does" <| fun () -> + let xs = [|1; 2; 3|] + raisedExceptionType (fun () -> Array.removeManyAt -1 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt -1 1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt 2 2 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.removeManyAt 4 0 xs) |> equal "ArgumentException" + Array.removeManyAt 3 0 xs |> equal [|1; 2; 3|] + Array.removeManyAt 1 0 xs |> equal [|1; 2; 3|] + Array.removeManyAt 0 0 ([||]: int[]) |> equal [||] + Array.removeManyAt 1 2 xs |> equal [|1|] + // testCase "System.Array.Resize works" <| fun () -> // let mutable xs = [|1; 2; 3; 4; 5|] // System.Array.Resize(&xs, 3) diff --git a/tests/Dart/src/ListTests.fs b/tests/Dart/src/ListTests.fs index acdde1fa7d..09a58fc102 100644 --- a/tests/Dart/src/ListTests.fs +++ b/tests/Dart/src/ListTests.fs @@ -1106,3 +1106,51 @@ let tests() = raisedExceptionType (fun () -> List.randomChoiceBy (fun () -> 1.0) [1]) |> equal "ArgumentOutOfRangeException" raisedExceptionType (fun () -> List.randomChoicesBy (fun () -> 1.0) 1 [1]) |> equal "ArgumentOutOfRangeException" raisedExceptionType (fun () -> List.randomSampleBy (fun () -> 1.0) 1 [1; 2]) |> equal "ArgumentOutOfRangeException" + + testCase "List functions on lists of different lengths raise ArgumentException" <| fun () -> + let xs = [1; 2] + let ys = [1; 2; 3] + raisedExceptionType (fun () -> List.zip xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip3 xs ys ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.zip3 ys ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map2 (+) [] [1]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.mapi2 (fun i x y -> i + x + y) ys xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map3 (fun x y z -> x + y + z) ys xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.fold2 (fun acc x y -> acc + x + y) 0 xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.iter2 (fun _ _ -> ()) [1] []) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.iteri2 (fun _ _ _ -> ()) xs ys) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.forall2 (fun _ _ -> true) xs ys) |> equal "ArgumentException" + + testCase "List functions on lists of different lengths call the function on the common prefix first" <| fun () -> + let calls = ResizeArray() + raisedExceptionType (fun () -> List.iter2 (fun x y -> calls.Add(x + y)) [1; 2] [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.map3 (fun x y z -> calls.Add(x + y + z)) [1; 2; 3] [1; 2] [1; 2]) |> equal "ArgumentException" + calls |> Seq.toList |> equal [2; 4; 3; 6] + + testCase "List.forall2 returns false before reaching the end of lists of different lengths" <| fun () -> + List.forall2 (fun x y -> x + y < 3) [1; 2] [1; 2; 3] |> equal false + List.forall2 (fun _ _ -> false) [1; 2; 3] [1; 2] |> equal false + raisedExceptionType (fun () -> List.forall2 (fun _ _ -> false) [] [1]) |> equal "ArgumentException" + + testCase "List.init and List.replicate with a negative count raise ArgumentException" <| fun () -> + raisedExceptionType (fun () -> List.init -1 id) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.replicate -1 "a") |> equal "ArgumentException" + List.init 0 id |> equal [] + + testCase "List.average and List.averageBy on an empty list raise ArgumentException" <| fun () -> + let xs: float list = [] + raisedExceptionType (fun () -> List.average xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.averageBy (fun x -> x * 2.) xs) |> equal "ArgumentException" + + testCase "List.removeManyAt raises ArgumentException only when FSharp.Core does" <| fun () -> + let xs = [1; 2; 3] + raisedExceptionType (fun () -> List.removeManyAt -1 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt -1 1 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 2 2 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.removeManyAt 4 0 xs) |> equal "ArgumentException" + List.removeManyAt 3 0 xs |> equal [1; 2; 3] + List.removeManyAt 1 0 xs |> equal [1; 2; 3] + List.removeManyAt 2 -1 xs |> equal [1; 2; 3] + List.removeManyAt 0 0 ([]: int list) |> equal [] + List.removeManyAt 1 2 xs |> equal [1] diff --git a/tests/Dart/src/SeqTests.fs b/tests/Dart/src/SeqTests.fs index 795c9e297e..f59e0f0aaa 100644 --- a/tests/Dart/src/SeqTests.fs +++ b/tests/Dart/src/SeqTests.fs @@ -1196,3 +1196,41 @@ let tests() = raisedExceptionType (fun () -> [1 .. 0 .. 5]) |> equal "ArgumentException" raisedExceptionType (fun () -> [|1L .. 0L .. 5L|]) |> equal "ArgumentException" raisedExceptionType (fun () -> [1. .. 0. .. 5.]) |> equal "ArgumentException" + + testCase "Seq.init, Seq.replicate and Seq.take with a negative count raise when called" <| fun () -> + raisedExceptionType (fun () -> Seq.init -1 id) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.replicate -1 "a") |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Seq.take -1 [1; 2]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.take -1 (seq { 1; 2 })) |> equal "ArgumentException" + + testCase "Seq.chunkBySize, Seq.windowed and Seq.splitInto check their size when called" <| fun () -> + raisedExceptionType (fun () -> Seq.chunkBySize 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.windowed 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.splitInto 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.chunkBySize -1 Seq.empty) |> equal "ArgumentException" + + testCase "Seq.removeManyAt raises ArgumentException only when FSharp.Core does" <| fun () -> + let xs = [1; 2; 3] + raisedExceptionType (fun () -> Seq.removeManyAt -1 0 xs) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 0 0 Seq.empty |> Seq.toList) |> equal "ArgumentException" + let yielded = ResizeArray() + raisedExceptionType (fun () -> Seq.removeManyAt 3 0 xs |> Seq.iter (fun x -> yielded.Add x)) |> equal "ArgumentException" + yielded |> Seq.toList |> equal [1; 2; 3] + Seq.removeManyAt 2 2 xs |> Seq.toList |> equal [1; 2] + Seq.removeManyAt 1 3 xs |> Seq.toList |> equal [1] + Seq.removeManyAt 1 -1 xs |> Seq.toList |> equal [1; 2; 3] + Seq.removeManyAt 1 1 xs |> Seq.toList |> equal [1; 3] + + testCase "Seq.transpose groups the elements of sequences of different lengths by index" <| fun () -> + let toLists (xss: seq>) = xss |> Seq.map Seq.toList |> Seq.toList + Seq.transpose [[1; 2]; [3]] |> toLists |> equal [[1; 3]; [2]] + Seq.transpose [[1]; [2; 3]] |> toLists |> equal [[1; 2]; [3]] + Seq.transpose [[]; [1]] |> toLists |> equal [[1]] + Seq.transpose [[1; 2; 3]; [4]; [5; 6]] |> toLists |> equal [[1; 4; 5]; [2; 6]; [3]] + Seq.transpose (seq [seq { 1; 2 }; seq { 3 }]) |> toLists |> equal [[1; 3]; [2]] + Seq.transpose [|[|1|]; [||]|] |> toLists |> equal [[1]] + Seq.transpose ([[]]: int list list) |> toLists |> equal [] + let enumerated = ResizeArray() + let xss = Seq.transpose (seq { enumerated.Add 1; yield [1; 2]; yield [3] }) + enumerated.Count |> equal 0 + xss |> toLists |> equal [[1; 3]; [2]] diff --git a/tests/Dart/src/Tests.Util.fs b/tests/Dart/src/Tests.Util.fs index d821dfb7b0..db6751fa4d 100644 --- a/tests/Dart/src/Tests.Util.fs +++ b/tests/Dart/src/Tests.Util.fs @@ -32,6 +32,7 @@ let raisedExceptionType (f: unit -> 'a): string = | :? System.ArgumentException -> "ArgumentException" | :? System.InvalidOperationException -> "InvalidOperationException" | :? System.Collections.Generic.KeyNotFoundException -> "KeyNotFoundException" + | :? System.IndexOutOfRangeException -> "IndexOutOfRangeException" | e -> "Unexpected exception: " + e.Message let private run (f: unit -> 'a) = From e2cba39aea511bf7e4ab222e86455a426becc180 Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Sun, 25 Oct 2026 08:18:37 -0500 Subject: [PATCH 4/5] fix(beam): add FSharp.Core's argument checks to collection functions Some collection functions returned a value (often a wrong one), looped forever or failed with an untyped Erlang error where FSharp.Core rejects the argument, and a few raised where FSharp.Core does not: - Array.sub raises ArgumentException for a negative startIndex or count and ArgumentOutOfRangeException for a range past the end, instead of truncating through lists:sublist/3. It still walks only the items it needs. - Array.blit checks its arguments like System.Array.Copy, in the same order, before it copies anything. A source range past the end shortened the target instead of raising ArgumentException. - Seq.init and Seq.take raise ArgumentException, and Seq.replicate ArgumentOutOfRangeException (it is Enumerable.Repeat on .NET), for a negative count when they are called. They returned an empty sequence. - List, Array and Seq chunkBySize, windowed and splitInto reject a size below 1 with FSharp.Core's parameter names, the Seq ones when they are called. chunkBySize 0 looped forever, and so did splitInto with some negative counts. - splitInto returns an empty result for an empty input instead of dividing by zero. - List.transpose and Array.transpose raise ArgumentException for jagged input instead of truncating it or failing in hd/1. Seq.transpose groups jagged input by index like FSharp.Core instead of going through Array.transpose. - List.forall2 and List.exists2 walk both lists like FSharp.Core instead of zipping them first, so they return as soon as the predicate decides and raise ArgumentException only when one list runs out first. Array.forall2 and Array.exists2 check the lengths up front. - Seq.removeManyAt follows FSharp.Core: a count past the end or below 1 is not an error, and a source without an element at index raises ArgumentException once it has been enumerated. The 3-argument System.Array.Copy, which ignores its destination, is a different bug and is left alone. --- src/Fable.Transforms/Beam/Replacements.fs | 8 +- src/fable-library-beam/Seq.fs | 77 ++++++++----- src/fable-library-beam/fable_list.erl | 125 ++++++++++++++++++++-- tests/Beam/ArrayTests.fs | 49 +++++++++ tests/Beam/ListTests.fs | 30 ++++++ tests/Beam/SeqTests.fs | 36 +++++++ 6 files changed, 284 insertions(+), 41 deletions(-) diff --git a/src/Fable.Transforms/Beam/Replacements.fs b/src/Fable.Transforms/Beam/Replacements.fs index c0f298fca7..c0c94f7b81 100644 --- a/src/Fable.Transforms/Beam/Replacements.fs +++ b/src/Fable.Transforms/Beam/Replacements.fs @@ -2907,7 +2907,7 @@ let private arrayModule | ("Sub" | "GetSubArray"), [ arr; start; count ] -> let arr = derefArr r arr - emitExpr r t [ arr; start; count ] "lists:sublist($0, $1 + 1, $2)" + Helper.LibCall(com, "fable_list", "sub", t, [ arr; start; count ]) |> wrapArr com r t |> Some | "Except", [ excludeArr; arr ] -> @@ -2978,11 +2978,11 @@ let private arrayModule | "Exists2", [ fn; a1; a2 ] -> let a1 = derefArr r a1 let a2 = derefArr r a2 - Helper.LibCall(com, "fable_list", "exists2", t, [ fn; a1; a2 ]) |> Some + Helper.LibCall(com, "fable_list", "array_exists2", t, [ fn; a1; a2 ]) |> Some | "ForAll2", [ fn; a1; a2 ] -> let a1 = derefArr r a1 let a2 = derefArr r a2 - Helper.LibCall(com, "fable_list", "forall2", t, [ fn; a1; a2 ]) |> Some + Helper.LibCall(com, "fable_list", "array_forall2", t, [ fn; a1; a2 ]) |> Some | "Fold2", [ fn; state; a1; a2 ] -> let a1 = derefArr r a1 let a2 = derefArr r a2 @@ -3134,7 +3134,7 @@ let private arrayModule let blitted = Helper.LibCall( com, - "fable_resize_array", + "fable_list", "blit", t, [ derefSource; sourceIdx; derefTarget; targetIdx; count ] diff --git a/src/fable-library-beam/Seq.fs b/src/fable-library-beam/Seq.fs index 71671d5394..1771f65c86 100644 --- a/src/fable-library-beam/Seq.fs +++ b/src/fable-library-beam/Seq.fs @@ -13,6 +13,7 @@ type IEnumerable<'T> = System.Collections.Generic.IEnumerable<'T> module SR = let enumerationAlreadyFinished = "Enumeration already finished." let enumerationNotStarted = "Enumeration has not started. Call MoveNext." + let inputMustBePositive = "The input must be positive." let inputSequenceEmpty = "The input sequence was empty." let inputSequenceTooLong = "The input sequence contains more than one element." @@ -683,6 +684,9 @@ let head (xs: seq<'T>) = | None -> invalidArg "source" SR.inputSequenceEmpty let initialize count f = + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + unfold (fun i -> if (i < count) then @@ -1002,7 +1006,12 @@ let reduceBack folder (xs: seq<'T>) = else invalidArg "source" SR.inputSequenceEmpty -let replicate n x = initialize n (fun _ -> x) +let replicate n x = + // FSharp.Core uses Enumerable.Repeat, which raises ArgumentOutOfRangeException + if n < 0 then + raise (System.ArgumentOutOfRangeException("count")) + + initialize n (fun _ -> x) let reverse (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.rev |> ofArray) @@ -1074,6 +1083,9 @@ let tail (xs: seq<'T>) = ) let take count (xs: seq<'T>) = + if count < 0 then + invalidArg "count" SR.inputMustBeNonNegative + generateIndexed (fun () -> ofSeq xs) (fun i e -> @@ -1122,19 +1134,27 @@ let pairwise (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.pairwise |> ofArray) let splitInto (chunks: int) (xs: seq<'T>) : 'T[] seq = + if chunks < 1 then + invalidArg "count" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.splitInto chunks |> ofArray) let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = + if windowSize < 1 then + invalidArg "windowSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) +// Not Array.transpose, which rejects jagged input: FSharp.Core's Seq.transpose groups +// the elements by their index, so [[1; 2]; [3]] gives [[1; 3]; [2]]. let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> - xss - |> toArray - |> Array.map toArray - |> Array.transpose - |> Array.map ofArray - |> ofArray + let rec columns (rows: 'T list list) = + match rows |> List.filter (List.isEmpty >> not) with + | [] -> [] + | rows -> ofList (List.map List.head rows) :: columns (List.map List.tail rows) + + xss |> toList |> List.map toList |> columns |> ofList ) let sortWith (comparer: 'T -> 'T -> int) (xs: seq<'T>) = @@ -1238,6 +1258,9 @@ let permute f (xs: seq<'T>) = delay (fun () -> xs |> toArray |> Array.permute f |> ofArray) let chunkBySize (chunkSize: int) (xs: seq<'T>) : seq<'T[]> = + if chunkSize < 1 then + invalidArg "chunkSize" SR.inputMustBePositive + delay (fun () -> xs |> toArray |> Array.chunkBySize chunkSize |> ofArray) // let init = initialize @@ -1340,30 +1363,32 @@ let removeAt (index: int) (xs: seq<'T>) : seq<'T> = ) (fun e -> e.Dispose()) +// Like FSharp.Core: a count past the end (or a negative one) isn't an error, but the input +// must be longer than index, which can only be checked once it has been enumerated let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then - invalidArg "index" SR.indexOutOfBounds + invalidArg "index" "index must be within bounds of the array" - generateIndexed - (fun () -> ofSeq xs) - (fun i e -> + let endIndex = index + count + + delay (fun () -> + let mutable i = -1 + + let rest () = if i < index then - if e.MoveNext() then - Some e.Current - else - invalidArg "index" SR.indexOutOfBounds - else - if i = index then - for _ = 1 to count do - if not (e.MoveNext()) then - invalidArg "count" SR.indexOutOfBounds + invalidArg "index" "index must be within bounds of the array" - if e.MoveNext() then - Some e.Current - else - None - ) - (fun e -> e.Dispose()) + empty () + + let ys = + xs + |> filter (fun _ -> + i <- i + 1 + i < index || i >= endIndex + ) + + append ys (delay rest) + ) let updateAt (index: int) (y: 'T) (xs: seq<'T>) : seq<'T> = let mutable isDone = false diff --git a/src/fable-library-beam/fable_list.erl b/src/fable-library-beam/fable_list.erl index 580eb7c9e5..633f07ac75 100644 --- a/src/fable-library-beam/fable_list.erl +++ b/src/fable-library-beam/fable_list.erl @@ -47,6 +47,8 @@ pairwise/1, exists2/3, forall2/3, + array_exists2/3, + array_forall2/3, map2/3, map3/4, mapi2/3, @@ -79,6 +81,8 @@ remove_many_at/3, index_of_value/2, get_slice/3, + sub/3, + blit/5, take/2, random_shuffle_by/2, random_shuffle_with/2, @@ -134,6 +138,8 @@ -spec pairwise(list()) -> list({term(), term()}). -spec exists2(fun(), list(), list()) -> boolean(). -spec forall2(fun(), list(), list()) -> boolean(). +-spec array_exists2(fun(), list(), list()) -> boolean(). +-spec array_forall2(fun(), list(), list()) -> boolean(). -spec map2(fun(), list(), list()) -> list(). -spec map3(fun(), list(), list(), list()) -> list(). -spec mapi2(fun(), list(), list()) -> list(). @@ -147,9 +153,9 @@ -spec group_by(fun(), list()) -> list({term(), list()}). -spec unfold(fun(), term()) -> list(). -spec split_at(non_neg_integer(), list()) -> {list(), list()}. --spec chunk_by_size(pos_integer(), list()) -> list(list()). --spec windowed(pos_integer(), list()) -> list(list()). --spec split_into(pos_integer(), list()) -> list(list()). +-spec chunk_by_size(integer(), list()) -> list(list()). +-spec windowed(integer(), list()) -> list(list()). +-spec split_into(integer(), list()) -> list(list()). -spec except(list(), list()) -> list(). -spec all_pairs(list(), list()) -> list({term(), term()}). -spec permute(fun(), list()) -> list(). @@ -424,11 +430,40 @@ pairwise([]) -> []; pairwise([_]) -> []; pairwise([A, B | T]) -> [{A, B} | pairwise([B | T])]. -exists2(Fn, L1, L2) -> - lists:any(fun({A, B}) -> (Fn(A))(B) end, lists:zip(L1, L2)). - -forall2(Fn, L1, L2) -> - lists:all(fun({A, B}) -> (Fn(A))(B) end, lists:zip(L1, L2)). +%% List.exists2 and List.forall2 walk both lists like FSharp.Core, so the predicate can +%% decide the result before the shorter list runs out. +exists2(Fn, [A | T1], [B | T2]) -> + case (Fn(A))(B) of + true -> true; + false -> exists2(Fn, T1, T2) + end; +exists2(_Fn, [], []) -> + false; +exists2(_Fn, _L1, _L2) -> + lists_had_different_lengths(<<"list2">>). + +forall2(Fn, [A | T1], [B | T2]) -> + case (Fn(A))(B) of + true -> forall2(Fn, T1, T2); + false -> false + end; +forall2(_Fn, [], []) -> + true; +forall2(_Fn, [], _L2) -> + lists_had_different_lengths(<<"list1">>); +forall2(_Fn, _L1, []) -> + lists_had_different_lengths(<<"list2">>). + +%% Array.exists2 and Array.forall2 check the lengths before calling the predicate. +array_exists2(Fn, A1, A2) when length(A1) =:= length(A2) -> + exists2(Fn, A1, A2); +array_exists2(_Fn, _A1, _A2) -> + invalid_arg(<<"array1">>, <<"The arrays have different lengths.">>). + +array_forall2(Fn, A1, A2) when length(A1) =:= length(A2) -> + forall2(Fn, A1, A2); +array_forall2(_Fn, _A1, _A2) -> + invalid_arg(<<"array1">>, <<"The arrays have different lengths.">>). map2(Fn, L1, L2) -> lists:zipwith(fun(A, B) -> (Fn(A))(B) end, L1, L2). @@ -507,6 +542,8 @@ unfold(Fn, State) -> split_at(Index, List) -> {lists:sublist(List, Index), lists:nthtail(Index, List)}. +chunk_by_size(Size, _List) when Size < 1 -> + invalid_arg(<<"chunkSize">>, <<"The input must be positive.">>); chunk_by_size(Size, List) -> chunk_by_size(Size, List, []). chunk_by_size(_Size, [], Acc) -> @@ -519,6 +556,8 @@ chunk_by_size(Size, List, Acc) -> end, chunk_by_size(Size, Rest, [Chunk | Acc]). +windowed(Size, _List) when Size < 1 -> + invalid_arg(<<"windowSize">>, <<"The input must be positive.">>); windowed(Size, List) -> windowed(Size, List, []). windowed(Size, List, Acc) when length(List) < Size -> lists:reverse(Acc); @@ -526,6 +565,10 @@ windowed(Size, [_ | T] = List, Acc) -> Window = lists:sublist(List, Size), windowed(Size, T, [Window | Acc]). +split_into(Count, _List) when Count < 1 -> + invalid_arg(<<"count">>, <<"The input must be positive.">>); +split_into(_Count, []) -> + []; split_into(Count, List) -> Len = length(List), ActualCount = min(Count, Len), @@ -615,14 +658,23 @@ try_find_index_back(Fn, [H | T], Idx) -> false -> try_find_index_back(Fn, T, Idx - 1) end. +%% List.transpose and Array.transpose share this, so jagged input raises Array.transpose's +%% exception for both, as on the other targets. transpose([]) -> []; -transpose([[] | _]) -> +transpose([First | Rest] = Lists) -> + Len = length(First), + case lists:all(fun(L) -> length(L) =:= Len end, Rest) of + true -> transpose_rows(Lists); + false -> invalid_arg(<<"array.[0]">>, <<"The arrays have different lengths.">>) + end. + +transpose_rows([[] | _]) -> []; -transpose(Lists) -> +transpose_rows(Lists) -> Heads = [hd(L) || L <- Lists], Tails = [tl(L) || L <- Lists], - [Heads | transpose(Tails)]. + [Heads | transpose_rows(Tails)]. compare_with(_Fn, [], []) -> 0; @@ -676,6 +728,46 @@ get_slice(Lower, Upper, List) -> end, lists:sublist(List, Start + 1, End - Start + 1). +%% Array.sub checks its range like FSharp.Core, where lists:sublist/3 would truncate it. +-spec sub(list(), integer(), integer()) -> list(). +sub(_List, StartIndex, _Count) when StartIndex < 0 -> + invalid_arg(<<"startIndex">>, <<"The input must be non-negative.">>); +sub(_List, _StartIndex, Count) when Count < 0 -> + invalid_arg(<<"count">>, <<"The input must be non-negative.">>); +%% Walks only the first StartIndex + Count items, so a slice near the front of a long list stays cheap +sub(List, 0, Count) -> + case lists:sublist(List, Count) of + Sub when length(Sub) =:= Count -> Sub; + _ -> argument_out_of_range(<<"count">>, <<"The index is outside the legal range.">>) + end; +sub([_ | T], StartIndex, Count) -> + sub(T, StartIndex - 1, Count); +sub([], _StartIndex, _Count) -> + argument_out_of_range(<<"count">>, <<"The index is outside the legal range.">>). + +%% Array.blit is System.Array.Copy on .NET, so it checks the arguments in the same order before +%% anything is copied. HashSet.CopyTo calls fable_resize_array:blit/5 directly, because its count +%% may be larger than the set. +-spec blit(list(), integer(), list(), integer(), integer()) -> list(). +blit(_Source, _SourceIdx, _Target, _TargetIdx, Count) when Count < 0 -> + argument_out_of_range(<<"length">>, <<"Non-negative number required.">>); +blit(_Source, SourceIdx, _Target, _TargetIdx, _Count) when SourceIdx < 0 -> + argument_out_of_range(<<"sourceIndex">>, <<"Non-negative number required.">>); +blit(Source, SourceIdx, _Target, _TargetIdx, Count) when SourceIdx + Count > length(Source) -> + invalid_arg( + <<"sourceArray">>, + <<"Source array was not long enough. Check the source index, length, and the array's lower bounds.">> + ); +blit(_Source, _SourceIdx, _Target, TargetIdx, _Count) when TargetIdx < 0 -> + argument_out_of_range(<<"destinationIndex">>, <<"Non-negative number required.">>); +blit(_Source, _SourceIdx, Target, TargetIdx, Count) when TargetIdx + Count > length(Target) -> + invalid_arg( + <<"destinationArray">>, + <<"Destination array was not long enough. Check the destination index, length, and the array's lower bounds.">> + ); +blit(Source, SourceIdx, Target, TargetIdx, Count) -> + fable_resize_array:blit(Source, SourceIdx, Target, TargetIdx, Count). + -spec take(non_neg_integer(), list()) -> list(). take(Count, List) -> case erlang:length(List) >= Count of @@ -834,3 +926,14 @@ invalid_arg(ParamName, Message) -> exn_type => argument_exception, message => fable_utils:argument_exception_message(Message, ParamName) }). + +%% The same for an ArgumentOutOfRangeException. +argument_out_of_range(ParamName, Message) -> + erlang:error(#{ + exn_type => argument_out_of_range_exception, + message => fable_utils:argument_exception_message(Message, ParamName) + }). + +%% FSharp.Core's ArgumentException for lists that ran out at different times. +lists_had_different_lengths(ParamName) -> + invalid_arg(ParamName, <<"The lists had different lengths.">>). diff --git a/tests/Beam/ArrayTests.fs b/tests/Beam/ArrayTests.fs index 36cc63eb54..37ba2b0213 100644 --- a/tests/Beam/ArrayTests.fs +++ b/tests/Beam/ArrayTests.fs @@ -1506,3 +1506,52 @@ let ``test Array random functions raise ArgumentException for invalid inputs`` ( raisedExceptionType (fun () -> Array.randomSample -1 [|1|]) |> equal "ArgumentException" raisedExceptionType (fun () -> Array.randomSample 3 [|1; 2|]) |> equal "ArgumentException" raisedExceptionType (fun () -> Array.randomSample 1 [||]) |> equal "ArgumentException" + +[] +let ``test Array.sub raises when the range is outside the array`` () = + raisedExceptionType (fun () -> Array.sub [|1; 2; 3|] -1 2) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.sub [|1; 2; 3|] 1 -1) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.sub [|1; 2; 3|] 4 -1) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.sub [|1; 2; 3|] 2 5) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.sub [|1; 2; 3|] 3 1) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Array.sub [|1; 2; 3|] 4 0) |> equal "ArgumentOutOfRangeException" + Array.sub [|1; 2; 3|] 3 0 |> equal [||] + +[] +let ``test Array.exists2 and Array.forall2 check the lengths first`` () = + raisedExceptionType (fun () -> Array.exists2 (fun _ _ -> true) [|1; 2|] [|1; 2; 3|]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.forall2 (fun _ _ -> false) [|1; 2|] [|1; 2; 3|]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.forall2 (fun _ _ -> true) [|1; 2; 3|] [|1; 2|]) |> equal "ArgumentException" + +[] +let ``test Array.chunkBySize and Array.splitInto reject a size below 1`` () = + raisedExceptionType (fun () -> Array.chunkBySize 0 [|1|]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.chunkBySize 0 [||]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.splitInto 0 [|1|]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.splitInto -1 [||]) |> equal "ArgumentException" + +[] +let ``test Array.splitInto works with an empty array`` () = + Array.splitInto 3 [||] |> equal [||] + +[] +let ``test Array.transpose raises ArgumentException for arrays of different lengths`` () = + raisedExceptionType (fun () -> Array.transpose [|[|1|]; [|2; 3|]|]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.transpose [|[|1; 2|]; [|3|]|]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Array.transpose [|[||]; [|1|]|]) |> equal "ArgumentException" + +[] +let ``test Array.blit raises like System.Array.Copy and leaves the target untouched`` () = + let target = Array.zeroCreate 10 + let blit sourceIndex targetIndex count = + raisedExceptionType (fun () -> Array.blit [|1; 2; 3|] sourceIndex target targetIndex count) + blit 0 0 5 |> equal "ArgumentException" + blit 2 0 2 |> equal "ArgumentException" + blit 4 0 0 |> equal "ArgumentException" + blit 0 9 2 |> equal "ArgumentException" + blit 0 0 -1 |> equal "ArgumentOutOfRangeException" + blit -1 0 2 |> equal "ArgumentOutOfRangeException" + blit 0 -1 2 |> equal "ArgumentOutOfRangeException" + blit 3 0 0 |> equal "no exception" + blit 0 10 0 |> equal "no exception" + target |> equal (Array.zeroCreate 10) diff --git a/tests/Beam/ListTests.fs b/tests/Beam/ListTests.fs index 498beca6e1..ddb0027a45 100644 --- a/tests/Beam/ListTests.fs +++ b/tests/Beam/ListTests.fs @@ -940,3 +940,33 @@ let ``test List random functions raise ArgumentException for invalid inputs`` () raisedExceptionType (fun () -> List.randomSample -1 [1]) |> equal "ArgumentException" raisedExceptionType (fun () -> List.randomSample 3 [1; 2]) |> equal "ArgumentException" raisedExceptionType (fun () -> List.randomSample 1 []) |> equal "ArgumentException" + +[] +let ``test List.forall2 and List.exists2 stop once the result is known`` () = + List.forall2 (fun _ _ -> false) [1; 2] [1; 2; 3] |> equal false + List.exists2 (fun _ _ -> true) [1; 2] [1; 2; 3] |> equal true + raisedExceptionType (fun () -> List.forall2 (fun _ _ -> true) [1; 2] [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.forall2 (fun _ _ -> true) [1; 2; 3] [1; 2]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.exists2 (fun _ _ -> false) [1; 2] [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.exists2 (fun _ _ -> false) [1] []) |> equal "ArgumentException" + +[] +let ``test List.chunkBySize, List.windowed and List.splitInto reject a size below 1`` () = + raisedExceptionType (fun () -> List.chunkBySize 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.chunkBySize -1 []) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.windowed 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.windowed -1 []) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.splitInto 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.splitInto -4 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.splitInto -1 []) |> equal "ArgumentException" + +[] +let ``test List.splitInto works with an empty list`` () = + List.splitInto 1 [] |> equal [] + List.splitInto 3 [] |> equal [] + +[] +let ``test List.transpose raises ArgumentException for lists of different lengths`` () = + raisedExceptionType (fun () -> List.transpose [[1]; [2; 3]]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.transpose [[1; 2]; [3]]) |> equal "ArgumentException" + raisedExceptionType (fun () -> List.transpose [[]; [1]]) |> equal "ArgumentException" diff --git a/tests/Beam/SeqTests.fs b/tests/Beam/SeqTests.fs index ea637949d3..76237f65d1 100644 --- a/tests/Beam/SeqTests.fs +++ b/tests/Beam/SeqTests.fs @@ -1195,3 +1195,39 @@ let ``test Range expressions raise ArgumentException for a zero step`` () = raisedExceptionType (fun () -> [1 .. 0 .. 5]) |> equal "ArgumentException" raisedExceptionType (fun () -> [1L .. 0L .. 5L]) |> equal "ArgumentException" raisedExceptionType (fun () -> [1.0 .. 0.0 .. 5.0]) |> equal "ArgumentException" + +[] +let ``test Seq.init, Seq.replicate and Seq.take reject a negative count when called`` () = + raisedExceptionType (fun () -> Seq.init -1 id) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.replicate -1 0) |> equal "ArgumentOutOfRangeException" + raisedExceptionType (fun () -> Seq.take -1 [1; 2]) |> equal "ArgumentException" + +[] +let ``test Seq.chunkBySize, Seq.windowed and Seq.splitInto reject a size below 1 when called`` () = + raisedExceptionType (fun () -> Seq.chunkBySize 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.windowed 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.splitInto 0 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.chunkBySize -1 Seq.empty) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.windowed -1 Seq.empty) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.splitInto -1 Seq.empty) |> equal "ArgumentException" + +[] +let ``test Seq.splitInto works with an empty sequence`` () = + Seq.splitInto 2 Seq.empty |> Seq.toList |> equal [] + +[] +let ``test Seq.removeManyAt raises only when the sequence is too short for index`` () = + raisedExceptionType (fun () -> Seq.removeManyAt -1 1 [1; 2; 3]) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 3 0 [1; 2; 3] |> Seq.toList) |> equal "ArgumentException" + raisedExceptionType (fun () -> Seq.removeManyAt 0 0 Seq.empty |> Seq.toList) |> equal "ArgumentException" + Seq.removeManyAt 1 3 [1; 2; 3] |> Seq.toList |> equal [1] + Seq.removeManyAt 2 4 [1; 2; 3] |> Seq.toList |> equal [1; 2] + Seq.removeManyAt 1 0 [1; 2; 3] |> Seq.toList |> equal [1; 2; 3] + Seq.removeManyAt 0 -1 [1; 2; 3] |> Seq.toList |> equal [1; 2; 3] + +[] +let ``test Seq.transpose groups jagged input by index`` () = + [[1; 2]; [3]] |> Seq.transpose |> Seq.map Seq.toList |> Seq.toList |> equal [[1; 3]; [2]] + [|[|1|]; [|2; 3|]|] |> Seq.transpose |> Seq.map Seq.toList |> Seq.toList |> equal [[1; 2]; [3]] + [[1; 2; 3]; [4]; [5; 6]] |> Seq.transpose |> Seq.map Seq.toList |> Seq.toList |> equal [[1; 4; 5]; [2; 6]; [3]] + [[]; [1]] |> Seq.transpose |> Seq.map Seq.toList |> Seq.toList |> equal [[1]] From ce0ad5248d073800a6ed1b4b4c6b118f84d9a3bf Mon Sep 17 00:00:00 2001 From: Mangel Maxime Date: Wed, 28 Oct 2026 20:39:33 +0100 Subject: [PATCH 5/5] chore(all): trim comments in collection checks --- src/Fable.Transforms/Dart/Replacements.fs | 2 +- src/Fable.Transforms/Replacements.fs | 2 -- src/fable-library-beam/Seq.fs | 4 ---- src/fable-library-beam/fable_list.erl | 13 +------------ src/fable-library-dart/Array.fs | 8 ++------ src/fable-library-dart/List.fs | 2 -- src/fable-library-dart/Seq.fs | 3 --- src/fable-library-py/fable_library/List.fs | 2 -- src/fable-library-py/fable_library/Seq.fs | 3 --- src/fable-library-py/fable_library/seq_native.py | 1 - src/fable-library-ts/Array.fs | 7 ++----- src/fable-library-ts/List.fs | 2 -- src/fable-library-ts/Seq.fs | 3 --- 13 files changed, 6 insertions(+), 46 deletions(-) diff --git a/src/Fable.Transforms/Dart/Replacements.fs b/src/Fable.Transforms/Dart/Replacements.fs index bd6774959d..354372048a 100644 --- a/src/Fable.Transforms/Dart/Replacements.fs +++ b/src/Fable.Transforms/Dart/Replacements.fs @@ -1877,7 +1877,7 @@ let arrayModule (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (_: Ex | "IsEmpty", [ ar ] -> getFieldWith r t ar "isEmpty" |> Some | "Contains", [ value; ar ] -> Helper.LibCall(com, "Array", "contains", t, [ value; ar ], ?loc = r) |> Some | "CopyTo", args -> copyToArray com r t i args - // Not getSubArray, which ResizeArray.GetRange also uses: Array.sub checks its arguments like FSharp.Core + // getSubArray is shared with ResizeArray.GetRange, which must stay unchecked | "GetSubArray", args -> Helper.LibCall(com, "Array", "sub", t, args, i.SignatureArgTypes, genArgs = i.GenericArgs, ?loc = r) |> Some diff --git a/src/Fable.Transforms/Replacements.fs b/src/Fable.Transforms/Replacements.fs index f3e0cd8d38..616d36d473 100644 --- a/src/Fable.Transforms/Replacements.fs +++ b/src/Fable.Transforms/Replacements.fs @@ -2339,8 +2339,6 @@ let tuples (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (thisArg: E | "ToTuple", _ -> changeKind false args | _ -> None -/// System.Array.Copy compiles to Array.copyTo, which checks the arguments like it (Util.copyToArray doesn't). -/// Array.copyTo takes ints, so the Int64 overloads convert their arguments first. let arrayCopy (com: ICompiler) (ctx: Context) r t source sourceIndex target targetIndex count = let toInt32 (arg: Expr) = match arg.Type with diff --git a/src/fable-library-beam/Seq.fs b/src/fable-library-beam/Seq.fs index 1771f65c86..d4e496146e 100644 --- a/src/fable-library-beam/Seq.fs +++ b/src/fable-library-beam/Seq.fs @@ -1145,8 +1145,6 @@ let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) -// Not Array.transpose, which rejects jagged input: FSharp.Core's Seq.transpose groups -// the elements by their index, so [[1; 2]; [3]] gives [[1; 3]; [2]]. let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> let rec columns (rows: 'T list list) = @@ -1363,8 +1361,6 @@ let removeAt (index: int) (xs: seq<'T>) : seq<'T> = ) (fun e -> e.Dispose()) -// Like FSharp.Core: a count past the end (or a negative one) isn't an error, but the input -// must be longer than index, which can only be checked once it has been enumerated let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then invalidArg "index" "index must be within bounds of the array" diff --git a/src/fable-library-beam/fable_list.erl b/src/fable-library-beam/fable_list.erl index 633f07ac75..ea7d918ecb 100644 --- a/src/fable-library-beam/fable_list.erl +++ b/src/fable-library-beam/fable_list.erl @@ -430,8 +430,6 @@ pairwise([]) -> []; pairwise([_]) -> []; pairwise([A, B | T]) -> [{A, B} | pairwise([B | T])]. -%% List.exists2 and List.forall2 walk both lists like FSharp.Core, so the predicate can -%% decide the result before the shorter list runs out. exists2(Fn, [A | T1], [B | T2]) -> case (Fn(A))(B) of true -> true; @@ -454,7 +452,6 @@ forall2(_Fn, [], _L2) -> forall2(_Fn, _L1, []) -> lists_had_different_lengths(<<"list2">>). -%% Array.exists2 and Array.forall2 check the lengths before calling the predicate. array_exists2(Fn, A1, A2) when length(A1) =:= length(A2) -> exists2(Fn, A1, A2); array_exists2(_Fn, _A1, _A2) -> @@ -658,8 +655,6 @@ try_find_index_back(Fn, [H | T], Idx) -> false -> try_find_index_back(Fn, T, Idx - 1) end. -%% List.transpose and Array.transpose share this, so jagged input raises Array.transpose's -%% exception for both, as on the other targets. transpose([]) -> []; transpose([First | Rest] = Lists) -> @@ -728,13 +723,11 @@ get_slice(Lower, Upper, List) -> end, lists:sublist(List, Start + 1, End - Start + 1). -%% Array.sub checks its range like FSharp.Core, where lists:sublist/3 would truncate it. -spec sub(list(), integer(), integer()) -> list(). sub(_List, StartIndex, _Count) when StartIndex < 0 -> invalid_arg(<<"startIndex">>, <<"The input must be non-negative.">>); sub(_List, _StartIndex, Count) when Count < 0 -> invalid_arg(<<"count">>, <<"The input must be non-negative.">>); -%% Walks only the first StartIndex + Count items, so a slice near the front of a long list stays cheap sub(List, 0, Count) -> case lists:sublist(List, Count) of Sub when length(Sub) =:= Count -> Sub; @@ -745,9 +738,7 @@ sub([_ | T], StartIndex, Count) -> sub([], _StartIndex, _Count) -> argument_out_of_range(<<"count">>, <<"The index is outside the legal range.">>). -%% Array.blit is System.Array.Copy on .NET, so it checks the arguments in the same order before -%% anything is copied. HashSet.CopyTo calls fable_resize_array:blit/5 directly, because its count -%% may be larger than the set. +%% HashSet.CopyTo calls fable_resize_array:blit/5 directly, its count may exceed the set -spec blit(list(), integer(), list(), integer(), integer()) -> list(). blit(_Source, _SourceIdx, _Target, _TargetIdx, Count) when Count < 0 -> argument_out_of_range(<<"length">>, <<"Non-negative number required.">>); @@ -927,13 +918,11 @@ invalid_arg(ParamName, Message) -> message => fable_utils:argument_exception_message(Message, ParamName) }). -%% The same for an ArgumentOutOfRangeException. argument_out_of_range(ParamName, Message) -> erlang:error(#{ exn_type => argument_out_of_range_exception, message => fable_utils:argument_exception_message(Message, ParamName) }). -%% FSharp.Core's ArgumentException for lists that ran out at different times. lists_had_different_lengths(ParamName) -> invalid_arg(ParamName, <<"The lists had different lengths.">>). diff --git a/src/fable-library-dart/Array.fs b/src/fable-library-dart/Array.fs index d38bda594d..1753bb1914 100644 --- a/src/fable-library-dart/Array.fs +++ b/src/fable-library-dart/Array.fs @@ -149,8 +149,7 @@ let fill (target: 'T[]) (targetIndex: int) (count: int) (value: 'T) : 'T[] = if count < 0 then invalidArg "count" SR.inputMustBeNonNegative - // FSharp.Core sets the items one by one: a count of 0 is fine even past the end, and - // it fills up to the end before raising, where this checks first + // .NET fills up to the end of the array before raising, this checks first if count > 0 then if targetIndex + count > target.Length then raise (System.IndexOutOfRangeException(SR.Arg_IndexOutOfRangeException)) @@ -162,7 +161,6 @@ let fill (target: 'T[]) (targetIndex: int) (count: int) (value: 'T) : 'T[] = let getSubArray (array: 'T[]) (start: int) (count: int) : 'T[] = Native.sublist (array, start, start + count) -// Array.sub checks its arguments like FSharp.Core (unlike getSubArray, which ResizeArray.GetRange also uses) let sub (array: 'T[]) (startIndex: int) (count: int) : 'T[] = if startIndex < 0 then invalidArg "startIndex" SR.inputMustBeNonNegative @@ -426,7 +424,6 @@ let removeAllInPlace (predicate: 'T -> bool) (array: 'T[]) : int = Native.removeWhere array predicate len - array.Length -// Array.blit and System.Array.Copy: checks the arguments in the same order as System.Array.Copy let copyTo (source: 'T[]) (sourceIndex: int) (target: 'T[]) (targetIndex: int) (count: int) : unit = if count < 0 then raise (System.ArgumentOutOfRangeException("length", SR.Arg_ArgumentOutOfRangeException)) @@ -1176,8 +1173,7 @@ let removeAt (index: int) (xs: 'T[]) : 'T[] = let removeManyAt (index: int) (count: int) (xs: 'T[]) : 'T[] = let len = xs.Length - // FSharp.Core's check. A negative count removes nothing, like in List.removeManyAt and - // Seq.removeManyAt, but the index can't be past the end + // Unlike FSharp.Core, a negative count removes nothing instead of duplicating items if index < 0 || index > len - count || index > len then invalidArg "index" "index must be within bounds of the array" diff --git a/src/fable-library-dart/List.fs b/src/fable-library-dart/List.fs index 0c0c5ad1f6..6d2d365faa 100644 --- a/src/fable-library-dart/List.fs +++ b/src/fable-library-dart/List.fs @@ -268,7 +268,6 @@ let fold2<'T1, 'T2, 'State> (folder: 'State -> 'T1 -> 'T2 -> 'State) (state: 'St xs <- xs.Tail ys <- ys.Tail - // Like FSharp.Core, name the list that ran out if not xs.IsEmpty then differentLengths "list2" elif not ys.IsEmpty then @@ -983,7 +982,6 @@ let removeManyAt (index: int) (count: int) (xs: 'T list) : 'T list = i < index || i >= endIndex ) - // i + 1 is now the length of the list if endIndex > i + 1 then invalidArg "index" "index must be within bounds of the list" diff --git a/src/fable-library-dart/Seq.fs b/src/fable-library-dart/Seq.fs index 1949d71aec..6e63f866fe 100644 --- a/src/fable-library-dart/Seq.fs +++ b/src/fable-library-dart/Seq.fs @@ -1096,7 +1096,6 @@ let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) -// Like FSharp.Core, groups the elements by their index, so sequences of different lengths are fine let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> let rows = ResizeArray>() @@ -1303,8 +1302,6 @@ let removeAt (index: int) (xs: seq<'T>) : seq<'T> = ) (fun e -> e.Dispose()) -// Like FSharp.Core: a count past the end (or a negative one) isn't an error, but the input -// must be longer than index, which can only be checked once it has been enumerated let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then invalidArg "index" "index must be within bounds of the array" diff --git a/src/fable-library-py/fable_library/List.fs b/src/fable-library-py/fable_library/List.fs index 63a973162c..c4d4f49048 100644 --- a/src/fable-library-py/fable_library/List.fs +++ b/src/fable-library-py/fable_library/List.fs @@ -284,7 +284,6 @@ let fold2<'T1, 'T2, 'State> (folder: 'State -> 'T1 -> 'T2 -> 'State) (state: 'St xs <- xs.Tail ys <- ys.Tail - // FSharp.Core names the list that ran out first if not xs.IsEmpty then differentLengths "list2" elif not ys.IsEmpty then @@ -1008,7 +1007,6 @@ let removeManyAt (index: int) (count: int) (xs: 'T list) : 'T list = i < index || i >= endIndex ) - // i + 1 is now the length of the list if endIndex > i + 1 then invalidArg "index" "index must be within bounds of the list" diff --git a/src/fable-library-py/fable_library/Seq.fs b/src/fable-library-py/fable_library/Seq.fs index 7693beed62..4456f22e7e 100644 --- a/src/fable-library-py/fable_library/Seq.fs +++ b/src/fable-library-py/fable_library/Seq.fs @@ -995,7 +995,6 @@ let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) -// Groups the items by index like FSharp.Core, so jagged input gives shorter rows instead of raising let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> let rows = ResizeArray>() @@ -1223,8 +1222,6 @@ let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then invalidArg "index" "index must be within bounds of the array" - // Like FSharp.Core, count <= 0 removes nothing, and a source too short for index - // raises only once it has been enumerated let endIndex = index + count mkSeq (fun () -> diff --git a/src/fable-library-py/fable_library/seq_native.py b/src/fable-library-py/fable_library/seq_native.py index c9a3bae59e..cbdcdf2f9a 100644 --- a/src/fable-library-py/fable_library/seq_native.py +++ b/src/fable-library-py/fable_library/seq_native.py @@ -161,7 +161,6 @@ def gen() -> Iterator[T]: def take[T](count: int, xs: IEnumerable_1[T]) -> NativeSeq[T]: - # Checked when called, not when enumerated, like FSharp.Core if count < 0: raise ArgumentException__ctor_Z384F8060("The input must be non-negative.", "count") diff --git a/src/fable-library-ts/Array.fs b/src/fable-library-ts/Array.fs index 35bd0bb11c..d1e0ee51e8 100644 --- a/src/fable-library-ts/Array.fs +++ b/src/fable-library-ts/Array.fs @@ -368,8 +368,7 @@ let private argumentOutOfRange (paramName: string) (value: int) (requirement: st System.ArgumentOutOfRangeException(paramName, paramName + " ('" + string value + "') must be " + requirement) ) -// System.Array.Copy calls compile to this function too, so it checks the arguments like -// System.Array.Copy, in the same order, and copies the items itself like Util.copyToArray +// System.Array.Copy compiles to this function, so it cannot call System.Array.Copy itself let copyTo (source: 'T[]) (sourceIndex: int) (target: 'T[]) (targetIndex: int) (count: int) = if count < 0 then argumentOutOfRange "length" count "a non-negative value." @@ -393,7 +392,6 @@ let copyTo (source: 'T[]) (sourceIndex: int) (target: 'T[]) (targetIndex: int) ( if isTypedArrayImpl source && isTypedArrayImpl target then copyTypedArray (source :> obj) sourceIndex (target :> obj) targetIndex count elif sourceIndex < targetIndex then - // Backwards, so a copy within one array doesn't overwrite items before reading them for i = count - 1 downto 0 do arraySetItemImpl target (targetIndex + i) (arrayAccessImpl (sourceIndex + i) source) else @@ -1412,8 +1410,7 @@ let removeAt (index: int) (xs: 'T[]) : 'T[] = ) let removeManyAt (index: int) (count: int) (xs: 'T[]) : 'T[] = - // FSharp.Core's check. Negative counts, which FSharp.Core handles oddly, remove nothing - // but the index can't be past the end. + // Unlike FSharp.Core, a negative count removes nothing instead of duplicating items if index < 0 || index > xs.Length - count || index > xs.Length then invalidArg "index" "index must be within bounds of the array" diff --git a/src/fable-library-ts/List.fs b/src/fable-library-ts/List.fs index 5dda298daf..3d5b14dc1f 100644 --- a/src/fable-library-ts/List.fs +++ b/src/fable-library-ts/List.fs @@ -291,7 +291,6 @@ let fold2<'T1, 'T2, 'State> (folder: 'State -> 'T1 -> 'T2 -> 'State) (state: 'St xs <- xs.Tail ys <- ys.Tail - // Like FSharp.Core, name the list that ran out if not xs.IsEmpty then differentLengths "list2" elif not ys.IsEmpty then @@ -1028,7 +1027,6 @@ let removeManyAt (index: int) (count: int) (xs: 'T list) : 'T list = i < index || i >= endIndex ) - // i + 1 is now the length of the list if endIndex > i + 1 then invalidArg "index" "index must be within bounds of the list" diff --git a/src/fable-library-ts/Seq.fs b/src/fable-library-ts/Seq.fs index c37cf6da0f..cdad084f7f 100644 --- a/src/fable-library-ts/Seq.fs +++ b/src/fable-library-ts/Seq.fs @@ -1151,7 +1151,6 @@ let windowed (windowSize: int) (xs: seq<'T>) : 'T[] seq = delay (fun () -> xs |> toArray |> Array.windowed windowSize |> ofArray) -// Groups the items by index like FSharp.Core, so jagged input gives shorter rows instead of raising let transpose (xss: seq<#seq<'T>>) : seq> = delay (fun () -> let rows = ResizeArray>() @@ -1375,8 +1374,6 @@ let removeAt (index: int) (xs: seq<'T>) : seq<'T> = ) (fun e -> e.Dispose()) -// Like FSharp.Core: a count past the end (or a negative one) isn't an error, but the input -// must be longer than index, which can only be checked once it has been enumerated let removeManyAt (index: int) (count: int) (xs: seq<'T>) : seq<'T> = if index < 0 then invalidArg "index" "index must be within bounds of the array"