From 67d1b75dce110b35a9f0fd886cf33c48285cb83a Mon Sep 17 00:00:00 2001 From: gusty <1261319+gusty@users.noreply.github.com> Date: Tue, 21 Feb 2023 12:33:47 +0100 Subject: [PATCH] Fix bug in foldi for Map --- src/FSharpPlus/Control/Indexable.fs | 2 +- .../FSharpPlus.Tests/FSharpPlus.Tests.fsproj | 1 + tests/FSharpPlus.Tests/General.fs | 42 ---------- tests/FSharpPlus.Tests/Indexables.fs | 80 +++++++++++++++++++ tests/FSharpPlus.Tests/Traversals.fs | 5 -- 5 files changed, 82 insertions(+), 48 deletions(-) create mode 100644 tests/FSharpPlus.Tests/Indexables.fs diff --git a/src/FSharpPlus/Control/Indexable.fs b/src/FSharpPlus/Control/Indexable.fs index 58ed36c82..dbf88fdda 100644 --- a/src/FSharpPlus/Control/Indexable.fs +++ b/src/FSharpPlus/Control/Indexable.fs @@ -123,7 +123,7 @@ type FoldIndexed = inherit Default1 static member FoldIndexed (x: list<_> , f, z, _impl: FoldIndexed) = x |> List.fold (fun (p, i) t -> (f p i t, i + 1)) (z, 0) |> fst static member FoldIndexed (x: _ [] , f, z, _impl: FoldIndexed) = x |> Array.fold (fun (p, i) t -> (f p i t, i + 1)) (z, 0) |> fst - static member FoldIndexed (_: Map<'k,'t>, f, z, _impl: FoldIndexed) = Map.fold f z + static member FoldIndexed (x: Map<'k,'t>, f, z, _impl: FoldIndexed) = Map.fold f z x static member inline Invoke (folder: 'State->'Key->'T->'State) (state: 'State) (foldable: '``Foldable<'T>``) : 'State = let inline call_2 (a: ^a, b: ^b, f, z) = ((^a or ^b) : (static member FoldIndexed : _*_*_*_ -> _) b, f, z, a) diff --git a/tests/FSharpPlus.Tests/FSharpPlus.Tests.fsproj b/tests/FSharpPlus.Tests/FSharpPlus.Tests.fsproj index d842f6d24..46ee59d92 100644 --- a/tests/FSharpPlus.Tests/FSharpPlus.Tests.fsproj +++ b/tests/FSharpPlus.Tests/FSharpPlus.Tests.fsproj @@ -21,6 +21,7 @@ + diff --git a/tests/FSharpPlus.Tests/General.fs b/tests/FSharpPlus.Tests/General.fs index 819275b79..7f26ffd7b 100644 --- a/tests/FSharpPlus.Tests/General.fs +++ b/tests/FSharpPlus.Tests/General.fs @@ -915,48 +915,6 @@ module Foldable = areEqual sb' None () -module Indexable = - [] - let testCompileAndExecuteItem () = - - let a = Map.ofSeq [1, "one"; 2, "two"] - let _ = item 1 a - - let b = dict [1, "one"; 2, "two"] - let _ = item 1 b - - let c = "two" - let _ = item 1 c - - let d = System.Text.StringBuilder "one" - let _ = item 1 d - - let e = array2D [[1;2];[3;4];[5;6]] - let _ = item (1, 1) e - - let f = [1, "one"; 2, "two"] - let _ = item 1 f - - let g = [|1, "one"; 2, "two"|] - let _ = item 1 g - - let h = ResizeArray [1, "one"; 2, "two"] - let _ = item 1 h - - let i = Array3D.create 3 2 2 0 - let _ = item (1, 1, 1) i - - let j = Array4D.create 3 2 2 3 0 - let _ = item (1, 1, 1, 1) j - - let k = NonEmptyMap.Create (("a", 1), ("b", 2)) - let _ = item "b" k - - // This doesn't intentionally compile: seq is not Indexable. Not all foldables are Indexable, for example a Set is foldable but not Indexable. For seq use nth instead. - // let f = seq [1, "one"; 2, "two"] - // let _ = item 1 f - - () [] let testCompileAndExecuteTryItem () = diff --git a/tests/FSharpPlus.Tests/Indexables.fs b/tests/FSharpPlus.Tests/Indexables.fs new file mode 100644 index 000000000..b1238fc40 --- /dev/null +++ b/tests/FSharpPlus.Tests/Indexables.fs @@ -0,0 +1,80 @@ +namespace FSharpPlus.Tests + +#nowarn "686" + +open System +open System.Collections.Generic +open FSharpPlus +open FSharpPlus.Data +open FSharpPlus.Control +open NUnit.Framework +open Helpers + + +module Indexables = + + [] + let testCompileAndExecuteItem () = + + let a = Map.ofSeq [1, "one"; 2, "two"] + let _ = item 1 a + + let b = dict [1, "one"; 2, "two"] + let _ = item 1 b + + let c = "two" + let _ = item 1 c + + let d = System.Text.StringBuilder "one" + let _ = item 1 d + + let e = array2D [[1;2];[3;4];[5;6]] + let _ = item (1, 1) e + + let f = [1, "one"; 2, "two"] + let _ = item 1 f + + let g = [|1, "one"; 2, "two"|] + let _ = item 1 g + + let h = ResizeArray [1, "one"; 2, "two"] + let _ = item 1 h + + let i = Array3D.create 3 2 2 0 + let _ = item (1, 1, 1) i + + let j = Array4D.create 3 2 2 3 0 + let _ = item (1, 1, 1, 1) j + + let k = NonEmptyMap.Create (("a", 1), ("b", 2)) + let _ = item "b" k + + // This doesn't intentionally compile: seq is not Indexable. Not all foldables are Indexable, for example a Set is foldable but not Indexable. For seq use nth instead. + // let f = seq [1, "one"; 2, "two"] + // let _ = item 1 f + + () + + [] + let foldIndexed () = + Assert.AreEqual (123, foldi (fun a b c -> a + b + c) 0 (seq [20; 40; 60])) + Assert.AreEqual (123, foldi (fun a b c -> a + b + c) 0 [20; 40; 60]) + Assert.AreEqual (123, foldi (fun a b c -> a + b + c) 0 (Map.ofSeq [0, 20; 1, 40; 2, 60])) + + + [] + let traverseIndexed () = + let m1 = Map.ofList [(1, [1;1;1]); (2, [2;2;2])] + let r1 = m1 |> traversei (fun _ _ -> None) + let r2 = m1 |> traversei (fun i v -> if List.forall ((=) i) v then Some (i :: v) else None) + Assert.AreEqual (None, r1) + CollectionAssert.AreEqual (Map.ofList [(1, [1;1;1;1]); (2, [2;2;2;2])], r2.Value) + Assert.IsInstanceOf>> (Some r2.Value) + + let r3 = seq [ ( [0;0;0]); ( [1;1;1]); ( [2;2;2])] |> traversei (fun i v -> if List.forall ((=) i) v then Some (i :: v) else None) + CollectionAssert.AreEqual (seq [[0; 0; 0]; [1; 1; 1]; [2; 2; 2]], r3.Value) + Assert.IsInstanceOf>> (Some r3.Value) + + let r4 = [ ( [0;0;0]); ( [1;1;1]); ( [2;2;2])] |> traversei (fun i v -> if List.forall ((=) i) v then Some (i :: v) else None) + CollectionAssert.AreEqual ([[0; 0; 0]; [1; 1; 1]; [2; 2; 2]], r4.Value) + Assert.IsInstanceOf> (Some r4.Value) \ No newline at end of file diff --git a/tests/FSharpPlus.Tests/Traversals.fs b/tests/FSharpPlus.Tests/Traversals.fs index bd216e4b5..6f0265226 100644 --- a/tests/FSharpPlus.Tests/Traversals.fs +++ b/tests/FSharpPlus.Tests/Traversals.fs @@ -273,11 +273,6 @@ module Traversable = CollectionAssert.AreEqual (r2.Value, m) let m1 = Map.ofList [(1, [1;1;1]); (2, [2;2;2])] - let r1 = m1 |> traversei (fun _ _ -> None) - let r2 = m1 |> traversei (fun i v -> if List.forall ((=) i) v then Some (i :: v) else None) - Assert.AreEqual(None, r1) - CollectionAssert.AreEqual (Map.ofList [(1, [1;1;1;1]); (2, [2;2;2;2])], r2.Value) - let expected = [Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]; Map.ofList [(1, 1); (2, 2)]]