[IMP] formulas: accept ranges as meta arguments

Why this commit ?

We would like to introduce the SUBTOTAL formula:
This formula has the particularity of taking meta arguments as input,
and based on certain criteria, returning the value related to the
passed reference.

Compared to other formulas using meta arguments, using the values
belonging to the cells of the meta arguments is a new behavior.

So, for this function to be viable, two things must be done:
- compute the values of the passed meta references (which was not
done until now)
- compute all the values of the cells belonging to the matrix when a
range is passed as a meta reference

This last point (computing all the cell values when a meta range is
passed) implies differentiating between simple meta arguments and
matrix meta arguments in the code. Hence the main purpose of this
commit.

Two other implications of this commit:
- This commit enables vectorization for meta arguments.
- We revised the way arguments were defined in meta functions,
resulting in a complete redefinition for certain functions, such as
COLUMNS and ROWS, which no longer use meta arguments. This change
involved redefining the error type for references whose sheetname
doesn't exist. This also promotes consistency.

Task: 4873384
Part-of: #6774
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>

Author: laa-odoo

src/formulas/compiler.ts

@@ -138,7 +138,7 @@ function compileTokensOrThrow(tokens: Token[]): CompiledFormula {
         const argTypes = argDefinition.type || [];
 
         // detect when an argument need to be evaluated as a meta argument
-        const isMeta = argTypes.includes("META");
+        const isMeta = argTypes.includes("META") || argTypes.includes("RANGE<META>");
         const hasRange = argTypes.some((t) => isRangeType(t));
 
         compiledArgs.push(compileAST(currentArg, isMeta, hasRange));
@@ -178,11 +178,11 @@ function compileTokensOrThrow(tokens: Token[]): CompiledFormula {
         case "STRING":
           return code.return(`this.literalValues.strings[${stringCount++}]`);
         case "REFERENCE":
-          if ((!isMeta && ast.value.includes(":")) || hasRange) {
-            return code.return(`range(deps[${dependencyCount++}])`);
-          } else {
-            return code.return(`ref(deps[${dependencyCount++}], ${isMeta ? "true" : "false"})`);
-          }
+          return code.return(
+            `${ast.value.includes(":") || hasRange ? `range` : `ref`}(deps[${dependencyCount++}], ${
+              isMeta ? "true" : "false"
+            })`
+          );
         case "FUNCALL":
           const args = compileFunctionArgs(ast).map((arg) => arg.assignResultToVariable());
           code.append(...args);

src/functions/arguments.ts

@@ -17,6 +17,7 @@ const ARG_TYPES: ArgType[] = [
   "RANGE<NUMBER>",
   "RANGE<STRING>",
   "META",
+  "RANGE<META>",
 ];
 
 export function arg(definition: string, description: string = ""): ArgDefinition {
@@ -266,6 +267,8 @@ export function _argTargeting(
 // Argument validation
 //------------------------------------------------------------------------------
 
+const META_TYPES: ArgType[] = ["META", "RANGE<META>"];
+
 export function validateArguments(descr: FunctionDescription) {
   if (descr.nbrArgRepeating && descr.nbrArgOptional >= descr.nbrArgRepeating) {
     throw new Error(`Function ${descr.name} has more optional arguments than repeatable ones.`);
@@ -274,9 +277,12 @@ export function validateArguments(descr: FunctionDescription) {
   let foundRepeating = false;
   let consecutiveRepeating = false;
   for (const current of descr.args) {
-    if (current.type.includes("META") && current.type.length > 1) {
+    if (
+      current.type.some((t) => META_TYPES.includes(t)) &&
+      current.type.some((t) => !META_TYPES.includes(t))
+    ) {
       throw new Error(
-        `Function ${descr.name} has an argument that has been declared with more than one type whose type 'META'. The 'META' type can only be declared alone.`
+        `Function ${descr.name} has a mix of META and non-META types in the same argument: ${current.type}.`
       );
     }
 

src/functions/module_info.ts

@@ -1,7 +1,13 @@
 import { getFullReference, splitReference } from "../helpers";
 import { setXcToFixedReferenceType } from "../helpers/reference_type";
 import { _t } from "../translation";
-import { AddFunctionDescription, CellValueType, FunctionResultObject, Maybe } from "../types";
+import {
+  AddFunctionDescription,
+  CellValueType,
+  FunctionResultObject,
+  Matrix,
+  Maybe,
+} from "../types";
 import { CellErrorType, EvaluationError } from "../types/errors";
 import { arg } from "./arguments";
 import { isEvaluationError, toString } from "./helpers";
@@ -18,9 +24,9 @@ export const CELL = {
       "info_type (string)",
       _t("The type of information requested. Can be one of %s", CELL_INFO_TYPES.join(", "))
     ),
-    arg("reference (meta)", _t("The reference to the cell.")),
+    arg("reference (meta, range<meta>)", _t("The reference to the cell.")),
   ],
-  compute: function (info: Maybe<FunctionResultObject>, reference: Maybe<{ value: string }>) {
+  compute: function (info: Maybe<FunctionResultObject>, reference: Matrix<{ value: string }>) {
     const _info = toString(info).toLowerCase();
     if (!CELL_INFO_TYPES.includes(_info)) {
       return new EvaluationError(
@@ -29,9 +35,8 @@ export const CELL = {
     }
 
     const sheetId = this.__originSheetId;
-    const _reference = toString(reference);
-    const topLeftReference = _reference.includes(":") ? _reference.split(":")[0] : _reference;
-    let { sheetName, xc } = splitReference(topLeftReference);
+    const _reference = reference[0][0].value;
+    let { sheetName, xc } = splitReference(_reference);
     // only put the sheet name if the referenced range is in another sheet than the cell the formula is on
     sheetName = sheetName === this.getters.getSheetName(sheetId) ? undefined : sheetName;
     const fixedRef = getFullReference(sheetName, setXcToFixedReferenceType(xc, "colrow"));

src/functions/module_lookup.ts

@@ -118,16 +118,13 @@ export const COLUMN = {
   description: _t("Column number of a specified cell."),
   args: [
     arg(
-      "cell_reference (meta, default='this cell')",
+      "cell_reference (meta, range<meta>, default='this cell')",
       _t(
         "The cell whose column number will be returned. Column A corresponds to 1. By default, the function use the cell in which the formula is entered."
       )
     ),
   ],
-  compute: function (cellReference: Maybe<{ value: string }>) {
-    if (isEvaluationError(cellReference?.value)) {
-      return cellReference;
-    }
+  compute: function (cellReference: Matrix<{ value: string }> | undefined) {
     if (cellReference === undefined) {
       if (this.__originCellPosition?.col === undefined) {
         return new EvaluationError(
@@ -136,21 +133,22 @@ export const COLUMN = {
           )
         );
       }
-      return this.__originCellPosition!.col! + 1;
+      return this.__originCellPosition.col + 1;
     }
-
-    const zone = this.getters.getRangeFromSheetXC(
+    if (cellReference[0][0] === undefined) {
+      return new EvaluationError(_t("The range is out of bounds."));
+    }
+    if (cellReference[0][0].value === CellErrorType.InvalidReference) {
+      return cellReference[0][0]; // return the same error
+    }
+    const left = this.getters.getRangeFromSheetXC(
       this.getters.getActiveSheetId(),
-      cellReference.value
-    ).zone;
-
-    if (zone.left === zone.right) {
-      return zone.left + 1;
+      cellReference[0][0].value
+    ).zone.left;
+    if (cellReference.length === 1) {
+      return left + 1;
     }
-
-    return generateMatrix(zone.right - zone.left + 1, 1, (col, row) => ({
-      value: zone.left + col + 1,
-    }));
+    return generateMatrix(cellReference.length, 1, (col, row) => ({ value: left + col + 1 }));
   },
   isExported: true,
 } satisfies AddFunctionDescription;
@@ -161,13 +159,16 @@ export const COLUMN = {
 
 export const COLUMNS = {
   description: _t("Number of columns in a specified array or range."),
-  args: [arg("range (meta)", _t("The range whose column count will be returned."))],
-  compute: function (range: { value: string }) {
-    if (isEvaluationError(range?.value)) {
-      return range;
+  args: [arg("range (any, range<any>)", _t("The range whose column count will be returned."))],
+  compute: function (range: Arg) {
+    const _range = toMatrix(range);
+    if (_range[0][0] === undefined) {
+      return new EvaluationError(_t("The range is out of bounds."));
+    }
+    if (_range[0][0].value === CellErrorType.InvalidReference) {
+      return _range[0][0];
     }
-    const zone = toZone(range.value);
-    return zone.right - zone.left + 1;
+    return _range.length;
   },
   isExported: true,
 } satisfies AddFunctionDescription;
@@ -504,16 +505,13 @@ export const ROW = {
   description: _t("Row number of a specified cell."),
   args: [
     arg(
-      "cell_reference (meta, default='this cell')",
+      "cell_reference (meta, range<meta>, default='this cell')",
       _t(
         "The cell whose row number will be returned. By default, this function uses the cell in which the formula is entered."
       )
     ),
   ],
-  compute: function (cellReference: Maybe<{ value: string }>) {
-    if (isEvaluationError(cellReference?.value)) {
-      return cellReference;
-    }
+  compute: function (cellReference: Matrix<{ value: string }> | undefined) {
     if (cellReference === undefined) {
       if (this.__originCellPosition?.row === undefined) {
         return new EvaluationError(
@@ -522,21 +520,22 @@ export const ROW = {
           )
         );
       }
-      return this.__originCellPosition!.row! + 1;
+      return this.__originCellPosition.row + 1;
     }
-
-    const zone = this.getters.getRangeFromSheetXC(
+    if (cellReference[0][0] === undefined) {
+      return new EvaluationError(_t("The range is out of bounds."));
+    }
+    if (cellReference[0][0].value === CellErrorType.InvalidReference) {
+      return cellReference[0][0]; // return the same error
+    }
+    const top = this.getters.getRangeFromSheetXC(
       this.getters.getActiveSheetId(),
-      cellReference.value
-    ).zone;
-
-    if (zone.top === zone.bottom) {
-      return zone.top + 1;
+      cellReference[0][0].value
+    ).zone.top;
+    if (cellReference[0].length === 1) {
+      return top + 1;
     }
-
-    return generateMatrix(1, zone.bottom - zone.top + 1, (col, row) => ({
-      value: zone.top + row + 1,
-    }));
+    return generateMatrix(1, cellReference[0].length, (col, row) => ({ value: top + row + 1 }));
   },
   isExported: true,
 } satisfies AddFunctionDescription;
@@ -547,13 +546,16 @@ export const ROW = {
 
 export const ROWS = {
   description: _t("Number of rows in a specified array or range."),
-  args: [arg("range (meta)", _t("The range whose row count will be returned."))],
-  compute: function (range: { value: string }) {
-    if (isEvaluationError(range?.value)) {
-      return range;
+  args: [arg("range (any, range<any>)", _t("The range whose row count will be returned."))],
+  compute: function (range: Arg) {
+    const _range = toMatrix(range);
+    if (_range[0][0] === undefined) {
+      return new EvaluationError(_t("The range is out of bounds."));
+    }
+    if (_range[0][0].value === CellErrorType.InvalidReference) {
+      return _range[0][0];
     }
-    const zone = toZone(range.value);
-    return zone.bottom - zone.top + 1;
+    return _range[0].length;
   },
   isExported: true,
 } satisfies AddFunctionDescription;
@@ -975,7 +977,7 @@ export const OFFSET = {
   ),
   args: [
     arg(
-      "cell_reference (meta)",
+      "cell_reference (meta, range<meta>)",
       _t("The starting point from which to count the offset rows and columns.")
     ),
     arg("offset_rows (number)", _t("The number of rows to offset by.")),
@@ -990,26 +992,26 @@ export const OFFSET = {
     ),
   ],
   compute: function (
-    cellReference: Maybe<{ value: string }>,
+    cellReference: Matrix<{ value: string }>,
     offsetRows: Maybe<FunctionResultObject>,
     offsetColumns: Maybe<FunctionResultObject>,
     height: Maybe<FunctionResultObject>,
     width: Maybe<FunctionResultObject>
   ) {
-    if (isEvaluationError(cellReference?.value)) {
-      return cellReference;
+    if (isEvaluationError(cellReference[0][0].value)) {
+      return cellReference[0][0];
     }
 
-    const _cellReference = cellReference?.value;
-    if (!_cellReference) {
+    const ref0 = cellReference[0][0].value;
+    if (!ref0) {
       return new EvaluationError(
         "In this context, the function OFFSET needs to have a cell or range in parameter."
       );
     }
-    const zone = toZone(_cellReference);
+    const zone = toZone(ref0);
 
-    let offsetHeight = zone.bottom - zone.top + 1;
-    let offsetWidth = zone.right - zone.left + 1;
+    let offsetHeight = cellReference[0].length;
+    let offsetWidth = cellReference.length;
 
     if (height) {
       const _height = toNumber(height, this.locale);
@@ -1031,7 +1033,7 @@ export const OFFSET = {
       offsetWidth = _width;
     }
 
-    const { sheetName } = splitReference(_cellReference);
+    const { sheetName } = splitReference(ref0);
 
     const sheetId =
       (sheetName && this.getters.getSheetIdByName(sheetName)) || this.getters.getActiveSheetId();

src/plugins/ui_core_views/cell_evaluation/compilation_parameters.ts

@@ -1,5 +1,5 @@
 import { functionRegistry } from "../../../functions";
-import { getFullReference, intersection, isZoneValid, zoneToXc } from "../../../helpers";
+import { getFullReference, intersection, isZoneValid, toXC, zoneToXc } from "../../../helpers";
 import { ModelConfig } from "../../../model";
 import { _t } from "../../../translation";
 import {
@@ -74,13 +74,14 @@ class CompilationParametersBuilder {
     if (rangeError) {
       return rangeError;
     }
+    // the compiler guarantees only single cell ranges reach this part of the code
+    const position = { sheetId: range.sheetId, col: range.zone.left, row: range.zone.top };
     if (isMeta) {
+      this.computeCell(position); // ensure the cell is computed: sometimes formulas that use meta parameters ending by return the value of the corresponding reference
       // Use zoneToXc of zone instead of getRangeString to avoid sending unbounded ranges
       const sheetName = this.getters.getSheetName(range.sheetId);
       return { value: getFullReference(sheetName, zoneToXc(range.zone)) };
     }
-    // the compiler guarantees only single cell ranges reach this part of the code
-    const position = { sheetId: range.sheetId, col: range.zone.left, row: range.zone.top };
     return this.computeCell(position);
   }
 
@@ -92,7 +93,7 @@ class CompilationParametersBuilder {
    * Note that each col is possibly sparse: it only contain the values of cells
    * that are actually present in the grid.
    */
-  private range(range: Range): Matrix<FunctionResultObject> {
+  private range(range: Range, isMeta: boolean): Matrix<FunctionResultObject> {
     const rangeError = this.getRangeError(range);
     if (rangeError) {
       return [[rangeError]];
@@ -107,22 +108,29 @@ class CompilationParametersBuilder {
     if (!_zone) {
       return [[]];
     }
+
+    // Performance issue: cache the range results to avoid recomputing them
     const { top, left, bottom, right } = zone;
-    const cacheKey = `${sheetId}-${top}-${left}-${bottom}-${right}`;
+    const cacheKey = `${sheetId}-${top}-${left}-${bottom}-${right}-${isMeta}`;
     if (cacheKey in this.rangeCache) {
       return this.rangeCache[cacheKey];
     }
 
     const height = _zone.bottom - _zone.top + 1;
     const width = _zone.right - _zone.left + 1;
     const matrix: Matrix<FunctionResultObject> = new Array(width);
+    const sheetName = this.getters.getSheetName(range.sheetId);
+
     // Performance issue: nested loop is faster than a map here
     for (let col = _zone.left; col <= _zone.right; col++) {
       const colIndex = col - _zone.left;
       matrix[colIndex] = new Array(height);
       for (let row = _zone.top; row <= _zone.bottom; row++) {
         const rowIndex = row - _zone.top;
-        matrix[colIndex][rowIndex] = this.computeCell({ sheetId, col, row });
+        const computedCell = this.computeCell({ sheetId, col, row });
+        matrix[colIndex][rowIndex] = isMeta
+          ? { value: getFullReference(sheetName, toXC(col, row)) }
+          : computedCell;
       }
     }
 
@@ -135,7 +143,7 @@ class CompilationParametersBuilder {
       return new InvalidReferenceError();
     }
     if (range.invalidSheetName) {
-      return new EvaluationError(_t("Invalid sheet name: %s", range.invalidSheetName));
+      return new InvalidReferenceError(_t("Invalid sheet name: %s", range.invalidSheetName));
     }
     return undefined;
   }

src/types/functions.ts

@@ -16,7 +16,8 @@ export type ArgType =
   | "RANGE<DATE>"
   | "RANGE<STRING>"
   | "RANGE<ANY>"
-  | "META";
+  | "META"
+  | "RANGE<META>";
 
 export interface ArgDefinition {
   acceptMatrix?: boolean;

src/types/misc.ts

@@ -173,7 +173,7 @@ export type ReferenceDenormalizer = (
   paramNumber: number
 ) => FunctionResultObject;
 
-export type EnsureRange = (range: Range) => Matrix<FunctionResultObject>;
+export type EnsureRange = (range: Range, isMeta: boolean) => Matrix<FunctionResultObject>;
 
 export type GetSymbolValue = (symbolName: string) => Arg;