HF-131 (4/7): expose which function produced an error, and which argument

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 - Added a message after `npm i hyperformula` that points AI assistants to the SKILLS file. The message needs `--foreground-scripts` to be visible in npm, and pnpm 10 needs the `postinstall` script to be approved first. [#1829](https://github.com/handsontable/hyperformula/pull/1829)
 - Added `DetailedCellError.hasMessage`, so a consumer can tell a cell error that never carried a message (e.g. a custom function that omitted one) apart from one with a deliberately empty message — both previously surfaced identically as `message: ''`. [#1547](https://github.com/handsontable/hyperformula/issues/1547)
+- Added `DetailedCellError.originFunction` and `DetailedCellError.argumentIndex`, which name the function or operator that produced a cell error and, when it failed to coerce an argument, that argument's position as written in the formula. [#1547](https://github.com/handsontable/hyperformula/issues/1547)
 
 ### Changed
 

hyperformula/src/Cell.ts

@@ -180,10 +180,16 @@ export class CellError {
    * propagated error never acquires one: the reading function's own argument slot is not the
    * offending argument.
    *
+   * Also a no-op once `originFunction` is set, even for a non-propagated error: a nested call's
+   * own error (e.g. `SQRT(-1)` inside `=DATE(1,1,SQRT(-1))`) stamps its origin before the outer
+   * call's coercion loop ever sees it, and that loop's own argument slot is not the one that
+   * actually produced the error — attaching an index here would pair someone else's origin with
+   * this call's argument position, which is incoherent.
+   *
    * @param {number} index - zero-based index of the offending argument
    */
   public withArgumentIndex(index: number): CellError {
-    if (this.propagated || this.argumentIndex !== undefined) {
+    if (this.propagated || this.originFunction !== undefined || this.argumentIndex !== undefined) {
       return this
     }
     return new CellError(this.type, this.message, this.root, this.originFunction, index, this.propagated, this.originAddress, this.originAddressVersion)

hyperformula/src/CellValue.ts

@@ -24,6 +24,48 @@ export class DetailedCellError {
    */
   public readonly hasMessage: boolean
 
+  /**
+   * What produced this error: the function or operator that rejected a value, e.g. `'SQRT'`
+   * or `'divide'`. An operator is named after the arithmetic helper it dispatches to.
+   *
+   * First occurrence wins, so a function that only received the error from a nested call
+   * never replaces it: `=SUM(SQRT(-1))` reports `'SQRT'`. An error read from another cell
+   * keeps the origin it had there.
+   *
+   * `undefined` when no function or operator produced the error, e.g. for `#SPILL!`,
+   * `#CYCLE!`, a parsing error, or an error value typed into a cell. An error from a
+   * reference the engine cannot resolve is attributed to the function that reads it.
+   */
+  public readonly originFunction?: string
+
+  /**
+   * The zero-based index of the argument that was rejected, when `originFunction` names a
+   * function whose own argument coercion produced this error.
+   *
+   * The index counts the arguments AS WRITTEN in the formula. For a function that accepts
+   * ranges, a range is one argument however many cells it covers, so `=AND(A1:A100,"x")`
+   * reports `1` for `"x"` — the index describes the formula, not the data behind it.
+   *
+   * `undefined` whenever the error was not a coercion failure on one of the origin
+   * function's own arguments — including when it came from a nested call (its own
+   * `originFunction` already claimed it), when it was propagated from elsewhere, or when
+   * the offending value sat inside a range that the function IGNORES
+   * (`=AND(A1:A2,TRUE())` over text returns `TRUE`, so there is no error to attribute).
+   *
+   * When a function does NOT ignore it — `MULTINOMIAL` coerces every value to a number —
+   * a bad cell inside a range is attributed to the RANGE's own argument slot, since that
+   * is the argument the user wrote: `=MULTINOMIAL(1,A1:A3)` with text in `A3` reports `1`.
+   * The index names the argument to look at, not the cell; the address of the offending
+   * cell is not part of this field.
+   *
+   * Two cases report an index that is less useful than it looks: an argument that is a
+   * reference the engine cannot resolve is claimed by the function reading it
+   * (`=DATE(2020,A99999999999,1)` reports `DATE` with index `1`), and an error raised per
+   * element while a function is applied across an array carries an index with no
+   * `originFunction` (`=DATE(2020,A1:A2,1)` over text, with array arithmetic on).
+   */
+  public readonly argumentIndex?: number
+
   constructor(
     error: CellError,
     public readonly value: string,
@@ -32,6 +74,8 @@ export class DetailedCellError {
     this.type = error.type
     this.message = error.message ?? ''
     this.hasMessage = error.message !== undefined
+    this.originFunction = error.originFunction
+    this.argumentIndex = error.argumentIndex
   }
 
   public toString(): string {

hyperformula/src/interpreter/plugin/FunctionPlugin.ts

@@ -296,19 +296,27 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck<Function
     return this.arraySizePredictor.checkArraySizeForAst(ast, state)
   }
 
-  protected listOfScalarValues(asts: Ast[], state: InterpreterState): [InternalScalarValue, boolean, boolean][] {
-    const ret: [InternalScalarValue, boolean, boolean][] = []
-    for (const argAst of asts) {
+  /**
+   * Flattens the arguments into scalars, expanding any range into its values.
+   *
+   * The fourth element of each tuple is the index of the ARGUMENT THE USER WROTE that the
+   * scalar came from. Every value expanded out of one range keeps that range's own index,
+   * so a later error can name the argument rather than its position in this flattened
+   * list — `=AND(A1:A3,"x")` must blame argument 1, not entry 3.
+   */
+  protected listOfScalarValues(asts: Ast[], state: InterpreterState): [InternalScalarValue, boolean, boolean, number][] {
+    const ret: [InternalScalarValue, boolean, boolean, number][] = []
+    asts.forEach((argAst, argumentIndex) => {
       const isSyntacticallyEmpty = argAst.type === AstNodeType.EMPTY
       const value = this.evaluateAst(argAst, state)
       if (value instanceof SimpleRangeValue) {
         for (const scalarValue of value.valuesFromTopLeftCorner()) {
-          ret.push([scalarValue, true, isSyntacticallyEmpty])
+          ret.push([scalarValue, true, isSyntacticallyEmpty, argumentIndex])
         }
       } else {
-        ret.push([value, false, isSyntacticallyEmpty])
+        ret.push([value, false, isSyntacticallyEmpty, argumentIndex])
       }
-    }
+    })
     return ret
   }
 
@@ -410,6 +418,10 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck<Function
     const argumentValues: InterpreterValue[] = evaluatedArguments.map(([value]) => value)
     const argumentIgnorableFlags = evaluatedArguments.map(([, ignorable]) => ignorable)
     const syntacticallyEmptyFlags = evaluatedArguments.map(([, , empty]) => empty)
+    // Which argument each value came from. Identical to the position in this list unless a
+    // range was expanded above; a custom plugin overriding `listOfScalarValues` with the
+    // old three-element tuple yields undefined here and falls back to the position.
+    const sourceArgumentIndexes = evaluatedArguments.map(([, , , index]) => index)
     const argumentMetadata = this.buildMetadataForEachArgumentValue(argumentValues.length, metadata)
     const isVectorizationOn = state.arraysFlag && !metadata.vectorizationForbidden
 
@@ -421,13 +433,13 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck<Function
 
     if (resultArrayHeight === 1 && resultArrayWidth === 1) {
       const vectorizedArguments = this.vectorizeAndBroadcastArgumentsIfNecessary(isVectorizationOn, argumentValues, argumentMetadata, 0, 0)
-      return this.calculateSingleCellOfResultArray(state, vectorizedArguments, argumentMetadata, argumentIgnorableFlags, syntacticallyEmptyFlags, functionImplementation, metadata.returnNumberType)
+      return this.calculateSingleCellOfResultArray(state, vectorizedArguments, argumentMetadata, argumentIgnorableFlags, syntacticallyEmptyFlags, functionImplementation, metadata.returnNumberType, sourceArgumentIndexes)
     }
 
     const resultArray: InternalScalarValue[][] = [ ...Array(resultArrayHeight).keys() ].map(row =>
       [ ...Array(resultArrayWidth).keys() ].map(col => {
         const vectorizedArguments = this.vectorizeAndBroadcastArgumentsIfNecessary(isVectorizationOn, argumentValues, argumentMetadata, row, col)
-        const result = this.calculateSingleCellOfResultArray(state, vectorizedArguments, argumentMetadata, argumentIgnorableFlags, syntacticallyEmptyFlags, functionImplementation, metadata.returnNumberType)
+        const result = this.calculateSingleCellOfResultArray(state, vectorizedArguments, argumentMetadata, argumentIgnorableFlags, syntacticallyEmptyFlags, functionImplementation, metadata.returnNumberType, sourceArgumentIndexes)
 
         if (result instanceof SimpleRangeValue) {
           throw new Error('Function returning array cannot be vectorized.')
@@ -448,8 +460,9 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck<Function
     syntacticallyEmptyFlags: boolean[],
     functionImplementation: (...arg: any) => InterpreterValue,
     returnNumberType: NumberType | undefined,
+    sourceArgumentIndexes: number[] = [],
   ): RawInterpreterValue {
-    const coercedArguments = this.coerceArgumentsToRequiredTypes(state, vectorizedArguments, argumentsMetadata, argumentIgnorableFlags, syntacticallyEmptyFlags)
+    const coercedArguments = this.coerceArgumentsToRequiredTypes(state, vectorizedArguments, argumentsMetadata, argumentIgnorableFlags, syntacticallyEmptyFlags, sourceArgumentIndexes)
 
     if (coercedArguments instanceof CellError) {
       return coercedArguments
@@ -465,8 +478,13 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck<Function
     argumentsMetadata: FunctionArgument[],
     argumentIgnorableFlags: boolean[],
     syntacticallyEmptyFlags: boolean[],
+    sourceArgumentIndexes: number[] = [],
   ):  CellError | Maybe<InterpreterValue | complex | RawNoErrorScalarValue>[] {
     const coercedArguments: Maybe<InterpreterValue | complex | RawNoErrorScalarValue>[] = []
+    // The argument the user wrote, which is the position in this loop only when no range
+    // was expanded. Reporting the loop index made the reported argument depend on how many
+    // cells a preceding range happened to cover.
+    const writtenArgumentIndex = (i: number): number => sourceArgumentIndexes[i] ?? i
 
     for (let i = 0; i < argumentsMetadata.length; i++) {
       const argumentMetadata = argumentsMetadata[i]
@@ -485,11 +503,11 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck<Function
       const coercedValue = this.coerceToType(argumentValue, argumentMetadata, state)
 
       if (coercedValue === undefined && !argumentIgnorableFlags[i]) {
-        return new CellError(ErrorType.VALUE, ErrorMessage.WrongType)
+        return new CellError(ErrorType.VALUE, ErrorMessage.WrongType).withArgumentIndex(writtenArgumentIndex(i))
       }
 
       if (coercedValue instanceof CellError && argumentMetadata.argumentType !== FunctionArgumentType.SCALAR) {
-        return coercedValue
+        return coercedValue.withArgumentIndex(writtenArgumentIndex(i))
       }
 
       coercedArguments.push(coercedValue)
@@ -513,10 +531,10 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck<Function
     return argumentValue.data[targetRowNum]?.[targetColNum]
   }
 
-  protected evaluateArguments(args: Ast[], state: InterpreterState, metadata: FunctionMetadata): [InterpreterValue, boolean, boolean][] {
+  protected evaluateArguments(args: Ast[], state: InterpreterState, metadata: FunctionMetadata): [InterpreterValue, boolean, boolean, number][] {
     return metadata.expandRanges
       ? this.listOfScalarValues(args, state)
-      : args.map((ast) => [this.evaluateAst(ast, state), false, ast.type === AstNodeType.EMPTY])
+      : args.map((ast, argumentIndex) => [this.evaluateAst(ast, state), false, ast.type === AstNodeType.EMPTY, argumentIndex])
   }
 
   protected buildMetadataForEachArgumentValue(numberOfArgumentValuesPassed: number, metadata: FunctionMetadata): FunctionArgument[] {