From d2c5975f5bfaa643717e4e9356bf7f2846340091 Mon Sep 17 00:00:00 2001
From: Alexis Lacroix <laa@odoo.com>
Date: Fri, 2 Aug 2024 08:54:15 +0000
Subject: [PATCH] [FIX] evaluation: handle throwing error on each vectorized
 position
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this commit, if an error was throw when evaluating a
vectorized formula, then the evaluation stopped and only the
cell with the formula was marked as an error.
No other values ​​were spread.

This behavior is functionally false, because depending on the
arguments passed, it is possible that on this vector position,
the evaluation does not return an error.

This commit changes that by making sure to try for each vector
position if the evaluation works. Even though it only spreads
errors in certain cases.

closes odoo/o-spreadsheet#4798

Task: 4091276
X-original-commit: 29775e12b242d8311715eec62ebf836dc70e04c7
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
---
 src/functions/index.ts                | 31 ++++++++++++++-------------
 tests/functions/vectorization.test.ts | 19 ++++++++++++++++
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/src/functions/index.ts b/src/functions/index.ts
index 69af219e71..99fa41dd1a 100644
--- a/src/functions/index.ts
+++ b/src/functions/index.ts
@@ -117,17 +117,6 @@ function createComputeFunction(
   descr: FunctionDescription,
   functionName: string
 ): ComputeFunction<Matrix<FunctionResultObject> | FunctionResultObject> {
-  function runtimeCompute(
-    this: EvalContext,
-    ...args: Arg[]
-  ): Matrix<FunctionResultObject> | FunctionResultObject {
-    try {
-      return vectorizedCompute.apply(this, args);
-    } catch (e) {
-      return handleError(e, functionName);
-    }
-  }
-
   function vectorizedCompute(
     this: EvalContext,
     ...args: Arg[]
@@ -174,7 +163,8 @@ function createComputeFunction(
       if (!isMatrix(arg) && argDefinition.acceptMatrixOnly) {
         throw new BadExpressionError(
           _t(
-            "Function [[FUNCTION_NAME]] expects the parameter '%s' to be reference to a cell or range.",
+            "Function %s expects the parameter '%s' to be reference to a cell or range.",
+            functionName,
             (i + 1).toString()
           )
         );
@@ -184,7 +174,7 @@ function createComputeFunction(
 
     if (countVectorizableCol === 1 && countVectorizableRow === 1) {
       // either this function is not vectorized or it ends up with a 1x1 dimension
-      return computeFunctionToObject.apply(this, args);
+      return errorHandlingCompute.apply(this, args);
     }
 
     const getArgOffset: (i: number, j: number) => Arg[] = (i, j) =>
@@ -205,7 +195,7 @@ function createComputeFunction(
       if (col > vectorizableColLimit - 1 || row > vectorizableRowLimit - 1) {
         return notAvailableError;
       }
-      const singleCellComputeResult = computeFunctionToObject.apply(this, getArgOffset(col, row));
+      const singleCellComputeResult = errorHandlingCompute.apply(this, getArgOffset(col, row));
       // In the case where the user tries to vectorize arguments of an array formula, we will get an
       // array for every combination of the vectorized arguments, which will lead to a 3D matrix and
       // we won't be able to return the values.
@@ -221,6 +211,17 @@ function createComputeFunction(
     });
   }
 
+  function errorHandlingCompute(
+    this: EvalContext,
+    ...args: Arg[]
+  ): Matrix<FunctionResultObject> | FunctionResultObject {
+    try {
+      return computeFunctionToObject.apply(this, args);
+    } catch (e) {
+      return handleError(e, functionName);
+    }
+  }
+
   function computeFunctionToObject(
     this: EvalContext,
     ...args: Arg[]
@@ -245,7 +246,7 @@ function createComputeFunction(
     return matrixMap(result as Matrix<CellValue>, (row) => ({ value: row }));
   }
 
-  return runtimeCompute;
+  return vectorizedCompute;
 }
 
 export function handleError(e: unknown, functionName: string): FunctionResultObject {
diff --git a/tests/functions/vectorization.test.ts b/tests/functions/vectorization.test.ts
index 4c528deaf3..1160688518 100644
--- a/tests/functions/vectorization.test.ts
+++ b/tests/functions/vectorization.test.ts
@@ -153,6 +153,25 @@ describe("vectorization", () => {
     expect(checkFunctionDoesntSpreadBeyondRange(model, "D1:E2")).toBeTruthy();
   });
 
+  test("function which throws an error during the evaluation of the position of one of its vectors, will apply the error to this position and continue the evaluation for each vector position", () => {
+    // prettier-ignore
+    const grid = {
+      A1: "A1", B1: "B1",
+      A2: "A2", B2: "#ERROR",
+      A3: "A3", B3: "B3",
+    };
+    const model = createModelFromGrid(grid);
+    setCellContent(model, "D1", '=FUNCTION.WITHOUT.RANGE.ARGS("this is ", B1:B3)');
+    expect(getRangeValuesAsMatrix(model, "D1:D3")).toEqual([
+      ["this is B1"],
+      ["#ERROR"],
+      ["this is B3"],
+    ]);
+
+    setCellContent(model, "D1", '=FUNCTION.WITHOUT.RANGE.ARGS("#ERROR", A1:A3)');
+    expect(getRangeValuesAsMatrix(model, "D1:D3")).toEqual([["#ERROR"], ["#ERROR"], ["#ERROR"]]);
+  });
+
   test("binary operators should always accept vectors", () => {
     // mean binary operators args should always be simple args
     for (let op in OPERATOR_MAP) {