diff --git a/CHANGELOG.md b/CHANGELOG.md index 546bf8b1e..488405f88 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/hyperformula/src/Cell.ts b/hyperformula/src/Cell.ts index 2dbb2b9d7..9bf224ae7 100644 --- a/hyperformula/src/Cell.ts +++ b/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) diff --git a/hyperformula/src/CellValue.ts b/hyperformula/src/CellValue.ts index ffae21679..412bf8056 100644 --- a/hyperformula/src/CellValue.ts +++ b/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 { diff --git a/hyperformula/src/interpreter/plugin/FunctionPlugin.ts b/hyperformula/src/interpreter/plugin/FunctionPlugin.ts index 810f14047..d005e7d1f 100644 --- a/hyperformula/src/interpreter/plugin/FunctionPlugin.ts +++ b/hyperformula/src/interpreter/plugin/FunctionPlugin.ts @@ -296,19 +296,27 @@ export abstract class FunctionPlugin implements FunctionPluginTypecheck { 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 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 [ ...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 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[] { const coercedArguments: Maybe[] = [] + // 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 [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[] {