diff --git a/CHANGELOG.md b/CHANGELOG.md index 488405f880..9f99e4c2e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +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) +- Added `DetailedCellError.originFunction` and `DetailedCellError.argumentIndex`, which name what produced a cell error (a function, an operator, or a source such as `reference` or `parser`) and, when it failed to coerce an argument, that argument's position as written. [#1547](https://github.com/handsontable/hyperformula/issues/1547) ### Changed @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fixed the `EDATE` function returning the 28th of February instead of the 29th when the shifted date falls in February of a leap year (e.g. `=EDATE(DATE(2028, 1, 31), 1)` now returns `2028-02-29` instead of `2028-02-28`). [#1781](https://github.com/handsontable/hyperformula/pull/1781) - Fixed a cell holding a static error value, such as a rejected formula or a typed-in error, being reported under the address of the cell that read it first instead of its own address. [#1547](https://github.com/handsontable/hyperformula/issues/1547) +- Fixed `#SPILL!` errors reporting no address; their `address` is now the cell whose array result could not spill. [#1547](https://github.com/handsontable/hyperformula/issues/1547) ## [3.5.0] - 2026-10-09 diff --git a/docs/guide/known-limitations.md b/docs/guide/known-limitations.md index d78cac018d..5bca16c1e2 100644 --- a/docs/guide/known-limitations.md +++ b/docs/guide/known-limitations.md @@ -47,6 +47,7 @@ a circular reference. * The FILTER function accepts either single rows of equal width or single columns of equal height. In other words, all arrays passed to the FILTER function must have equal dimensions, and at least one of those dimensions must be 1. * Array-producing functions (e.g., SEQUENCE, FILTER) require their output dimensions to be determinable at parse time. Passing cell references or formulas as dimension arguments (e.g., `=SEQUENCE(A1)`) results in a `#VALUE!` error, because the output size cannot be resolved before evaluation. * The TEXT function does not accept embedded double-quote literals in the format string. In Excel, `""` inside a format string is an escape sequence for a literal `"` character — e.g. `=TEXT(1234.5, "#,##0.00 ""zł""")` returns `"1,234.50 zł"`. If your application requires this escape sequence, supply a custom [`stringifyCurrency`](currency-handling.md) callback. +* `#SPILL!` and `#CYCLE!` errors report the cell they affect but name no `originFunction`, because no function produced them — they follow from the layout of the sheet rather than from evaluating a value. Every other cell error names what produced it: the function or operator that rejected a value, `reference` for a reference that cannot be resolved, `removed reference` for one destroyed by removing rows or columns, `parser`, `user input`, `literal`, or `array literal`. ### UNIQUE function diff --git a/hyperformula/src/CellContentParser.ts b/hyperformula/src/CellContentParser.ts index 03f4612ae7..3bc5c97570 100644 --- a/hyperformula/src/CellContentParser.ts +++ b/hyperformula/src/CellContentParser.ts @@ -71,7 +71,7 @@ export namespace CellContent { // caller passes no message, because the rule only inspects this call site, // not who calls it. constructor(errorType: ErrorType, message?: string) { - this.value = new CellError(errorType, message) + this.value = new CellError(errorType, message).withOrigin('user input') } } diff --git a/hyperformula/src/CellValue.ts b/hyperformula/src/CellValue.ts index 412bf8056c..4466c4c741 100644 --- a/hyperformula/src/CellValue.ts +++ b/hyperformula/src/CellValue.ts @@ -25,16 +25,22 @@ 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. + * What produced this error. * - * 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. + * Usually the function or operator that rejected a value, e.g. `'SQRT'` or `'divide'`; an + * operator is named after the arithmetic helper it dispatches to. An error that exists + * before any function reads it names what built it instead: `'reference'` for a reference + * that cannot be resolved, `'removed reference'` for one destroyed by removing rows or + * columns, `'parser'` for a formula that could not be parsed, `'user input'` for an error + * value typed into a cell, `'literal'` for one written into a formula, and + * `'array literal'` for an array literal whose rows differ in length. * - * `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. + * First occurrence wins, so a function that only read the error never replaces that: + * `=SUM(SQRT(-1))` reports `'SQRT'`, and `=SUM(A2:A99999999999)` reports `'reference'`. + * An error inside an array result names the operation that built the array. + * + * `undefined` for `#SPILL!` and `#CYCLE!`, which arise from the layout of a sheet rather + * than from evaluating a value. */ public readonly originFunction?: string @@ -57,12 +63,6 @@ export class DetailedCellError { * 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 diff --git a/hyperformula/src/DependencyGraph/DependencyGraph.ts b/hyperformula/src/DependencyGraph/DependencyGraph.ts index 81270bccc5..6e9b07da39 100644 --- a/hyperformula/src/DependencyGraph/DependencyGraph.ts +++ b/hyperformula/src/DependencyGraph/DependencyGraph.ts @@ -609,7 +609,7 @@ export class DependencyGraph { public getCellValue(address: SimpleCellAddress): InterpreterValue { if (this.isPlaceholder(address.sheet)) { - return new CellError(ErrorType.REF, ErrorMessage.SheetRef) + return new CellError(ErrorType.REF, ErrorMessage.SheetRef).withOrigin('reference') } const value = this.addressMapping.getCellValue(address) diff --git a/hyperformula/src/DependencyGraph/FormulaVertex.ts b/hyperformula/src/DependencyGraph/FormulaVertex.ts index 84f54499ab..fbea5f644e 100644 --- a/hyperformula/src/DependencyGraph/FormulaVertex.ts +++ b/hyperformula/src/DependencyGraph/FormulaVertex.ts @@ -177,7 +177,7 @@ export class ArrayFormulaVertex extends FormulaVertex { } setNoSpace(): InterpreterValue { - this.array = new ErroredArray(new CellError(ErrorType.SPILL, ErrorMessage.NoSpaceForArrayResult), ArraySize.error()) + this.array = new ErroredArray(new CellError(ErrorType.SPILL, ErrorMessage.NoSpaceForArrayResult, this), ArraySize.error()) return this.getCellValue() } diff --git a/hyperformula/src/DependencyGraph/ParsingErrorVertex.ts b/hyperformula/src/DependencyGraph/ParsingErrorVertex.ts index 756b32f680..65333b4956 100644 --- a/hyperformula/src/DependencyGraph/ParsingErrorVertex.ts +++ b/hyperformula/src/DependencyGraph/ParsingErrorVertex.ts @@ -26,7 +26,7 @@ export class ParsingErrorVertex extends CellVertex { */ public getCellValue(): CellError { const firstNonemptyMessage = this.errors.map(error => error.message).find((msg) => msg) - return CellError.parsingError(firstNonemptyMessage) + return CellError.parsingError(firstNonemptyMessage).withOrigin('parser') } /** diff --git a/hyperformula/src/dependencyTransformers/Transformer.ts b/hyperformula/src/dependencyTransformers/Transformer.ts index fc937d1c27..5d0258a83a 100644 --- a/hyperformula/src/dependencyTransformers/Transformer.ts +++ b/hyperformula/src/dependencyTransformers/Transformer.ts @@ -113,7 +113,7 @@ export abstract class Transformer implements FormulaTransformer { if (newCellAddress instanceof CellAddress) { return {...ast, reference: newCellAddress} } else if (newCellAddress === ErrorType.REF) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved).withOrigin('removed reference')) } else { return ast } @@ -124,7 +124,7 @@ export abstract class Transformer implements FormulaTransformer { if (Array.isArray(newRange)) { return {...ast, start: newRange[0], end: newRange[1]} } else if (newRange === ErrorType.REF) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved).withOrigin('removed reference')) } else { return ast } @@ -135,7 +135,7 @@ export abstract class Transformer implements FormulaTransformer { if (Array.isArray(newRange)) { return {...ast, start: newRange[0], end: newRange[1]} } else if (newRange === ErrorType.REF) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved).withOrigin('removed reference')) } else { return ast } @@ -146,7 +146,7 @@ export abstract class Transformer implements FormulaTransformer { if (Array.isArray(newRange)) { return {...ast, start: newRange[0], end: newRange[1]} } else if (newRange === ErrorType.REF) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved).withOrigin('removed reference')) } else { return ast } diff --git a/hyperformula/src/interpreter/Interpreter.ts b/hyperformula/src/interpreter/Interpreter.ts index a62372a103..41195f7190 100644 --- a/hyperformula/src/interpreter/Interpreter.ts +++ b/hyperformula/src/interpreter/Interpreter.ts @@ -82,6 +82,9 @@ export class Interpreter { if (val instanceof CellError) { val = stampOriginForAstNode(val, ast) } + if (val instanceof SimpleRangeValue) { + val = stampOriginForArrayElements(val, ast) + } return wrapperForRootVertex(val, state.formulaVertex) } @@ -100,11 +103,11 @@ export class Interpreter { const address = ast.reference.toSimpleCellAddress(state.formulaAddress) if (isColOrRowInvalid(address)) { - return new CellError(ErrorType.REF, ErrorMessage.BadRef) + return new CellError(ErrorType.REF, ErrorMessage.BadRef).withOrigin('reference') } if (!this.isSheetValid(ast.reference)) { - return new CellError(ErrorType.REF, ErrorMessage.SheetRef) + return new CellError(ErrorType.REF, ErrorMessage.SheetRef).withOrigin('reference') } return this.dependencyGraph.getCellValue(address) @@ -202,16 +205,16 @@ export class Interpreter { if (namedExpression) { return this.dependencyGraph.getCellValue(namedExpression.address) } else { - return new CellError(ErrorType.NAME, ErrorMessage.NamedExpressionName(ast.expressionName)) + return new CellError(ErrorType.NAME, ErrorMessage.NamedExpressionName(ast.expressionName)).withOrigin('reference') } } case AstNodeType.CELL_RANGE: { if (!this.isSheetValid(ast.start) || !this.isSheetValid(ast.end)) { - return new CellError(ErrorType.REF, ErrorMessage.SheetRef) + return new CellError(ErrorType.REF, ErrorMessage.SheetRef).withOrigin('reference') } if (!this.rangeSpansOneSheet(ast)) { - return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets) + return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets).withOrigin('reference') } const range = AbsoluteCellRange.fromCellRange(ast, state.formulaAddress) @@ -234,22 +237,22 @@ export class Interpreter { } case AstNodeType.COLUMN_RANGE: { if (!this.isSheetValid(ast.start) || !this.isSheetValid(ast.end)) { - return new CellError(ErrorType.REF, ErrorMessage.SheetRef) + return new CellError(ErrorType.REF, ErrorMessage.SheetRef).withOrigin('reference') } if (!this.rangeSpansOneSheet(ast)) { - return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets) + return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets).withOrigin('reference') } const range = AbsoluteColumnRange.fromColumnRange(ast, state.formulaAddress) return SimpleRangeValue.onlyRange(range, this.dependencyGraph) } case AstNodeType.ROW_RANGE: { if (!this.isSheetValid(ast.start) || !this.isSheetValid(ast.end)) { - return new CellError(ErrorType.REF, ErrorMessage.SheetRef) + return new CellError(ErrorType.REF, ErrorMessage.SheetRef).withOrigin('reference') } if (!this.rangeSpansOneSheet(ast)) { - return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets) + return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets).withOrigin('reference') } const range = AbsoluteRowRange.fromRowRangeAst(ast, state.formulaAddress) return SimpleRangeValue.onlyRange(range, this.dependencyGraph) @@ -523,6 +526,9 @@ function originNameForAstNode(ast: Ast): Maybe { switch (ast.type) { case AstNodeType.FUNCTION_CALL: return ast.procedureName + case AstNodeType.ARRAY: + // An array literal with mismatched rows builds its own error before any call sees it. + return 'array literal' case AstNodeType.CONCATENATE_OP: return 'concat' case AstNodeType.EQUALS_OP: @@ -558,6 +564,43 @@ function originNameForAstNode(ast: Ast): Maybe { } } +/** + * Stamps the origin function on errors sitting inside an array this node produced. + * + * An error that is one element of a returned array never reaches the scalar branch above, so + * without this it would arrive at the enclosing call unclaimed and that call would take it: an + * `#N/A` left by a broadcast, or a per-element coercion failure, would be reported as produced by + * whichever function happened to read the array. The operation that built the array is the one + * that produced them, so it is the one that signs. + * + * `withOrigin` is first-wins, so an element that already carries an origin keeps it, and the array + * is rebuilt only if at least one element actually changed. + * + * @param {SimpleRangeValue} val - the array this node produced + * @param {Ast} ast - the node being evaluated + */ +function stampOriginForArrayElements(val: SimpleRangeValue, ast: Ast): SimpleRangeValue { + const originName = originNameForAstNode(ast) + if (originName === undefined || val.range !== undefined) { + // A range-backed value is a view over cells, and an error read out of a cell was already + // marked as propagated when it was read, so there is nothing here to claim. Skipping it also + // keeps this off the hot path: reading `data` on such a value materialises the whole range. + return val + } + + let changed = false + const stamped = val.data.map(row => row.map(cell => { + if (!(cell instanceof CellError)) { + return cell + } + const withOrigin = cell.withOrigin(originName) + changed = changed || withOrigin !== cell + return withOrigin + })) + + return changed ? SimpleRangeValue.onlyValues(stamped) : val +} + /** * Stamps the origin function on an error, once per evaluation. * diff --git a/hyperformula/src/interpreter/plugin/NumericAggregationPlugin.ts b/hyperformula/src/interpreter/plugin/NumericAggregationPlugin.ts index 2bb056a758..0ddbf7d3bd 100644 --- a/hyperformula/src/interpreter/plugin/NumericAggregationPlugin.ts +++ b/hyperformula/src/interpreter/plugin/NumericAggregationPlugin.ts @@ -587,14 +587,14 @@ export class NumericAggregationPlugin extends FunctionPlugin implements Function range = AbsoluteCellRange.fromAst(ast, state.formulaAddress) } catch (err) { if (err instanceof SheetsNotEqual) { - return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets) + return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets).withOrigin('reference') } else { throw err } } if (!this.isSheetValid(range)) { - return new CellError(ErrorType.REF, ErrorMessage.SheetRef) + return new CellError(ErrorType.REF, ErrorMessage.SheetRef).withOrigin('reference') } const rangeVertex = this.dependencyGraph.getRange(range.start, range.end) diff --git a/hyperformula/src/parser/FormulaParser.ts b/hyperformula/src/parser/FormulaParser.ts index 3bfb977a44..b7efab1b7d 100644 --- a/hyperformula/src/parser/FormulaParser.ts +++ b/hyperformula/src/parser/FormulaParser.ts @@ -213,11 +213,11 @@ export class FormulaParser extends EmbeddedActionsParser { const secondAddress = this.ACTION(() => columnAddressFromString(endImage, this.formulaAddress, this.resolveSheetReference)) if (firstAddress === undefined || secondAddress === undefined) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference).withOrigin('reference')) } if (firstAddress.exceedsSheetSizeLimits(this.lexerConfig.maxColumns) || secondAddress.exceedsSheetSizeLimits(this.lexerConfig.maxColumns)) { - return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), range.leadingWhitespace) + return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize).withOrigin('reference'), range.leadingWhitespace) } if (firstAddress.sheet === undefined && secondAddress.sheet !== undefined) { @@ -239,11 +239,11 @@ export class FormulaParser extends EmbeddedActionsParser { const secondAddress = this.ACTION(() => rowAddressFromString(endImage, this.formulaAddress, this.resolveSheetReference)) if (firstAddress === undefined || secondAddress === undefined) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference).withOrigin('reference')) } if (firstAddress.exceedsSheetSizeLimits(this.lexerConfig.maxRows) || secondAddress.exceedsSheetSizeLimits(this.lexerConfig.maxRows)) { - return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), range.leadingWhitespace) + return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize).withOrigin('reference'), range.leadingWhitespace) } if (firstAddress.sheet === undefined && secondAddress.sheet !== undefined) { @@ -265,9 +265,9 @@ export class FormulaParser extends EmbeddedActionsParser { }) if (address === undefined) { - return buildErrorWithRawInputAst(cell.image, new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference), cell.leadingWhitespace) + return buildErrorWithRawInputAst(cell.image, new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference).withOrigin('reference'), cell.leadingWhitespace) } else if (address.exceedsSheetSizeLimits(this.lexerConfig.maxColumns, this.lexerConfig.maxRows)) { - return buildErrorWithRawInputAst(cell.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), cell.leadingWhitespace) + return buildErrorWithRawInputAst(cell.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize).withOrigin('reference'), cell.leadingWhitespace) } else { return buildCellReferenceAst(address, cell.leadingWhitespace) } @@ -288,12 +288,12 @@ export class FormulaParser extends EmbeddedActionsParser { if (startAddress === undefined || endAddress === undefined) { return this.ACTION(() => { - return buildErrorWithRawInputAst(`${start.image}:${end.image}`, new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference), start.leadingWhitespace) + return buildErrorWithRawInputAst(`${start.image}:${end.image}`, new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference).withOrigin('reference'), start.leadingWhitespace) }) } else if (startAddress.exceedsSheetSizeLimits(this.lexerConfig.maxColumns, this.lexerConfig.maxRows) || endAddress.exceedsSheetSizeLimits(this.lexerConfig.maxColumns, this.lexerConfig.maxRows)) { return this.ACTION(() => { - return buildErrorWithRawInputAst(`${start.image}:${end.image}`, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), start.leadingWhitespace) + return buildErrorWithRawInputAst(`${start.image}:${end.image}`, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize).withOrigin('reference'), start.leadingWhitespace) }) } @@ -319,7 +319,7 @@ export class FormulaParser extends EmbeddedActionsParser { return cellAddressFromString(start.image, this.formulaAddress, this.resolveSheetReference) }) if (startAddress === undefined) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference).withOrigin('reference')) } if (offsetProcedure.type === AstNodeType.CELL_REFERENCE) { return this.buildCellRange(startAddress, offsetProcedure.reference, start.leadingWhitespace?.image) @@ -352,7 +352,7 @@ export class FormulaParser extends EmbeddedActionsParser { if (endAddress === undefined) { return this.ACTION(() => { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference).withOrigin('reference')) }) } @@ -552,7 +552,7 @@ export class FormulaParser extends EmbeddedActionsParser { const errString = token.image.toUpperCase() const errorType = this.lexerConfig.errorMapping[errString] if (errorType) { - return buildCellErrorAst(new CellError(errorType, ErrorMessage.ErrorLiteral), token.leadingWhitespace) + return buildCellErrorAst(new CellError(errorType, ErrorMessage.ErrorLiteral).withOrigin('literal'), token.leadingWhitespace) } else { return this.parsingError(ParsingErrorType.ParserError, 'Unknown error literal') } @@ -835,7 +835,7 @@ export class FormulaParser extends EmbeddedActionsParser { } if (absoluteCol < 0 || absoluteRow < 0) { - return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.OutOfSheet)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.OutOfSheet).withOrigin('OFFSET')) } if (width === 1 && height === 1) { return buildCellReferenceAst(topLeftCorner)