Merge pull request #970 from camunda/957-remove

fix: List functions handle invalid zero position
camunda · Jan 21, 2025 · 970b8d5 · 970b8d5
2 parents 41be554 + 6bf385f
commit 970b8d5
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 23 deletions.
diff --git a/src/main/scala/org/camunda/feel/impl/builtin/ListBuiltinFunctions.scala b/src/main/scala/org/camunda/feel/impl/builtin/ListBuiltinFunctions.scala
@@ -275,20 +275,26 @@ class ListBuiltinFunctions(private val valueMapper: ValueMapper) {
   private def sublistFunction =
     builtinFunction(
       params = List("list", "start"),
-      invoke = { case List(ValList(list), ValNumber(start)) =>
-        ValList(list.slice(listIndex(list, start.intValue), list.length))
+      invoke = {
+        case List(ValList(_), ValNumber(start)) if start == 0 =>
+          ValError("start position must be a non-zero number")
+        case List(ValList(list), ValNumber(start))            =>
+          ValList(list.slice(listIndex(list, start.intValue), list.length))
       }
     )
 
   private def sublistFunction3 = builtinFunction(
     params = List("list", "start", "length"),
-    invoke = { case List(ValList(list), ValNumber(start), ValNumber(length)) =>
-      ValList(
-        list.slice(
-          listIndex(list, start.intValue),
-          listIndex(list, start.intValue) + length.intValue
+    invoke = {
+      case List(ValList(_), ValNumber(start), ValNumber(_)) if start == 0 =>
+        ValError("start position must be a non-zero number")
+      case List(ValList(list), ValNumber(start), ValNumber(length))       =>
+        ValList(
+          list.slice(
+            listIndex(list, start.intValue),
+            listIndex(list, start.intValue) + length.intValue
+          )
         )
-      )
     }
   )
 
@@ -325,23 +331,29 @@ class ListBuiltinFunctions(private val valueMapper: ValueMapper) {
 
   private def insertBeforeFunction = builtinFunction(
     params = List("list", "position", "newItem"),
-    invoke = { case List(ValList(list), ValNumber(position), newItem: Val) =>
-      ValList(
-        list
-          .take(listIndex(list, position.intValue)) ++ (newItem :: Nil) ++ list
-          .drop(listIndex(list, position.intValue))
-      )
+    invoke = {
+      case List(ValList(_), ValNumber(position), _) if position == 0 =>
+        ValError("position must be a non-zero number")
+      case List(ValList(list), ValNumber(position), newItem: Val)    =>
+        ValList(
+          list
+            .take(listIndex(list, position.intValue)) ++ (newItem :: Nil) ++ list
+            .drop(listIndex(list, position.intValue))
+        )
     }
   )
 
   private def removeFunction = builtinFunction(
     params = List("list", "position"),
-    invoke = { case List(ValList(list), ValNumber(position)) =>
-      ValList(
-        list.take(listIndex(list, position.intValue)) ++ list.drop(
-          listIndex(list, position.intValue + 1)
+    invoke = {
+      case List(ValList(_), ValNumber(position)) if position == 0 =>
+        ValError("position must be a non-zero number")
+      case List(ValList(list), ValNumber(position))               =>
+        ValList(
+          list.take(listIndex(list, position.intValue)) ++ list.drop(
+            listIndex(list, position.intValue + 1)
+          )
         )
-      )
     }
   )
 

diff --git a/src/test/scala/org/camunda/feel/impl/builtin/BuiltinListFunctionsTest.scala b/src/test/scala/org/camunda/feel/impl/builtin/BuiltinListFunctionsTest.scala
@@ -18,13 +18,10 @@ package org.camunda.feel.impl.builtin
 
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.flatspec.AnyFlatSpec
-import org.camunda.feel._
 import org.camunda.feel.api.EvaluationFailureType.FUNCTION_INVOCATION_FAILURE
-import org.camunda.feel.impl.{EvaluationResultMatchers, FeelEngineTest, FeelIntegrationTest}
-import org.camunda.feel.syntaxtree._
+import org.camunda.feel.impl.{EvaluationResultMatchers, FeelEngineTest}
 
 import java.time.LocalDate
-import scala.math.BigDecimal.int2bigDecimal
 
 /** @author
   *   Philipp
@@ -257,6 +254,21 @@ class BuiltinListFunctionsTest
     evaluateExpression(" sublist([1,2,3], 1, 2) ") should returnResult(List(1, 2))
   }
 
+  it should "return null if the start position is 0" in {
+
+    evaluateExpression(" sublist([1,2,3], 0) ") should (returnNull() and reportFailure(
+      failureType = FUNCTION_INVOCATION_FAILURE,
+      failureMessage =
+        "Failed to invoke function 'sublist': start position must be a non-zero number"
+    ))
+
+    evaluateExpression(" sublist([1,2,3], 0, 2) ") should (returnNull() and reportFailure(
+      failureType = FUNCTION_INVOCATION_FAILURE,
+      failureMessage =
+        "Failed to invoke function 'sublist': start position must be a non-zero number"
+    ))
+  }
+
   "A append() function" should "return list with item appended" in {
 
     evaluateExpression(" append([1,2], 3) ") should returnResult(List(1, 2, 3))
@@ -274,11 +286,28 @@ class BuiltinListFunctionsTest
     evaluateExpression(" insert before([1,3],2,2) ") should returnResult(List(1, 2, 3))
   }
 
+  it should "return null if the position is 0" in {
+
+    evaluateExpression(" insert before([1,3],0,2) ") should (returnNull() and reportFailure(
+      failureType = FUNCTION_INVOCATION_FAILURE,
+      failureMessage =
+        "Failed to invoke function 'insert before': position must be a non-zero number"
+    ))
+  }
+
   "A remove() function" should "return list with item at _ removed" in {
 
     evaluateExpression(" remove([1,1,3],2) ") should returnResult(List(1, 3))
   }
 
+  it should "return null if the position is 0" in {
+
+    evaluateExpression(" remove([1,2,3], 0) ") should (returnNull() and reportFailure(
+      failureType = FUNCTION_INVOCATION_FAILURE,
+      failureMessage = "Failed to invoke function 'remove': position must be a non-zero number"
+    ))
+  }
+
   "A reverse() function" should "reverse the list" in {
 
     evaluateExpression(" reverse([1,2,3]) ") should returnResult(List(3, 2, 1))