Merge pull request #226 from darklang/list-insert-at

Consistent edge-case index behaviour for Lists

rescript/src/TableclothList.ml

@@ -66,9 +66,21 @@ let any t ~f = List.exists f t
 
 let head l = Belt.List.head l
 
-let drop t ~count = Belt.List.drop t count |. Belt.Option.getWithDefault []
+let drop t ~count =
+  match Belt.List.drop t count with
+  | None ->
+      if count <= 0 then t else []
+  | Some v ->
+      v
+
+
+let take t ~count =
+  match Belt.List.take t count with
+  | None ->
+      if count <= 0 then [] else t
+  | Some v ->
+      v
 
-let take t ~count = Belt.List.take t count |. Belt.Option.getWithDefault []
 
 let initial l =
   match reverse l with [] -> None | _ :: rest -> Some (reverse rest)
@@ -110,20 +122,7 @@ let findIndex list ~f =
 
 let find_index = findIndex
 
-let splitAt t ~index =
-  if index < 0
-  then raise (Invalid_argument "List.splitAt called with negative index") ;
-  let rec loop front back i =
-    match back with
-    | [] ->
-        (t, [])
-    | element :: rest ->
-        if i = 0
-        then (reverse front, back)
-        else loop (element :: front) rest (i - 1)
-  in
-  loop [] t index
-
+let splitAt t ~index = (take ~count:index t, drop ~count:index t)
 
 let split_at = splitAt
 
@@ -262,18 +261,8 @@ let rec groupWhile t ~f =
 let group_while = groupWhile
 
 let insertAt t ~index ~value =
-  if index < 0
-  then raise (Invalid_argument "List.splitAt called with negative index") ;
-  let rec loop front back i =
-    match back with
-    | [] ->
-        reverse (value :: front)
-    | element :: rest ->
-        if i = 0
-        then append (reverse front) (value :: element :: rest)
-        else loop (element :: front) rest (index - 1)
-  in
-  loop [] t index
+  let front, back = splitAt t ~index in
+  append front (value :: back)
 
 
 let insert_at = insertAt

rescript/src/TableclothList.mli

@@ -145,15 +145,19 @@ val cons : 'a t -> 'a -> 'a t
 val take : 'a t -> count:int -> 'a t
 (** Attempt to take the first [count] elements of a list.
 
-   If the list has fewer than [count] elements, returns [None].
+   If the list has fewer than [count] elements, returns the entire list.
+
+   If count is zero or negative, returns [].
 
    {2 Examples}
 
-   {[List.take [1;2;3] ~count:2 = Some [1;2]]}
+   {[List.take [1;2;3] ~count:2 = [1;2]]}
+
+   {[List.take [] ~count:2 = []]}
 
-   {[List.take [] ~count:2 = None]}
+   {[List.take [1;2;3;4] ~count:8 = [1;2;3;4]]}
 
-   {[List.take [1;2;3;4] ~count:8 = None]}
+   {[List.take [1;2;3;4] ~count:(-1) = []]}
 *)
 
 val takeWhile : 'a t -> f:('a -> bool) -> 'a t
@@ -173,11 +177,17 @@ val take_while : 'a t -> f:('a -> bool) -> 'a t
 val drop : 'a t -> count:int -> 'a t
 (** Drop the first [count] elements from the front of a list.
 
+    If the list has fewer than [count] elements, returns [].
+
+    If count is zero or negative, returns the entire list.
+
     {2 Examples}
 
     {[List.drop [1;2;3;4] ~count:2 = [3;4]]}
 
     {[List.drop [1;2;3;4] ~count:6 = []]}
+
+    {[List.drop [1;2;3;4] ~count:-1 = [1;2;3;4]]}
 *)
 
 val dropWhile : 'a t -> f:('a -> bool) -> 'a t
@@ -736,11 +746,17 @@ val splitAt : 'a t -> index:int -> 'a t * 'a t
 
     Elements with an index greater than or equal to [index] will be in the second.
 
-    If [index] is outside of the bounds of the list, all elements will be in the first component of the tuple.
+    If [index] is zero or negative, all elements will be in the second component of the tuple.
+
+    If [index] is greater than the length of the list, all elements will be in the second component of the tuple.
 
     {2 Examples}
 
     {[List.splitAt [1;2;3;4;5] ~index:2 = ([1;2], [3;4;5])]}
+
+    {[List.splitAt [1;2;3;4;5] ~index:-1 = ([], [1;2;3;4;5])]}
+
+    {[List.splitAt [1;2;3;4;5] ~index:10 = ([1;2;3;4;5], 10)]}
 *)
 
 val split_at : 'a t -> index:int -> 'a t * 'a t

rescript/test/ListTest.re

@@ -16,24 +16,46 @@ let suite =
       test("from an empty list", () =>
         expect(drop([], ~count=1)) |> toEqual(Eq.(list(int)), [])
       );
-
       test("zero elements", () =>
         expect(drop([1, 2, 3], ~count=0))
         |> toEqual(Eq.(list(int)), [1, 2, 3])
       );
-
       test("the first element", () =>
         expect(drop([1, 2, 3], ~count=1))
         |> toEqual(Eq.(list(int)), [2, 3])
       );
-
       test("all elements", () =>
         expect(drop([1, 2, 3], ~count=3)) |> toEqual(Eq.(list(int)), [])
       );
-
       test("greater than the number of elements", () =>
         expect(drop([1, 2, 3], ~count=4)) |> toEqual(Eq.(list(int)), [])
       );
+      test("negative count", () =>
+        expect(drop([1, 2, 3], ~count=-1))
+        |> toEqual(Eq.(list(int)), [1, 2, 3])
+      );
+      test("zero count", () =>
+        expect(drop([1, 2, 3], ~count=-1))
+        |> toEqual(Eq.(list(int)), [1, 2, 3])
+      );
+    });
+
+    describe("take", () => {
+      test("normal", () =>
+        expect(take([1, 2, 3], ~count=2))
+        |> toEqual(Eq.(list(int)), [1, 2])
+      );
+      test("from an empty list", () =>
+        expect(take([], ~count=2)) |> toEqual(Eq.(list(int)), [])
+      );
+      test("overflow", () =>
+        expect(take([1, 2, 3, 4], ~count=8))
+        |> toEqual(Eq.(list(int)), [1, 2, 3, 4])
+      );
+      test("overflow", () =>
+        expect(take([1, 2, 3, 4], ~count=-1))
+        |> toEqual(Eq.(list(int)), [])
+      );
     });
 
     describe("findIndex", () => {
@@ -201,11 +223,14 @@ let suite =
         expect(splitAt(~index=3, [1, 3, 5]))
         |> toEqual(Eq.(pair(list(int), list(int))), ([1, 3, 5], []))
       });
-
       test("past end", () => {
         expect(splitAt(~index=6, [1, 3, 5]))
         |> toEqual(Eq.(pair(list(int), list(int))), ([1, 3, 5], []))
       });
+      test("negative", () => {
+        expect(splitAt(~index=-1, [1, 3, 5]))
+        |> toEqual(Eq.(pair(list(int), list(int))), ([], [1, 3, 5]))
+      });
     });
 
     describe("splitWhen", () => {
@@ -310,7 +335,6 @@ let suite =
         |> toEqual(Eq.int, -6)
       });
     });
-
     describe("insertAt", () => {
       test("empty list", () => {
         expect(insertAt(~index=0, ~value=1, []))
@@ -324,10 +348,33 @@ let suite =
         expect(insertAt(~index=0, ~value=2, [1, 3]))
         |> toEqual(Eq.(list(int)), [2, 1, 3])
       });
-
       test("after end of list", () => {
-        expect(insertAt(~index=4, ~value=2, [1, 3]) |> toArray)
-        |> toEqual(Eq.(array(int)), [|1, 3, 2|])
+        expect(insertAt(~index=4, ~value=2, [1, 3]))
+        |> toEqual(Eq.(list(int)), [1, 3, 2])
+      });
+      test("#216", () => {
+        expect(insertAt(~index=5, ~value=1, [0, 2, 3, 4, 5, 6, 7, 8, 9]))
+        |> toEqual(Eq.(list(int)), [0, 2, 3, 4, 5, 1, 6, 7, 8, 9])
+      });
+      test("doc 1", () => {
+        expect(insertAt(~index=2, ~value=999, [100, 101, 102, 103]))
+        |> toEqual(Eq.(list(int)), [100, 101, 999, 102, 103])
+      });
+      test("doc 2", () => {
+        expect(insertAt(~index=0, ~value=999, [100, 101, 102, 103]))
+        |> toEqual(Eq.(list(int)), [999, 100, 101, 102, 103])
+      });
+      test("doc 3", () => {
+        expect(insertAt(~index=4, ~value=999, [100, 101, 102, 103]))
+        |> toEqual(Eq.(list(int)), [100, 101, 102, 103, 999])
+      });
+      test("doc 4", () => {
+        expect(insertAt(~index=-1, ~value=999, [100, 101, 102, 103]))
+        |> toEqual(Eq.(list(int)), [999, 100, 101, 102, 103])
+      });
+      test("doc 5", () => {
+        expect(insertAt(~index=5, ~value=999, [100, 101, 102, 103]))
+        |> toEqual(Eq.(list(int)), [100, 101, 102, 103, 999])
       });
     });