From 1d2fa816a08546bd2a342287a4c9291f5ed93ecd Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Mon, 10 Mar 2025 20:48:47 +0400 Subject: [PATCH 1/7] Add Array.removeInPlace --- runtime/Stdlib_Array.res | 3 + runtime/Stdlib_Array.resi | 3 + tests/tests/src/core/Core_ArrayTests.mjs | 73 ++++++++++++++++++++++++ tests/tests/src/core/Core_ArrayTests.res | 33 +++++++++++ 4 files changed, 112 insertions(+) diff --git a/runtime/Stdlib_Array.res b/runtime/Stdlib_Array.res index 011d1cf564..d6536de0ba 100644 --- a/runtime/Stdlib_Array.res +++ b/runtime/Stdlib_Array.res @@ -108,6 +108,9 @@ external splice: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => u external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => array<'a> = "toSpliced" +@send +external removeInPlace: (array<'a>, int, @as(1) _) => unit = "splice" + @send external with: (array<'a>, int, 'a) => array<'a> = "with" @send external unshift: (array<'a>, 'a) => unit = "unshift" diff --git a/runtime/Stdlib_Array.resi b/runtime/Stdlib_Array.resi index e2bb72b1e6..707fcf44c8 100644 --- a/runtime/Stdlib_Array.resi +++ b/runtime/Stdlib_Array.resi @@ -302,6 +302,9 @@ external splice: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => u external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => array<'a> = "toSpliced" +@send +external removeInPlace: (array<'a>, int, @as(1) _) => unit = "splice" + @send external with: (array<'a>, int, 'a) => array<'a> = "with" /** diff --git a/tests/tests/src/core/Core_ArrayTests.mjs b/tests/tests/src/core/Core_ArrayTests.mjs index 9f528d119c..24992a5775 100644 --- a/tests/tests/src/core/Core_ArrayTests.mjs +++ b/tests/tests/src/core/Core_ArrayTests.mjs @@ -457,6 +457,79 @@ Test.run([ "last - empty" ], Stdlib_Array.last([]), eq, undefined); +let array = []; + +array.splice(1, 0, "foo"); + +Test.run([ + [ + "Core_ArrayTests.res", + 116, + 22, + 49 + ], + "splice - Insert no delete" +], array, eq, ["foo"]); + +let array$1 = [ + "bar", + "baz" +]; + +Test.run([ + [ + "Core_ArrayTests.res", + 122, + 15, + 43 + ], + "splice - Insert and delete" +], [ + (array$1.splice(1, 1, "foo"), undefined), + array$1 +], eq, [ + undefined, + [ + "bar", + "foo" + ] +]); + +let array$2 = []; + +array$2.splice(0, 1); + +Test.run([ + [ + "Core_ArrayTests.res", + 132, + 22, + 45 + ], + "removeInPlace - empty" +], array$2, eq, []); + +let array$3 = [ + "Hello", + "Hi", + "Good bye" +]; + +array$3.splice(1, 1); + +Test.run([ + [ + "Core_ArrayTests.res", + 138, + 22, + 51 + ], + "removeInPlace - from middle" +], array$3, eq, [ + "Hello", + "Good bye" +]); + export { eq, } diff --git a/tests/tests/src/core/Core_ArrayTests.res b/tests/tests/src/core/Core_ArrayTests.res index 01fdf39c34..3880faf4c4 100644 --- a/tests/tests/src/core/Core_ArrayTests.res +++ b/tests/tests/src/core/Core_ArrayTests.res @@ -109,3 +109,36 @@ Test.run( Test.run(__POS_OF__("last - with items"), [1, 2, 3]->Array.last, eq, Some(3)) Test.run(__POS_OF__("last - empty"), []->Array.last, eq, None) + +{ + let array = [] + array->Array.splice(~start=1, ~remove=0, ~insert=["foo"]) + Test.run(__POS_OF__("splice - Insert no delete"), array, eq, ["foo"]) +} + +{ + let array = ["bar", "baz"] + Test.run( + __POS_OF__("splice - Insert and delete"), + (array->Array.splice(~start=1, ~remove=1, ~insert=["foo"]), array), + eq, + ( + // Even though original .splice returns an array with the removed items, + // the binding returns unit so there's no confusion about it mutating the original array. + (), + ["bar", "foo"], + ), + ) +} + +{ + let array = [] + array->Array.removeInPlace(0) + Test.run(__POS_OF__("removeInPlace - empty"), array, eq, []) +} + +{ + let array = ["Hello", "Hi", "Good bye"] + array->Array.removeInPlace(1) + Test.run(__POS_OF__("removeInPlace - from middle"), array, eq, ["Hello", "Good bye"]) +} From 7d8f261c9a63e388f465045d726fa6fa0fa949a1 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Mon, 10 Mar 2025 20:49:53 +0400 Subject: [PATCH 2/7] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5b3c14401..b1ada7c4ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Add `Dict.has` and double `Dict.forEachWithKey`/`Dict.mapValues` performance. https://github.com/rescript-lang/rescript/pull/7316 - Add popover attributes to JsxDOM.domProps. https://github.com/rescript-lang/rescript/pull/7317 +- Add `Array.removeInPlace` helper based on `splice`. https://github.com/rescript-lang/rescript/pull/7321 #### :house: Internal From 8fb2a5b229c26cd6a365125a02bfba5c8231297d Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Wed, 12 Mar 2025 21:47:43 +0400 Subject: [PATCH 3/7] Add documentation --- runtime/Stdlib_Array.resi | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/runtime/Stdlib_Array.resi b/runtime/Stdlib_Array.resi index 707fcf44c8..aba4d2f4fd 100644 --- a/runtime/Stdlib_Array.resi +++ b/runtime/Stdlib_Array.resi @@ -302,6 +302,23 @@ external splice: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => u external toSpliced: (array<'a>, ~start: int, ~remove: int, ~insert: array<'a>) => array<'a> = "toSpliced" +/** +`removeInPlace(array, index)` removes the item at the specified `index` from `array`. + +Beware this will *mutate* the array. + +## Examples + +```rescript +let array = [] +array->Array.removeInPlace(0) +assertEqual(array, []) // Removing from an empty array does nothing + +let array2 = ["Hello", "Hi", "Good bye"] +array2->Array.removeInPlace(1) +assertEqual(array2, ["Hello", "Good bye"]) // Removes the item at index 1 +``` + */ @send external removeInPlace: (array<'a>, int, @as(1) _) => unit = "splice" From 245a57f10c2228d8a0ad58a82a0c77a22e5dcb91 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Fri, 14 Mar 2025 12:07:09 +0400 Subject: [PATCH 4/7] Commit new tests output --- tests/tests/src/core/Core_ArrayTests.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests/src/core/Core_ArrayTests.mjs b/tests/tests/src/core/Core_ArrayTests.mjs index 24992a5775..c652c8677a 100644 --- a/tests/tests/src/core/Core_ArrayTests.mjs +++ b/tests/tests/src/core/Core_ArrayTests.mjs @@ -502,7 +502,7 @@ array$2.splice(0, 1); Test.run([ [ "Core_ArrayTests.res", - 132, + 137, 22, 45 ], @@ -520,7 +520,7 @@ array$3.splice(1, 1); Test.run([ [ "Core_ArrayTests.res", - 138, + 143, 22, 51 ], From 81db867e3f9a886ee7577ab6d0e89329b75e7d41 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Fri, 14 Mar 2025 12:17:31 +0400 Subject: [PATCH 5/7] Fix analysis test --- tests/analysis_tests/tests/src/expected/Completion.res.txt | 6 ++++++ tests/analysis_tests/tests/test.sh | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/analysis_tests/tests/src/expected/Completion.res.txt b/tests/analysis_tests/tests/src/expected/Completion.res.txt index d8fafb4a77..db60cbe0e2 100644 --- a/tests/analysis_tests/tests/src/expected/Completion.res.txt +++ b/tests/analysis_tests/tests/src/expected/Completion.res.txt @@ -508,6 +508,12 @@ Path Array. "tags": [], "detail": "(array<'a>, int) => option<'a>", "documentation": {"kind": "markdown", "value": "\n`get(array, index)` returns the element at `index` of `array`.\n\nReturns `None` if the index does not exist in the array. Equivalent to doing `array[index]` in JavaScript.\n\n## Examples\n\n```rescript\nlet array = [\"Hello\", \"Hi\", \"Good bye\"]\n\narray\n->Array.get(0)\n->assertEqual(Some(\"Hello\"))\n\narray\n->Array.get(3)\n->assertEqual(None)\n```\n"} + }, { + "label": "removeInPlace", + "kind": 12, + "tags": [], + "detail": "(array<'a>, int) => unit", + "documentation": {"kind": "markdown", "value": "\n`removeInPlace(array, index)` removes the item at the specified `index` from `array`.\n\nBeware this will *mutate* the array.\n\n## Examples\n\n```rescript\nlet array = []\narray->Array.removeInPlace(0)\nassertEqual(array, []) // Removing from an empty array does nothing\n\nlet array2 = [\"Hello\", \"Hi\", \"Good bye\"]\narray2->Array.removeInPlace(1)\nassertEqual(array2, [\"Hello\", \"Good bye\"]) // Removes the item at index 1\n```\n "} }, { "label": "pushMany", "kind": 12, diff --git a/tests/analysis_tests/tests/test.sh b/tests/analysis_tests/tests/test.sh index a30bc3af9d..3564ce6176 100755 --- a/tests/analysis_tests/tests/test.sh +++ b/tests/analysis_tests/tests/test.sh @@ -22,9 +22,9 @@ reset='\033[0m' diff=$(git ls-files --modified src/expected) if [[ $diff = "" ]]; then - printf "${successGreen}✅ No unstaged tests difference.${reset}\n" + printf "${successGreen}✅ No analysis_tests snapshot changes detected.${reset}\n" else - printf "${warningYellow}⚠️ There are unstaged differences in tests/! Did you break a test?\n${diff}\n${reset}" + printf "${warningYellow}⚠️ The analysis_tests snapshot doesn't match. Double check that the output is correct and run 'make analysis_tests' if you get the error in CI\n${diff}\n${reset}" git --no-pager diff src/expected exit 1 fi From 98477844a9279e91e0d643223b49a6762801c233 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Fri, 14 Mar 2025 20:13:03 +0400 Subject: [PATCH 6/7] Update tests/analysis_tests/tests/test.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paul Tsnobiladzé --- tests/analysis_tests/tests/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/analysis_tests/tests/test.sh b/tests/analysis_tests/tests/test.sh index 3564ce6176..0d57d897aa 100755 --- a/tests/analysis_tests/tests/test.sh +++ b/tests/analysis_tests/tests/test.sh @@ -24,7 +24,7 @@ diff=$(git ls-files --modified src/expected) if [[ $diff = "" ]]; then printf "${successGreen}✅ No analysis_tests snapshot changes detected.${reset}\n" else - printf "${warningYellow}⚠️ The analysis_tests snapshot doesn't match. Double check that the output is correct and run 'make analysis_tests' if you get the error in CI\n${diff}\n${reset}" + printf "${warningYellow}⚠️ The analysis_tests snapshot doesn't match. Double check that the output is correct, run 'make analysis_tests' and stage the diff.\n${diff}\n${reset}" git --no-pager diff src/expected exit 1 fi From 3e501a16c5f214046c22706d553c5632b112f085 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Sun, 16 Mar 2025 14:11:29 +0400 Subject: [PATCH 7/7] Remove duplicated tests --- tests/tests/src/core/Core_ArrayTests.mjs | 35 ------------------------ tests/tests/src/core/Core_ArrayTests.res | 12 -------- 2 files changed, 47 deletions(-) diff --git a/tests/tests/src/core/Core_ArrayTests.mjs b/tests/tests/src/core/Core_ArrayTests.mjs index c652c8677a..88de742280 100644 --- a/tests/tests/src/core/Core_ArrayTests.mjs +++ b/tests/tests/src/core/Core_ArrayTests.mjs @@ -495,41 +495,6 @@ Test.run([ ] ]); -let array$2 = []; - -array$2.splice(0, 1); - -Test.run([ - [ - "Core_ArrayTests.res", - 137, - 22, - 45 - ], - "removeInPlace - empty" -], array$2, eq, []); - -let array$3 = [ - "Hello", - "Hi", - "Good bye" -]; - -array$3.splice(1, 1); - -Test.run([ - [ - "Core_ArrayTests.res", - 143, - 22, - 51 - ], - "removeInPlace - from middle" -], array$3, eq, [ - "Hello", - "Good bye" -]); - export { eq, } diff --git a/tests/tests/src/core/Core_ArrayTests.res b/tests/tests/src/core/Core_ArrayTests.res index 3880faf4c4..2ec0031dd0 100644 --- a/tests/tests/src/core/Core_ArrayTests.res +++ b/tests/tests/src/core/Core_ArrayTests.res @@ -130,15 +130,3 @@ Test.run(__POS_OF__("last - empty"), []->Array.last, eq, None) ), ) } - -{ - let array = [] - array->Array.removeInPlace(0) - Test.run(__POS_OF__("removeInPlace - empty"), array, eq, []) -} - -{ - let array = ["Hello", "Hi", "Good bye"] - array->Array.removeInPlace(1) - Test.run(__POS_OF__("removeInPlace - from middle"), array, eq, ["Hello", "Good bye"]) -}