From 584aeade443652375ee7bc749a60ade99527f96c Mon Sep 17 00:00:00 2001 From: ncave <777696+ncave@users.noreply.github.com> Date: Sat, 9 Nov 2024 11:07:00 -0800 Subject: [PATCH] [JS/TS] Added missing IReadOnlyCollection helpers --- src/Fable.Cli/CHANGELOG.md | 1 + src/Fable.Transforms/Replacements.fs | 6 ++- src/Fable.Transforms/Rust/Fable2Rust.fs | 4 +- src/Fable.Transforms/Transforms.Util.fs | 3 ++ src/fable-library-rust/src/Exception.rs | 2 +- src/fable-library-rust/src/Native.rs | 11 +++- src/fable-library-ts/CollectionUtil.ts | 69 +++++++++++++++++-------- tests/Js/Main/ArrayTests.fs | 51 ++++++++++++++++-- tests/Js/Main/DictionaryTests.fs | 5 ++ tests/Js/Main/HashSetTests.fs | 5 ++ tests/Js/Main/MapTests.fs | 5 ++ tests/Js/Main/ResizeArrayTests.fs | 5 ++ tests/Js/Main/SetTests.fs | 5 ++ tests/Rust/Cargo.toml | 3 +- 14 files changed, 142 insertions(+), 33 deletions(-) diff --git a/src/Fable.Cli/CHANGELOG.md b/src/Fable.Cli/CHANGELOG.md index 06f7870d6..201f982bd 100644 --- a/src/Fable.Cli/CHANGELOG.md +++ b/src/Fable.Cli/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [Rust] Updated string comparisons (by @ncave) * [Rust] Fixed derived traits mapping (by @ncave) * [JS/TS] Added missing ICollection helpers (#3914) (by @ncave) +* [JS/TS] Added missing IReadOnlyCollection helpers (by @ncave) ## 4.23.0 - 2024-10-28 diff --git a/src/Fable.Transforms/Replacements.fs b/src/Fable.Transforms/Replacements.fs index 2ec0501a6..49d29118d 100644 --- a/src/Fable.Transforms/Replacements.fs +++ b/src/Fable.Transforms/Replacements.fs @@ -2697,6 +2697,7 @@ let collections (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (thisA | ("get_Count" | "get_IsReadOnly" | "Add" | "Remove" | "Clear" | "Contains" | "CopyTo") as meth, Some ar -> let meth = Naming.removeGetSetPrefix meth |> Naming.lowerFirst Helper.LibCall(com, "CollectionUtil", meth, t, ar :: args, ?loc = r) |> Some + | "GetEnumerator", Some callee -> getEnumerator com r t callee |> Some | _ -> None let conditionalWeakTable (com: ICompiler) (ctx: Context) r t (i: CallInfo) (thisArg: Expr option) (args: Expr list) = @@ -3983,8 +3984,6 @@ let private replacedModules = Types.conditionalWeakTable, conditionalWeakTable Types.ienumerableGeneric, enumerables Types.ienumerable, enumerables - Types.valueCollection, enumerables - Types.keyCollection, enumerables "System.Collections.Generic.Dictionary`2.Enumerator", enumerators "System.Collections.Generic.Dictionary`2.ValueCollection.Enumerator", enumerators "System.Collections.Generic.Dictionary`2.KeyCollection.Enumerator", enumerators @@ -3994,6 +3993,9 @@ let private replacedModules = Types.resizeArray, resizeArrays "System.Collections.Generic.IList`1", resizeArrays "System.Collections.IList", resizeArrays + Types.valueCollection, collections + Types.keyCollection, collections + Types.ireadonlycollection, collections Types.icollectionGeneric, collections Types.icollection, collections "System.Collections.Generic.CollectionExtensions", collectionExtensions diff --git a/src/Fable.Transforms/Rust/Fable2Rust.fs b/src/Fable.Transforms/Rust/Fable2Rust.fs index 1e0a252d2..c19125fbc 100644 --- a/src/Fable.Transforms/Rust/Fable2Rust.fs +++ b/src/Fable.Transforms/Rust/Fable2Rust.fs @@ -362,7 +362,7 @@ module TypeInfo = [ ty ] |> makeImportType com ctx "Native" "Arc" let makeBoxTy com ctx (ty: Rust.Ty) : Rust.Ty = - [ ty ] |> makeImportType com ctx "Native" "Box" + [ ty ] |> makeImportType com ctx "Native" (rawIdent "Box") let makeMutTy com ctx (ty: Rust.Ty) : Rust.Ty = [ ty ] |> makeImportType com ctx "Native" "MutCell" @@ -1511,7 +1511,7 @@ module Util = [ value ] |> makeNew com ctx "Native" "Arc" let makeBoxValue com ctx (value: Rust.Expr) = - [ value ] |> makeNew com ctx "Native" "Box" + [ value ] |> makeNew com ctx "Native" (rawIdent "Box") let makeMutValue com ctx (value: Rust.Expr) = [ value ] |> makeNew com ctx "Native" "MutCell" diff --git a/src/Fable.Transforms/Transforms.Util.fs b/src/Fable.Transforms/Transforms.Util.fs index fb7b93d57..db6ab8ab6 100644 --- a/src/Fable.Transforms/Transforms.Util.fs +++ b/src/Fable.Transforms/Transforms.Util.fs @@ -327,6 +327,9 @@ module Types = [] let idictionary = "System.Collections.Generic.IDictionary`2" + [] + let ireadonlycollection = "System.Collections.Generic.IReadOnlyCollection`1" + [] let ireadonlydictionary = "System.Collections.Generic.IReadOnlyDictionary`2" diff --git a/src/fable-library-rust/src/Exception.rs b/src/fable-library-rust/src/Exception.rs index c744dd17f..e9a35df83 100644 --- a/src/fable-library-rust/src/Exception.rs +++ b/src/fable-library-rust/src/Exception.rs @@ -1,5 +1,5 @@ pub mod Exception_ { - use crate::Native_::{Any, Box_, Func0, LrcPtr}; + use crate::Native_::{Any, Box, Func0, LrcPtr}; use crate::String_::{fromSlice, string}; use crate::System::Exception; use crate::Util_::new_Exception; diff --git a/src/fable-library-rust/src/Native.rs b/src/fable-library-rust/src/Native.rs index 1cd36dd0b..a6969b681 100644 --- a/src/fable-library-rust/src/Native.rs +++ b/src/fable-library-rust/src/Native.rs @@ -10,7 +10,7 @@ pub mod Native_ { // re-export at module level // pub use alloc::borrow::Cow; - pub use alloc::boxed::Box as Box_; + pub use alloc::boxed::Box; pub use alloc::rc::Rc; pub use alloc::string::{String, ToString}; pub use alloc::sync::Arc; @@ -163,6 +163,15 @@ pub mod Native_ { ) } + #[cfg(feature = "no_std")] + pub fn get_args(argc: isize, argv: *const *const u8) -> impl Iterator { + (0..argc as usize).map(move |i| unsafe { + let curr_argv = argv.add(i).read_volatile(); + let c_str = core::ffi::CStr::from_ptr(curr_argv as *const _); + c_str.to_str().unwrap() + }) + } + // ----------------------------------------------------------- // IEqualityComparer key wrapper // ----------------------------------------------------------- diff --git a/src/fable-library-ts/CollectionUtil.ts b/src/fable-library-ts/CollectionUtil.ts index 70f693809..207414436 100644 --- a/src/fable-library-ts/CollectionUtil.ts +++ b/src/fable-library-ts/CollectionUtil.ts @@ -4,17 +4,21 @@ export function count(col: Iterable): number { if (typeof (col as any)["System.Collections.Generic.ICollection`1.get_Count"] === "function") { return (col as any)["System.Collections.Generic.ICollection`1.get_Count"](); // collection } else { - if (isArrayLike(col)) { - return col.length; // resize array + if (typeof (col as any)["System.Collections.Generic.IReadOnlyCollection`1.get_Count"] === "function") { + return (col as any)["System.Collections.Generic.IReadOnlyCollection`1.get_Count"](); // collection } else { - if (typeof (col as any).size === "number") { - return (col as any).size; // map, set + if (isArrayLike(col)) { + return col.length; // array, resize array } else { - let count = 0; - for (const _ of col) { - count++; + if (typeof (col as any).size === "number") { + return (col as any).size; // map, set + } else { + let count = 0; + for (const _ of col) { + count++; + } + return count; // other collections } - return count; } } } @@ -24,7 +28,16 @@ export function isReadOnly(col: Iterable): boolean { if (typeof (col as any)["System.Collections.Generic.ICollection`1.get_IsReadOnly"] === "function") { return (col as any)["System.Collections.Generic.ICollection`1.get_IsReadOnly"](); // collection } else { - return false; + if (isArrayLike(col)) { + return ArrayBuffer.isView(col); // true for typed arrays, false for other arrays + } else { + if (typeof (col as any).size === "number") { + return false; // map, set + } else { + return true; // other collections + } + } + } } @@ -45,7 +58,7 @@ export function contains(col: Iterable, item: T): boolean { return (col as any)["System.Collections.Generic.ICollection`1.Contains2B595"](item); // collection } else { if (isArrayLike(col)) { - let i = col.findIndex(x => equals(x, item)); // resize array + let i = col.findIndex(x => equals(x, item)); // array, resize array return i >= 0; } else { if (typeof (col as any).has === "function") { @@ -55,7 +68,7 @@ export function contains(col: Iterable, item: T): boolean { return (col as any).has(item); // set } } else { - return false; // unknown collection + return false; // other collections } } } @@ -66,7 +79,11 @@ export function add(col: Iterable, item: T): void { return (col as any)["System.Collections.Generic.ICollection`1.Add2B595"](item); // collection } else { if (isArrayLike(col)) { - col.push(item); // resize array + if (ArrayBuffer.isView(col)) { + // TODO: throw for typed arrays? + } else { + col.push(item); // array, resize array + } } else { if (typeof (col as any).add === "function") { return (col as any).add(item); // set @@ -80,7 +97,7 @@ export function add(col: Iterable, item: T): void { throw new Error("An item with the same key has already been added. Key: " + item[0]); } } else { - // unknown collection + // TODO: throw for other collections? } } } @@ -92,12 +109,17 @@ export function remove(col: Iterable, item: T): boolean { return (col as any)["System.Collections.Generic.ICollection`1.Remove2B595"](item); // collection } else { if (isArrayLike(col)) { - let i = col.findIndex(x => equals(x, item)); - if (i >= 0) { - col.splice(i, 1); // resize array - return true; - } else { + if (ArrayBuffer.isView(col)) { + // TODO: throw for typed arrays return false; + } else { + let i = col.findIndex(x => equals(x, item)); + if (i >= 0) { + col.splice(i, 1); // array, resize array + return true; + } else { + return false; + } } } else { if (typeof (col as any).delete === "function") { @@ -111,7 +133,8 @@ export function remove(col: Iterable, item: T): boolean { return (col as any).delete(item); // set } } else { - return false; // unknown collection + // TODO: throw for other collections? + return false; // other collections } } } @@ -122,12 +145,16 @@ export function clear(col: Iterable): void { return (col as any)["System.Collections.Generic.ICollection`1.Clear"](); // collection } else { if (isArrayLike(col)) { - col.splice(0); // resize array + if (ArrayBuffer.isView(col)) { + // TODO: throw for typed arrays? + } else { + col.splice(0); // array, resize array + } } else { if (typeof (col as any).clear === "function") { (col as any).clear(); // map, set } else { - // unknown collection + // TODO: throw for other collections? } } } diff --git a/tests/Js/Main/ArrayTests.fs b/tests/Js/Main/ArrayTests.fs index dad94e33c..faf0752f2 100644 --- a/tests/Js/Main/ArrayTests.fs +++ b/tests/Js/Main/ArrayTests.fs @@ -69,7 +69,7 @@ let tests = ParamArrayTest.Add(let ar = [|1;2;3|] in sideEffect ar; ar) |> equal 9 -#if FABLE_COMPILER && FABLE_TYPED_ARRAYS +#if FABLE_COMPILER_JAVASCRIPT testCase "Typed Arrays work" <| fun () -> let xs = [| 1; 2; 3; |] let ys = [| 1.; 2.; 3.; |] @@ -1236,10 +1236,19 @@ let tests = System.Array.Resize(&xs, 0) xs |> equal [||] - // testCase "Array ICollection.IsReadOnly works" <| fun _ -> - // let xs = [| ("A", 1); ("B", 2); ("C", 3) |] - // let coll = xs :> ICollection<_> - // coll.IsReadOnly |> equal false + testCase "Array IReadOnlyCollection.Count works" <| fun _ -> + let xs = [| ("A", 1); ("B", 2); ("C", 3) |] + let coll = xs :> IReadOnlyCollection<_> + coll.Count |> equal 3 + + testCase "Array ICollection.IsReadOnly works" <| fun _ -> + let xs = [| ("A", 1); ("B", 2); ("C", 3) |] + let coll = xs :> ICollection<_> +#if FABLE_COMPILER_JAVASCRIPT || FABLE_COMPILER_TYPESCRIPT + coll.IsReadOnly |> equal false // Arrays are the same as ResizeArrays +#else + coll.IsReadOnly |> equal true +#endif testCase "Array ICollection.Count works" <| fun _ -> let xs = [| ("A", 1); ("B", 2); ("C", 3) |] @@ -1259,4 +1268,36 @@ let tests = let ys = [| ("D", 4); ("E", 5); ("F", 6) |] coll.CopyTo(ys, 0) ys = xs |> equal true + + testCase "Array IReadOnlyCollection.Count with typed arrays works" <| fun _ -> + let xs = [| 1; 2; 3 |] + let coll = xs :> IReadOnlyCollection<_> + coll.Count |> equal 3 + + testCase "Array ICollection.IsReadOnly with typed arrays works" <| fun _ -> + let xs = [| 1; 2; 3 |] + let coll = xs :> ICollection<_> +#if FABLE_COMPILER_TYPESCRIPT + coll.IsReadOnly |> equal false // Arrays are the same as ResizeArrays +#else + coll.IsReadOnly |> equal true +#endif + + testCase "Array ICollection.Count with typed arrays works" <| fun _ -> + let xs = [| 1; 2; 3 |] + let coll = xs :> ICollection<_> + coll.Count |> equal 3 + + testCase "Array ICollection.Contains with typed arrays works" <| fun _ -> + let xs = [| 1; 2; 3 |] + let coll = xs :> ICollection<_> + coll.Contains(4) |> equal false + coll.Contains(2) |> equal true + + testCase "Array ICollection.CopyTo with typed arrays works" <| fun _ -> + let xs = [| 1; 2; 3 |] + let coll = xs :> ICollection<_> + let ys = [| 4; 5; 6 |] + coll.CopyTo(ys, 0) + ys = xs |> equal true ] diff --git a/tests/Js/Main/DictionaryTests.fs b/tests/Js/Main/DictionaryTests.fs index d52eb5600..462d11be9 100644 --- a/tests/Js/Main/DictionaryTests.fs +++ b/tests/Js/Main/DictionaryTests.fs @@ -271,6 +271,11 @@ let tests = table.add "C" 3 table.Dic.Count |> equal 3 + testCase "Dictionary IReadOnlyCollection.Count works" <| fun _ -> + let xs = [| ("A", 1); ("B", 2); ("C", 3) |] |> Array.map KeyValuePair + let coll = (Dictionary xs) :> IReadOnlyCollection<_> + coll.Count |> equal 3 + testCase "Dictionary ICollection.IsReadOnly works" <| fun _ -> let xs = [| ("A", 1); ("B", 2); ("C", 3) |] |> Array.map KeyValuePair let coll = (Dictionary xs) :> ICollection> diff --git a/tests/Js/Main/HashSetTests.fs b/tests/Js/Main/HashSetTests.fs index 5c7393061..b8900bfff 100644 --- a/tests/Js/Main/HashSetTests.fs +++ b/tests/Js/Main/HashSetTests.fs @@ -222,6 +222,11 @@ let tests = apa.Contains ({ i = 5; s = "foo"}) |> equal true apa.Contains ({ i = 5; s = "fo"}) |> equal false + testCase "HashSet IReadOnlyCollection.Count works" <| fun _ -> + let xs = [| ("A", 1); ("B", 2); ("C", 3) |] + let coll = (HashSet xs) :> IReadOnlyCollection<_> + coll.Count |> equal 3 + testCase "HashSet ICollection.IsReadOnly works" <| fun _ -> let xs = [| ("A", 1); ("B", 2); ("C", 3) |] let coll = (HashSet xs) :> ICollection<_> diff --git a/tests/Js/Main/MapTests.fs b/tests/Js/Main/MapTests.fs index d98ca3eb6..eee708d35 100644 --- a/tests/Js/Main/MapTests.fs +++ b/tests/Js/Main/MapTests.fs @@ -292,6 +292,11 @@ let tests = |> Map.count |> equal 1 + testCase "Map IReadOnlyCollection.Count works" <| fun _ -> + let xs = [| ("A", 1); ("B", 2); ("C", 3) |] + let coll = (Map xs) :> IReadOnlyCollection<_> + coll.Count |> equal 3 + testCase "Map ICollection.IsReadOnly works" <| fun _ -> let xs = [| ("A", 1); ("B", 2); ("C", 3) |] let coll = (Map xs) :> ICollection<_> diff --git a/tests/Js/Main/ResizeArrayTests.fs b/tests/Js/Main/ResizeArrayTests.fs index 2abf9eb2b..8f389094b 100644 --- a/tests/Js/Main/ResizeArrayTests.fs +++ b/tests/Js/Main/ResizeArrayTests.fs @@ -319,6 +319,11 @@ let tests = xs.CopyTo(2, ys, 1, 2) ys |> equal [|5;3;4;8;9|] + testCase "ResizeArray IReadOnlyCollection.Count works" <| fun _ -> + let xs = [| ("A", 1); ("B", 2); ("C", 3) |] + let coll = (ResizeArray xs) :> IReadOnlyCollection<_> + coll.Count |> equal 3 + testCase "ResizeArray ICollection.IsReadOnly works" <| fun _ -> let xs = [| ("A", 1); ("B", 2); ("C", 3) |] let coll = (ResizeArray xs) :> ICollection<_> diff --git a/tests/Js/Main/SetTests.fs b/tests/Js/Main/SetTests.fs index fd70d1ffd..2f752a51e 100644 --- a/tests/Js/Main/SetTests.fs +++ b/tests/Js/Main/SetTests.fs @@ -292,6 +292,11 @@ let tests = largeSetA.IsProperSubsetOf(largeSetB) |> equal false largeSetA.IsSubsetOf(largeSetB) |> equal true + testCase "Set IReadOnlyCollection.Count works" <| fun _ -> + let xs = [| ("A", 1); ("B", 2); ("C", 3) |] + let coll = (Set xs) :> IReadOnlyCollection<_> + coll.Count |> equal 3 + testCase "Set ICollection.IsReadOnly works" <| fun _ -> let xs = [| ("A", 1); ("B", 2); ("C", 3) |] let coll = (Set xs) :> ICollection<_> diff --git a/tests/Rust/Cargo.toml b/tests/Rust/Cargo.toml index e54915436..410eafe4b 100644 --- a/tests/Rust/Cargo.toml +++ b/tests/Rust/Cargo.toml @@ -7,7 +7,8 @@ edition = "2021" no_std = ["fable_library_rust/no_std"] static_do_bindings = ["fable_library_rust/static_do_bindings"] threaded = ["fable_library_rust/threaded"] -# default = ["threaded"] # Uncomment when attempting to debug/use rust analyzer to switch to threaded mode +# default = ["no_std"] # Uncomment to default to "no_std" mode +# default = ["threaded"] # Uncomment to default to "threaded" mode [dependencies] fable_library_rust = { path = "./fable_modules/fable-library-rust" }