From 43ef5f52c0f18f87bfabd76df1fe0a07abfb83f1 Mon Sep 17 00:00:00 2001 From: marcin-kordas-hoc Date: Mon, 7 Sep 2026 08:45:12 +0000 Subject: [PATCH 1/7] HF-131: stop a cell adopting an error it only read Reading another cell's value funnels through DependencyGraph.getCellValue, so that is where an error now gets marked as propagated -- one choke point instead of a marking at each interpreter case, which is what let three read paths (named expressions, range reads via SimpleRangeValue, and getScalarValue's aggregation callers) slip through an earlier attempt. getScalarValue delegates to getCellValue rather than reading the address mapping itself, so its callers inherit the mark instead of bypassing it. wrapperForRootVertex then skips a propagated error, so the reading cell is no longer attached as its root. A vertex holding a STATIC error has no FormulaVertex to serve as a lazily-resolved root, so getCellValue stamps the address it already has; CellError is immutable, so this copies rather than mutating what is stored, which is what keeps the address correct across row and column changes. evaluateAst's overflow guard now assigns instead of returning early, so a NaN/Infinity result reaches the same postprocessing as every other value and gets a root like anything else -- previously =SQRT(-1) had neither. Measured on this branch: A1='=1+' reports Sheet1!A1 where it reported no address; B1='=A1' reports Sheet1!A1 where it reported its own address; the same holds two hops out, through a range, and through a named expression; it follows the cell across addRows; and =1/0 with a propagating SUM still reports A1. Full private suite: 5 failed / 6157 passed, byte-identical to this worktree's baseline (those 5 are a pre-existing branch-pinning mismatch in the test repo). tsc and eslint clean. Also consolidates the toEqualError matcher. It existed twice, once per runner, with hand-copied logic and two strip lists that could drift -- and had drifted in a way that made a change appear inert under Jest. The decision now lives in one shared module both wrappers call with their own equality function, so there is a single strip list to extend. Verified by short-circuiting the shared module: 30 tests went red and came back green, proving Jest really reads it. The Karma half could not be executed here (no browser in this environment). NOT yet observable through the public API: originFunction and argumentIndex are carried on CellError and stamped in evaluateAst, but DetailedCellError does not expose them yet. Exposing them is one line plus a test migration -- 10 assertions in error-address-preservation.spec.ts, arrays.spec.ts and matrix-plugin.spec.ts use plain toEqual, which no matcher edit reaches, and they must NOT be moved to toEqualError because it also strips address, which is exactly what those tests assert. That split is the natural PR boundary here. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 + src/Cell.ts | 69 ++++++++++++++++- src/DependencyGraph/DependencyGraph.ts | 27 +++++-- src/Exporter.ts | 2 +- src/interpreter/Interpreter.ts | 75 ++++++++++++++++++- test/_setupFiles/jest/toEqualError.ts | 51 ++++++------- .../matchers/cellErrorComparison.ts | 74 ++++++++++++++++++ test/_setupFiles/matchers/toEqualError.ts | 14 +--- 8 files changed, 266 insertions(+), 50 deletions(-) create mode 100644 test/_setupFiles/matchers/cellErrorComparison.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 026e60695b..5b70844051 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Changed the cell errors thrown by the formula engine to always carry a message describing their cause. [#131](https://github.com/handsontable/hyperformula/issues/131) +### Fixed + +- A cell error read from another cell no longer reports the reading cell as its origin. Errors that come from a cell holding a static error value — a formula with a syntax error, or an error value entered directly — now report that cell's address instead of the address of whichever cell happened to read them first. [#131](https://github.com/handsontable/hyperformula/issues/131) + ## [3.4.0] - 2026-08-10 ### Added diff --git a/src/Cell.ts b/src/Cell.ts index faab24babe..1bcd808ab1 100644 --- a/src/Cell.ts +++ b/src/Cell.ts @@ -150,10 +150,75 @@ export class CellError { constructor( public readonly type: ErrorType, public readonly message?: string, - public readonly root?: FormulaVertex + public readonly root?: FormulaVertex, + public readonly originFunction?: string, + public readonly argumentIndex?: number, + public readonly propagated: boolean = false, + public readonly originAddress?: SimpleCellAddress, ) { } + /** + * Names the function or operator that produced this error. + * + * First occurrence wins, so an outer function that merely received the error back does not + * claim it: `=SUM(SQRT(-1))` keeps `SQRT`. A propagated error — one read out of another cell + * rather than produced here — can never acquire an origin it did not already have. + * + * @param {string} functionName - the function or operator helper that produced the error + */ + public withOrigin(functionName: string): CellError { + if (this.propagated || this.originFunction !== undefined) { + return this + } + return new CellError(this.type, this.message, this.root, functionName, this.argumentIndex, this.propagated, this.originAddress) + } + + /** + * Records which argument of the origin function was rejected. First occurrence wins, and a + * propagated error never acquires one: the reading function's own argument slot is not the + * offending argument. + * + * @param {number} index - zero-based index of the offending argument + */ + public withArgumentIndex(index: number): CellError { + if (this.propagated || this.argumentIndex !== undefined) { + return this + } + return new CellError(this.type, this.message, this.root, this.originFunction, index, this.propagated, this.originAddress) + } + + /** + * Marks this error as read from another cell rather than produced by the current evaluation. + * + * This is what stops a cell from adopting an error it only read: {@link attachRootVertex} skips + * a propagated error, so the reported address stays the one where the error actually appeared. + */ + public asPropagated(): CellError { + if (this.propagated) { + return this + } + return new CellError(this.type, this.message, this.root, this.originFunction, this.argumentIndex, true, this.originAddress) + } + + /** + * Records the address an otherwise rootless error appeared at. + * + * For vertices that hold a static error rather than computing one (a parse error, or an error + * value the user typed), there is no `FormulaVertex` to serve as a lazily-resolved `root`. This + * cannot go stale: `CellError` is immutable, so the stored value is never stamped — every read + * stamps a fresh copy with the address current at that moment, which is what keeps it correct + * across row and column changes. + * + * @param {SimpleCellAddress} address - the address the error was read from + */ + public withOriginAddress(address: SimpleCellAddress): CellError { + if (this.originAddress !== undefined) { + return this + } + return new CellError(this.type, this.message, this.root, this.originFunction, this.argumentIndex, this.propagated, address) + } + /** * Returns a CellError with a given message. * @param {string} detailedMessage - message to be displayed @@ -164,7 +229,7 @@ export class CellError { public attachRootVertex(vertex: FormulaVertex): CellError { if (this.root === undefined) { - return new CellError(this.type, this.message, vertex) + return new CellError(this.type, this.message, vertex, this.originFunction, this.argumentIndex, this.propagated, this.originAddress) } else { return this } diff --git a/src/DependencyGraph/DependencyGraph.ts b/src/DependencyGraph/DependencyGraph.ts index 962dedff1a..d28f7d3847 100644 --- a/src/DependencyGraph/DependencyGraph.ts +++ b/src/DependencyGraph/DependencyGraph.ts @@ -612,7 +612,24 @@ export class DependencyGraph { return new CellError(ErrorType.REF, ErrorMessage.SheetRef) } - return this.addressMapping.getCellValue(address) + const value = this.addressMapping.getCellValue(address) + if (!(value instanceof CellError)) { + return value + } + + // Every read of another cell's value funnels through here, which makes this the one place + // that can tell "this error was produced by the current evaluation" from "this error was + // read from somewhere else". Marking it propagated is what stops the reading cell from + // being reported as the origin. + // + // A vertex holding a STATIC error (a parse error, or an error value the user typed) has no + // FormulaVertex to act as a root, so the address is stamped here instead. The stamp cannot + // go stale: CellError is immutable, so this copies rather than mutating what is stored. + const vertex = this.addressMapping.getCell(address) + const holdsAStaticError = vertex instanceof ParsingErrorVertex || vertex instanceof ValueCellVertex + return holdsAStaticError + ? value.withOriginAddress(address).asPropagated() + : value.asPropagated() } public getRawValue(address: SimpleCellAddress): RawCellContent { @@ -624,11 +641,9 @@ export class DependencyGraph { } public getScalarValue(address: SimpleCellAddress): InternalScalarValue { - if (this.isPlaceholder(address.sheet)) { - return new CellError(ErrorType.REF, ErrorMessage.SheetRef) - } - - const value = this.addressMapping.getCellValue(address) + // Delegates rather than reading the address mapping itself, so that every caller of this + // method inherits the propagated marking above instead of bypassing it. + const value = this.getCellValue(address) if (value instanceof SimpleRangeValue) { return new CellError(ErrorType.VALUE, ErrorMessage.ScalarExpected) } diff --git a/src/Exporter.ts b/src/Exporter.ts index c5ff24f73b..1efadc1bd8 100644 --- a/src/Exporter.ts +++ b/src/Exporter.ts @@ -115,7 +115,7 @@ export class Exporter implements ChangeExporter { private detailedError(error: CellError): DetailedCellError { let address = undefined - const originAddress = error.root?.getAddress(this.lazilyTransformingService) + const originAddress = error.root?.getAddress(this.lazilyTransformingService) ?? error.originAddress if (originAddress !== undefined) { if (originAddress.sheet === NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS) { address = this.namedExpressions.namedExpressionInAddress(originAddress.row)?.displayName diff --git a/src/interpreter/Interpreter.ts b/src/interpreter/Interpreter.ts index b66e04a478..9f066daf6c 100644 --- a/src/interpreter/Interpreter.ts +++ b/src/interpreter/Interpreter.ts @@ -65,7 +65,9 @@ export class Interpreter { let val = this.evaluateAstWithoutPostprocessing(ast, state) if (isExtendedNumber(val)) { if (isNumberOverflow(getRawValue(val))) { - return new CellError(ErrorType.NUM, ErrorMessage.NaN) + // Assigned rather than returned, so an overflow error reaches the same postprocessing + // as every other result below and gets an origin and a root like anything else. + val = new CellError(ErrorType.NUM, ErrorMessage.NaN) } else { val = cloneNumber(val, fixNegativeZero(getRawValue(val))) } @@ -73,6 +75,9 @@ export class Interpreter { if (val instanceof SimpleRangeValue && val.height() === 1 && val.width() === 1) { [[val]] = val.data } + if (val instanceof CellError) { + val = stampOriginForAstNode(val, ast) + } return wrapperForRootVertex(val, state.formulaVertex) } @@ -493,7 +498,73 @@ function binaryErrorWrapper(op: (arg1: T, arg2: T) = function wrapperForRootVertex(val: InterpreterValue, vertex?: FormulaVertex): InterpreterValue { if (val instanceof CellError && vertex !== undefined) { - return val.attachRootVertex(vertex) + // A propagated error was read from another cell, so this cell is not its origin. + return val.propagated ? val : val.attachRootVertex(vertex) } return val } + +/** + * The name recorded as an error's `originFunction` when evaluating this AST node is what produced + * it: the function name for a call, or the `ArithmeticHelper` method the operator dispatches to. + * + * Every name below was read off this file's own operator definitions rather than guessed, which is + * why `PLUS_OP` maps to `addWithEpsilon` and `PERCENT_OP` to `unaryPercent`. Any other node type + * returns undefined and leaves whatever origin the value already carries untouched. + * + * @param {Ast} ast - the node being evaluated + */ +function originNameForAstNode(ast: Ast): Maybe { + switch (ast.type) { + case AstNodeType.FUNCTION_CALL: + return ast.procedureName + case AstNodeType.CONCATENATE_OP: + return 'concat' + case AstNodeType.EQUALS_OP: + return 'eq' + case AstNodeType.NOT_EQUAL_OP: + return 'neq' + case AstNodeType.GREATER_THAN_OP: + return 'gt' + case AstNodeType.LESS_THAN_OP: + return 'lt' + case AstNodeType.GREATER_THAN_OR_EQUAL_OP: + return 'geq' + case AstNodeType.LESS_THAN_OR_EQUAL_OP: + return 'leq' + case AstNodeType.PLUS_OP: + return 'addWithEpsilon' + case AstNodeType.MINUS_OP: + return 'subtract' + case AstNodeType.TIMES_OP: + return 'multiply' + case AstNodeType.POWER_OP: + return 'pow' + case AstNodeType.DIV_OP: + return 'divide' + case AstNodeType.PLUS_UNARY_OP: + return 'unaryPlus' + case AstNodeType.MINUS_UNARY_OP: + return 'unaryMinus' + case AstNodeType.PERCENT_OP: + return 'unaryPercent' + default: + return undefined + } +} + +/** + * Stamps the origin function on an error, once per evaluation. + * + * Placed in `evaluateAst`'s single postprocessing tail rather than in each `switch` case, so it + * also catches the overflow guard's freshly-minted error, and so no case can be forgotten. Every + * nested sub-expression runs its own `evaluateAst` first, so an inner error is always stamped + * before an outer function could overwrite it — and `withOrigin` is a no-op once set anyway. + * + * @param {CellError} val - the error produced by this evaluation + * @param {Ast} ast - the node being evaluated + */ +function stampOriginForAstNode(val: CellError, ast: Ast): CellError { + const originName = originNameForAstNode(ast) + return originName === undefined ? val : val.withOrigin(originName) +} diff --git a/test/_setupFiles/jest/toEqualError.ts b/test/_setupFiles/jest/toEqualError.ts index 80f274cfc2..1af55cb5f2 100644 --- a/test/_setupFiles/jest/toEqualError.ts +++ b/test/_setupFiles/jest/toEqualError.ts @@ -1,29 +1,22 @@ -type CustomMatcherResult = jest.CustomMatcherResult -type ExpectExtendMap = jest.ExpectExtendMap - -declare global { - namespace jest { - interface Matchers { - toEqualError(expected: any): CustomMatcherResult, - } - } -} - -export const toEqualError: ExpectExtendMap = { - toEqualError(received: any, expected: any): CustomMatcherResult { - let result = false - - if (typeof received === 'object' && typeof expected === 'object' && received.message != null && expected.message != null && received.message.includes(expected.message)) { - result = this.equals( - {...received, message: undefined, root: undefined, address: undefined}, - {...expected, message: undefined, root: undefined, address: undefined} - ) - } else { - result = this.equals(received, expected) - } - return { - pass: result, - message: () => (result ? '' : `Expected ${JSON.stringify(received, null, 2)} to match ${JSON.stringify(expected, null, 2)}.`) - } - } -} +import { cellErrorMismatchMessage, cellErrorsMatch } from '../matchers/cellErrorComparison' + +type CustomMatcherResult = jest.CustomMatcherResult +type ExpectExtendMap = jest.ExpectExtendMap + +declare global { + namespace jest { + interface Matchers { + toEqualError(expected: any): CustomMatcherResult, + } + } +} + +export const toEqualError: ExpectExtendMap = { + toEqualError(received: any, expected: any): CustomMatcherResult { + const result = cellErrorsMatch(received, expected, (a, b) => this.equals(a, b)) + return { + pass: result, + message: () => (result ? '' : cellErrorMismatchMessage(received, expected)) + } + } +} diff --git a/test/_setupFiles/matchers/cellErrorComparison.ts b/test/_setupFiles/matchers/cellErrorComparison.ts new file mode 100644 index 0000000000..8d391e833d --- /dev/null +++ b/test/_setupFiles/matchers/cellErrorComparison.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright (c) 2025 Handsoncode. All rights reserved. + */ + +/** + * The one definition of what `toEqualError` compares, shared by both runners. + * + * The suite runs under Jest AND Karma/Jasmine, and each has its own matcher wrapper + * (`_setupFiles/jest/toEqualError.ts`, `_setupFiles/matchers/toEqualError.ts`) because the + * two runners register matchers differently and hand in a different equality helper. Only + * the wrapper differs: the decision below used to be copy-pasted into both, and drifted — + * a field added to one strip list and not the other passes under Jest while the browser + * run reports hundreds of failures for a reason no diff explains. Keeping the decision here + * makes that class of bug impossible: there is exactly one strip list to extend. + */ + +/** Equality, supplied by whichever runner is active (`this.equals` in Jest, `util.equals` in Jasmine). */ +export type EqualsFn = (actual: unknown, expected: unknown) => boolean + +/** + * Fields excluded from the structural comparison. + * + * These are asserted explicitly by the specs that care about them, rather than implicitly by + * every error assertion in the suite. `message` is excluded because it is matched by substring + * above; `root` and `address` because an expectation built by the `detailedError` test helper + * cannot know the vertex or address the engine resolved. + * + * ADDING A FIELD TO `CellError` OR `DetailedCellError`? Add it here, once. Do not add it to a + * runner wrapper. + */ +const IGNORED_IN_STRUCTURAL_COMPARE = { + message: undefined, + root: undefined, + address: undefined, + originFunction: undefined, + argumentIndex: undefined, +} + +/** + * Whether a received error matches an expected one. + * + * When both sides carry a message, the expected message is treated as a substring of the + * received one and the remaining fields are compared structurally. Otherwise the two values + * are compared as-is — which is what makes an expectation on a raw, message-less `CellError` + * a strict, whole-object comparison. + * + * @param received - the value produced by the engine + * @param expected - the value the spec asserts + * @param equals - the active runner's deep-equality function + */ +export function cellErrorsMatch(received: any, expected: any, equals: EqualsFn): boolean { + const bothCarryAMessage = typeof received === 'object' && typeof expected === 'object' + && received.message != null && expected.message != null + + if (bothCarryAMessage && received.message.includes(expected.message)) { + return equals( + {...received, ...IGNORED_IN_STRUCTURAL_COMPARE}, + {...expected, ...IGNORED_IN_STRUCTURAL_COMPARE}, + ) + } + + return equals(received, expected) +} + +/** + * The failure text both runners print, so a mismatch reads identically whichever one found it. + * + * @param received - the value produced by the engine + * @param expected - the value the spec asserts + */ +export function cellErrorMismatchMessage(received: any, expected: any): string { + return `Expected ${JSON.stringify(received, null, 2)} to match ${JSON.stringify(expected, null, 2)}.` +} diff --git a/test/_setupFiles/matchers/toEqualError.ts b/test/_setupFiles/matchers/toEqualError.ts index 6c0b87bb56..65e3a171db 100644 --- a/test/_setupFiles/matchers/toEqualError.ts +++ b/test/_setupFiles/matchers/toEqualError.ts @@ -1,3 +1,5 @@ +import { cellErrorMismatchMessage, cellErrorsMatch } from './cellErrorComparison' + type CustomMatcher = jasmine.CustomMatcher type CustomMatcherFactories = jasmine.CustomMatcherFactories type CustomMatcherResult = jasmine.CustomMatcherResult @@ -15,18 +17,10 @@ export const toEqualErrorMatcher: CustomMatcherFactories = { toEqualError: function(util: MatchersUtil): CustomMatcher { return { compare: function(received: any, expected: any): CustomMatcherResult { - let result - if (typeof received === 'object' && typeof expected === 'object' && received.message != null && expected.message != null && received.message.includes(expected.message)) { - result = util.equals( - {...received, message: undefined, root: undefined, address: undefined}, - {...expected, message: undefined, root: undefined, address: undefined} - ) - } else { - result = util.equals(received, expected) - } + const result = cellErrorsMatch(received, expected, (a, b) => util.equals(a, b)) return { pass: result, - message: result ? '' : `Expected ${JSON.stringify(received, null, 2)} to match ${JSON.stringify(expected, null, 2)}.` + message: result ? '' : cellErrorMismatchMessage(received, expected) } }, } From 8b9826fb2b8cd661f800932955e2cbcb2dc8d2f4 Mon Sep 17 00:00:00 2001 From: marcin-kordas-hoc Date: Tue, 8 Sep 2026 05:43:32 +0000 Subject: [PATCH 2/7] HF-131: give every non-interpreter cell error a message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 2 of the HF-131 explainability plan: the 19 CellError construction sites outside src/interpreter/ (17) plus src/Evaluator.ts's two explicit-undefined CYCLE sites that passed no message at all now do, mirroring what Task 1 already did for src/interpreter/. Seven new catalogue constants cover semantically distinct groups rather than one generic REF/NAME sentence: a removed-reference in an existing formula (Transformer.ts, 4 sites), an array's source cell being overwritten (DependencyGraph.ts), a position outside an array's own computed bounds (FormulaVertex.ts), an unresolved parser reference (FormulaParser.ts, 6 REF sites), a reference exceeding sheet size limits (FormulaParser.ts, 4 NAME sites), an error value typed directly into a formula (FormulaParser.ts, 1 site), and a circular reference (Evaluator.ts, 2 CYCLE sites). No error type changed, no function signature changed, i18n/Cell.ts/ CellValue.ts/Exporter.ts/the DependencyGraph choke point untouched. Originally attempted via prep-ship's autonomous pipeline; the run crashed twice with an empty error at its own 'authoring AC specs' step (unrelated to this diff — see the private-test patch README for the diagnosis). The production src/ implementation it left behind was verified correct against every constraint in the task file and kept; the pre-existing private-test breakage it hadn't yet reached was fixed by hand. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 1 + src/DependencyGraph/DependencyGraph.ts | 2 +- src/DependencyGraph/FormulaVertex.ts | 2 +- src/Evaluator.ts | 5 +++-- src/dependencyTransformers/Transformer.ts | 9 +++++---- src/error-message.ts | 7 +++++++ src/parser/FormulaParser.ts | 22 +++++++++++----------- 7 files changed, 29 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b70844051..ec3c51da11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - Changed the cell errors thrown by the formula engine to always carry a message describing their cause. [#131](https://github.com/handsontable/hyperformula/issues/131) +- Changed the cell errors thrown outside the formula interpreter — invalidated references, references the parser cannot resolve, references exceeding the sheet size, circular references, cleaned-up array results, and error values typed directly into a formula — to also carry a message describing their cause. [#131](https://github.com/handsontable/hyperformula/issues/131) ### Fixed diff --git a/src/DependencyGraph/DependencyGraph.ts b/src/DependencyGraph/DependencyGraph.ts index d28f7d3847..68537a306d 100644 --- a/src/DependencyGraph/DependencyGraph.ts +++ b/src/DependencyGraph/DependencyGraph.ts @@ -981,7 +981,7 @@ export class DependencyGraph { const oldValue = vertex.getArrayCellValue(address) if (this.getCell(address) === vertex) { if (vertex.isLeftCorner(address)) { - this.changes.addChange(new CellError(ErrorType.REF), address, oldValue) + this.changes.addChange(new CellError(ErrorType.REF, ErrorMessage.ArraySourceRemoved), address, oldValue) } else { this.addressMapping.removeCell(address) this.changes.addChange(EmptyValue, address, oldValue) diff --git a/src/DependencyGraph/FormulaVertex.ts b/src/DependencyGraph/FormulaVertex.ts index 5aa448ffd8..84f54499ab 100644 --- a/src/DependencyGraph/FormulaVertex.ts +++ b/src/DependencyGraph/FormulaVertex.ts @@ -155,7 +155,7 @@ export class ArrayFormulaVertex extends FormulaVertex { try { return this.array.get(col, row) } catch (e) { - return new CellError(ErrorType.REF) + return new CellError(ErrorType.REF, ErrorMessage.OutsideArrayResult) } } diff --git a/src/Evaluator.ts b/src/Evaluator.ts index f810bee363..ecf234dfd9 100644 --- a/src/Evaluator.ts +++ b/src/Evaluator.ts @@ -10,6 +10,7 @@ import {Config} from './Config' import {ContentChanges} from './ContentChanges' import {ArrayFormulaVertex, DependencyGraph, RangeVertex, Vertex} from './DependencyGraph' import {FormulaVertex} from './DependencyGraph/FormulaVertex' +import {ErrorMessage} from './error-message' import {Interpreter} from './interpreter/Interpreter' import {InterpreterState} from './interpreter/InterpreterState' import {EmptyValue, getRawValue, InterpreterValue} from './interpreter/InterpreterValue' @@ -105,7 +106,7 @@ export class Evaluator { } else if (vertex instanceof FormulaVertex) { const address = vertex.getAddress(this.lazilyTransformingAstService) this.columnSearch.remove(getRawValue(vertex.valueOrUndef()), address) - const error = new CellError(ErrorType.CYCLE, undefined, vertex) + const error = new CellError(ErrorType.CYCLE, ErrorMessage.CircularReference, vertex) vertex.setCellValue(error) changes.addChange(error, address) } @@ -117,7 +118,7 @@ export class Evaluator { private recomputeFormulas(cycled: Vertex[], sorted: Vertex[]): void { cycled.forEach((vertex: Vertex) => { if (vertex instanceof FormulaVertex) { - vertex.setCellValue(new CellError(ErrorType.CYCLE, undefined, vertex)) + vertex.setCellValue(new CellError(ErrorType.CYCLE, ErrorMessage.CircularReference, vertex)) } }) sorted.forEach((vertex: Vertex) => { diff --git a/src/dependencyTransformers/Transformer.ts b/src/dependencyTransformers/Transformer.ts index 487f8bf918..fc937d1c27 100644 --- a/src/dependencyTransformers/Transformer.ts +++ b/src/dependencyTransformers/Transformer.ts @@ -5,6 +5,7 @@ import {CellError, ErrorType, SimpleCellAddress} from '../Cell' import {DependencyGraph} from '../DependencyGraph' +import {ErrorMessage} from '../error-message' import { Ast, AstNodeType, @@ -112,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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) } else { return ast } @@ -123,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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) } else { return ast } @@ -134,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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) } else { return ast } @@ -145,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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.ReferenceRemoved)) } else { return ast } diff --git a/src/error-message.ts b/src/error-message.ts index 3fe97e24ef..cc2d687e61 100644 --- a/src/error-message.ts +++ b/src/error-message.ts @@ -14,6 +14,8 @@ export class ErrorMessage { public static ArrayDimensions = 'Array dimensions are not compatible.' public static NoBroadcastValue = 'Operand ranges cannot be broadcast to a common size, so this position has no value.' public static NoSpaceForArrayResult = 'No space for array result.' + public static ArraySourceRemoved = 'Source cell of the array result was overwritten or removed.' + public static OutsideArrayResult = 'Position is outside the array result of this formula.' public static ValueSmall = 'Value too small.' public static ValueLarge = 'Value too large.' public static BadCriterion = 'Incorrect criterion.' @@ -28,6 +30,10 @@ export class ErrorMessage { public static BadMode = 'Mode not recognized.' public static DateBounds = 'Date outside of bounds.' public static OutOfSheet = 'Resulting reference is out of the sheet.' + public static UnresolvedReference = 'Reference in the formula could not be resolved.' + public static ReferenceRemoved = 'Reference points to a row or column that was removed.' + public static ReferenceExceedsSheetSize = 'Reference exceeds the maximum row or column size of the sheet.' + public static CircularReference = 'Cell is part of a circular reference.' public static WrongType = 'Wrong type of argument.' public static NaN = 'NaN or infinite value encountered.' public static EqualLength = 'Ranges need to be of equal length.' @@ -57,6 +63,7 @@ export class ErrorMessage { public static Formula = 'Expected formula.' public static NegativeCount = 'Count cannot be negative.' public static ParseError = 'Parsing error.' + public static ErrorLiteral = 'Error value was entered directly into the formula.' public static SheetRef = 'Sheet does not exist.' public static PeriodLong = 'Period number cannot exceed life length.' public static NonZeroPeriods = 'Number of periods cannot be 0 when the rate is -1.' diff --git a/src/parser/FormulaParser.ts b/src/parser/FormulaParser.ts index 82c70e1c1d..3bfb977a44 100644 --- a/src/parser/FormulaParser.ts +++ b/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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) } if (firstAddress.exceedsSheetSizeLimits(this.lexerConfig.maxColumns) || secondAddress.exceedsSheetSizeLimits(this.lexerConfig.maxColumns)) { - return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME), range.leadingWhitespace) + return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), 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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) } if (firstAddress.exceedsSheetSizeLimits(this.lexerConfig.maxRows) || secondAddress.exceedsSheetSizeLimits(this.lexerConfig.maxRows)) { - return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME), range.leadingWhitespace) + return buildErrorWithRawInputAst(range.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), 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), cell.leadingWhitespace) + return buildErrorWithRawInputAst(cell.image, new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference), cell.leadingWhitespace) } else if (address.exceedsSheetSizeLimits(this.lexerConfig.maxColumns, this.lexerConfig.maxRows)) { - return buildErrorWithRawInputAst(cell.image, new CellError(ErrorType.NAME), cell.leadingWhitespace) + return buildErrorWithRawInputAst(cell.image, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), 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), start.leadingWhitespace) + return buildErrorWithRawInputAst(`${start.image}:${end.image}`, new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference), 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), start.leadingWhitespace) + return buildErrorWithRawInputAst(`${start.image}:${end.image}`, new CellError(ErrorType.NAME, ErrorMessage.ReferenceExceedsSheetSize), 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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) } 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)) + return buildCellErrorAst(new CellError(ErrorType.REF, ErrorMessage.UnresolvedReference)) }) } @@ -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), token.leadingWhitespace) + return buildCellErrorAst(new CellError(errorType, ErrorMessage.ErrorLiteral), token.leadingWhitespace) } else { return this.parsingError(ParsingErrorType.ParserError, 'Unknown error literal') } From 128f261706324334d66f8d1d758049c8c71a974d Mon Sep 17 00:00:00 2001 From: marcin-kordas-hoc Date: Thu, 10 Sep 2026 13:31:53 +0000 Subject: [PATCH 3/7] HF-131: add the ESLint rule, scoped to src/interpreter/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enforces the guarantee this PR's message-filling work establishes: a message-less new CellError(...) construction under src/interpreter/ is now a lint error. Two selectors — a bare one-argument call, and an explicit literal undefined as the second argument (the exact shape the two CYCLE sites in src/Evaluator.ts still use; Evaluator.ts is outside src/interpreter/ so this rule doesn't reach them yet, and is unaffected by this commit). Verified: 0 lint errors on the full src/ tree (same pre-existing warning count as before this commit — no new warnings). The rule was proven to actually fire, not just parse, by temporarily reverting one src/interpreter/ plugin site to a bare 'new CellError(ErrorType.NUM)', observing the expected lint error, then restoring it (git diff empty afterward). Widened to all of src/ in the next PR in this stack (feat/hf-131-error-messages-outside-interpreter). Co-Authored-By: Claude Sonnet 5 --- .eslintrc.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index a5e9976e13..eb513cc6be 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -143,6 +143,25 @@ module.exports = { 'sort-keys': ['error', 'asc'], } }, + { + files: ['**/src/interpreter/**/*.ts'], + rules: { + // HF-131: an engine-thrown cell error must always state its cause. + // The public `CellError` constructor keeps `message` optional for custom + // functions (docs/guide/custom-functions.md); this rule enforces the + // stricter contract on our own code only. + 'no-restricted-syntax': ['error', + { + selector: "NewExpression[callee.name='CellError'][arguments.length<2]", + message: 'HF-131: pass an ErrorMessage constant as the second argument — no cell error may be thrown without a cause.', + }, + { + selector: "NewExpression[callee.name='CellError'] > Identifier.arguments:nth-child(2)[name='undefined']", + message: 'HF-131: an explicit undefined message is not allowed — pass an ErrorMessage constant.', + }, + ], + }, + }, { files: ['**/*.spec.ts'], rules: { From 9c4015e9eb73c1a87a139d4a02ff364be553d861 Mon Sep 17 00:00:00 2001 From: marcin-kordas-hoc Date: Thu, 10 Sep 2026 13:33:57 +0000 Subject: [PATCH 4/7] HF-131: widen the ESLint rule to all of src/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes the override's files glob from src/interpreter/ to all of src/, so the guard this PR's own message-filling work (and the earlier commit's interpreter-only rule) establishes covers the whole engine, not just the interpreter plugins. Documents the one call site the rule structurally cannot catch: CellContentParser.ts's CellContent.Error constructor keeps message optional on purpose (its one no-message caller — a user typing e.g. #REF! literally into a cell — has no engine-side cause to state), and the call 'new CellError(errorType, message)' is syntactically a two-argument construction that passes the rule cleanly regardless of what the caller actually passes. Verified: 0 lint errors on the full src/ tree (same pre-existing warning count as before this commit). The widened rule was proven to actually reach files outside src/interpreter/ by temporarily reverting one CYCLE site in src/Evaluator.ts to a bare 'new CellError(ErrorType.CYCLE)', observing the expected lint error, then restoring it (git diff empty afterward). Co-Authored-By: Claude Sonnet 5 --- .eslintrc.js | 6 ++++-- src/CellContentParser.ts | 9 +++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index eb513cc6be..47dd485932 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -144,12 +144,14 @@ module.exports = { } }, { - files: ['**/src/interpreter/**/*.ts'], + files: ['**/src/**/*.ts'], rules: { // HF-131: an engine-thrown cell error must always state its cause. // The public `CellError` constructor keeps `message` optional for custom // functions (docs/guide/custom-functions.md); this rule enforces the - // stricter contract on our own code only. + // stricter contract on our own code only. It is a syntactic floor, not a + // proof of completeness — see the comment at CellContentParser.ts's + // `CellContent.Error` constructor for a call site it cannot catch. 'no-restricted-syntax': ['error', { selector: "NewExpression[callee.name='CellError'][arguments.length<2]", diff --git a/src/CellContentParser.ts b/src/CellContentParser.ts index 916068ae00..4a14b443d2 100644 --- a/src/CellContentParser.ts +++ b/src/CellContentParser.ts @@ -61,6 +61,15 @@ export namespace CellContent { export class Error { public readonly value: CellError + // HF-131: `message` stays optional here on purpose. This constructor's only + // no-message caller is the `errorMapping` lookup below (a user typing e.g. + // `#REF!` literally into a cell) — the engine has no cause to state beyond + // "the user wrote this", so leaving it unset is honest, not an oversight. + // This is also the concrete case the HF-131 ESLint rule (.eslintrc.js) cannot + // catch: the call `new CellError(errorType, message)` is syntactically a + // two-argument construction and passes the rule cleanly even when the + // 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) } From e33b35f96da8bc98633dc32e5ff3aaad0fd5267b Mon Sep 17 00:00:00 2001 From: marcin-kordas-hoc Date: Mon, 14 Sep 2026 09:10:49 +0000 Subject: [PATCH 5/7] HF-131: keep a static error's address current as rows and columns move A cell that holds a static error - a formula the parser rejected, or an error value entered directly - has no formula vertex to resolve an address through, so DependencyGraph.getCellValue records the address on the error value itself when a dependent reads it. The reading cell caches that value and is not recomputed by a row or column change that merely shifts it, so the recorded address went stale: the dependent kept naming the pre-shift cell, which by then could be empty or hold an unrelated value. The record now carries the LazilyTransformingAstService version it was taken at, and Exporter replays the intervening transformations over it, the same way an error produced by a formula already resolves its address through its root vertex. The replay reuses each transformation's own transformSingleAst with an empty node rather than fixNodeAddress, because MoveCellsTransformer only moves an address that lies inside the range being moved and that decision lives in transformSingleAst. Once a compaction has discarded the transformations the record predates, the address cannot be reconstructed; it is omitted rather than guessed. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 ++--- src/Cell.ts | 27 ++++++++++------- src/DependencyGraph/DependencyGraph.ts | 8 +++-- src/Exporter.ts | 25 ++++++++++++++- src/LazilyTransformingAstService.ts | 42 +++++++++++++++++++++++++- 5 files changed, 88 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07ee8baee8..c8dcea21a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,14 +10,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - Changed the cell errors thrown by the formula engine to always carry a message describing their cause. [#131](https://github.com/handsontable/hyperformula/issues/131) -- Changed the cell errors thrown outside the formula interpreter — invalidated references, references the parser cannot resolve, references exceeding the sheet size, circular references, cleaned-up array results, and error values typed directly into a formula — to also carry a message describing their cause. [#131](https://github.com/handsontable/hyperformula/issues/131) - -### Fixed - -- A cell error read from another cell no longer reports the reading cell as its origin. Errors that come from a cell holding a static error value — a formula with a syntax error, or an error value entered directly — now report that cell's address instead of the address of whichever cell happened to read them first. [#131](https://github.com/handsontable/hyperformula/issues/131) +- Changed the cell errors thrown outside the formula interpreter — invalidated references, references the parser cannot resolve, references exceeding the sheet size, circular references, cleaned-up array results, and error values typed directly into a formula — to also carry a message describing their cause. [#1547](https://github.com/handsontable/hyperformula/issues/1547) ### Fixed +- Fixed a cell holding a static error value — a formula the parser rejected, or an error value entered directly — being reported under the address of whichever cell read it first. Such a cell now reports its own address, and keeps reporting it as rows and columns move around it. After a very long run of structural changes the original position can no longer be reconstructed, and the address is omitted rather than guessed. [#1547](https://github.com/handsontable/hyperformula/issues/1547) - Fixed the `AVERAGEIF` function returning a division-by-zero error when the calculated average was `0`. [#1733](https://github.com/handsontable/hyperformula/pull/1733) - Fixed the localized names of `VSTACK` and `HSTACK` in 14 language packs to match Microsoft Excel. [#1748](https://github.com/handsontable/hyperformula/pull/1748) - Fixed the MAXPOOL and MEDIANPOOL functions throwing an uncaught `TypeError` instead of returning the `#VALUE!` error when the range dimensions are not a whole multiple of the window size and the stride. [#1718](https://github.com/handsontable/hyperformula/pull/1718) diff --git a/src/Cell.ts b/src/Cell.ts index 1bcd808ab1..2dbb2b9d71 100644 --- a/src/Cell.ts +++ b/src/Cell.ts @@ -155,6 +155,7 @@ export class CellError { public readonly argumentIndex?: number, public readonly propagated: boolean = false, public readonly originAddress?: SimpleCellAddress, + public readonly originAddressVersion?: number, ) { } @@ -171,7 +172,7 @@ export class CellError { if (this.propagated || this.originFunction !== undefined) { return this } - return new CellError(this.type, this.message, this.root, functionName, this.argumentIndex, this.propagated, this.originAddress) + return new CellError(this.type, this.message, this.root, functionName, this.argumentIndex, this.propagated, this.originAddress, this.originAddressVersion) } /** @@ -185,7 +186,7 @@ export class CellError { if (this.propagated || this.argumentIndex !== undefined) { return this } - return new CellError(this.type, this.message, this.root, this.originFunction, index, this.propagated, this.originAddress) + return new CellError(this.type, this.message, this.root, this.originFunction, index, this.propagated, this.originAddress, this.originAddressVersion) } /** @@ -198,25 +199,29 @@ export class CellError { if (this.propagated) { return this } - return new CellError(this.type, this.message, this.root, this.originFunction, this.argumentIndex, true, this.originAddress) + return new CellError(this.type, this.message, this.root, this.originFunction, this.argumentIndex, true, this.originAddress, this.originAddressVersion) } /** - * Records the address an otherwise rootless error appeared at. + * Records the address an otherwise rootless error appeared at, and the moment it was recorded. * * For vertices that hold a static error rather than computing one (a parse error, or an error - * value the user typed), there is no `FormulaVertex` to serve as a lazily-resolved `root`. This - * cannot go stale: `CellError` is immutable, so the stored value is never stamped — every read - * stamps a fresh copy with the address current at that moment, which is what keeps it correct - * across row and column changes. + * value the user typed), there is no `FormulaVertex` to serve as a lazily-resolved `root`, so + * the address has to be recorded on the value itself. A recorded address is a snapshot and does + * go stale: the cell that read it caches this instance, and a later row or column change that + * merely shifts that cell does not recompute it. The `version` is what makes the snapshot + * repairable — it names the point in `LazilyTransformingAstService`'s history the address was + * true at, so `Exporter` can replay the intervening transformations over it and report where the + * origin sits now. See {@link LazilyTransformingAstService.applyTransformationsToAddress}. * * @param {SimpleCellAddress} address - the address the error was read from + * @param {number} version - the transformation version that address was current at */ - public withOriginAddress(address: SimpleCellAddress): CellError { + public withOriginAddress(address: SimpleCellAddress, version: number): CellError { if (this.originAddress !== undefined) { return this } - return new CellError(this.type, this.message, this.root, this.originFunction, this.argumentIndex, this.propagated, address) + return new CellError(this.type, this.message, this.root, this.originFunction, this.argumentIndex, this.propagated, address, version) } /** @@ -229,7 +234,7 @@ export class CellError { public attachRootVertex(vertex: FormulaVertex): CellError { if (this.root === undefined) { - return new CellError(this.type, this.message, vertex, this.originFunction, this.argumentIndex, this.propagated, this.originAddress) + return new CellError(this.type, this.message, vertex, this.originFunction, this.argumentIndex, this.propagated, this.originAddress, this.originAddressVersion) } else { return this } diff --git a/src/DependencyGraph/DependencyGraph.ts b/src/DependencyGraph/DependencyGraph.ts index 68537a306d..81270bccc5 100644 --- a/src/DependencyGraph/DependencyGraph.ts +++ b/src/DependencyGraph/DependencyGraph.ts @@ -623,12 +623,14 @@ export class DependencyGraph { // being reported as the origin. // // A vertex holding a STATIC error (a parse error, or an error value the user typed) has no - // FormulaVertex to act as a root, so the address is stamped here instead. The stamp cannot - // go stale: CellError is immutable, so this copies rather than mutating what is stored. + // FormulaVertex to act as a root, so the address is stamped here instead, together with the + // transformation version it is true at. The reading cell caches the stamped copy and is not + // recomputed by a row or column change that merely shifts it, so the address alone would go + // stale; the version lets Exporter replay the intervening transformations over it. const vertex = this.addressMapping.getCell(address) const holdsAStaticError = vertex instanceof ParsingErrorVertex || vertex instanceof ValueCellVertex return holdsAStaticError - ? value.withOriginAddress(address).asPropagated() + ? value.withOriginAddress(address, this.lazilyTransformingAstService.version()).asPropagated() : value.asPropagated() } diff --git a/src/Exporter.ts b/src/Exporter.ts index 1efadc1bd8..db23718ebc 100644 --- a/src/Exporter.ts +++ b/src/Exporter.ts @@ -11,6 +11,7 @@ import {ErrorMessage} from './error-message' import {EmptyValue, getRawValue, InterpreterValue, isExtendedNumber} from './interpreter/InterpreterValue' import {SimpleRangeValue} from './SimpleRangeValue' import {LazilyTransformingAstService} from './LazilyTransformingAstService' +import {Maybe} from './Maybe' import {NamedExpressions} from './NamedExpressions' import {simpleCellAddressToString} from './parser/addressRepresentationConverters' import { SheetMapping } from './DependencyGraph/SheetMapping' @@ -115,7 +116,7 @@ export class Exporter implements ChangeExporter { private detailedError(error: CellError): DetailedCellError { let address = undefined - const originAddress = error.root?.getAddress(this.lazilyTransformingService) ?? error.originAddress + const originAddress = error.root?.getAddress(this.lazilyTransformingService) ?? this.currentOriginAddress(error) if (originAddress !== undefined) { if (originAddress.sheet === NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS) { address = this.namedExpressions.namedExpressionInAddress(originAddress.row)?.displayName @@ -126,6 +127,28 @@ export class Exporter implements ChangeExporter { return new DetailedCellError(error, this.config.translationPackage.getErrorTranslation(error.type), address) } + /** + * Where a rootless error's origin sits now. + * + * An error produced by a formula resolves its address through its root vertex, which keeps + * itself current. An error read out of a cell that merely holds it — a parse error, or a value + * the user typed — has no such vertex, so it carries a snapshot address plus the transformation + * version that snapshot was true at, and the intervening transformations are replayed here. + * A named expression is exempt: its rows are handed out monotonically and never shifted. + * + * @param {CellError} error - the error being exported + */ + private currentOriginAddress(error: CellError): Maybe { + const {originAddress, originAddressVersion} = error + if (originAddress === undefined || originAddressVersion === undefined) { + return originAddress + } + if (originAddress.sheet === NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS) { + return originAddress + } + return this.lazilyTransformingService.applyTransformationsToAddress(originAddress, originAddressVersion) + } + private cellValueRounding(value: number): number { if (value === 0) { return value diff --git a/src/LazilyTransformingAstService.ts b/src/LazilyTransformingAstService.ts index 204905d5c6..b6b21acbc7 100644 --- a/src/LazilyTransformingAstService.ts +++ b/src/LazilyTransformingAstService.ts @@ -6,11 +6,18 @@ import {SimpleCellAddress} from './Cell' import {CombinedTransformer} from './dependencyTransformers/CombinedTransformer' import {FormulaTransformer} from './dependencyTransformers/Transformer' -import {Ast, ParserWithCaching} from './parser' +import {Maybe} from './Maybe' +import {Ast, AstNodeType, ParserWithCaching} from './parser' import {StatType} from './statistics' import {Statistics} from './statistics/Statistics' import {UndoRedo} from './UndoRedo' +/** + * Stand-in node for an address-only transformation. Every transformer leaves an EMPTY node + * untouched, so only the address it returns alongside it carries information. + */ +const ADDRESS_ONLY_NODE: Ast = {type: AstNodeType.EMPTY} + /** * Manages lazy application of formula AST transformations. * @@ -106,6 +113,39 @@ export class LazilyTransformingAstService { return [cachedAst, address, currentVersion] } + /** + * Replays stored transformations over a bare address that carries no formula. + * + * `applyTransformations` above does the same walk for a formula vertex, which owns an AST and so + * also records undo data as it goes. A value has no AST to record, and the vertex that owns the + * formula still writes its own undo entry when it catches up, so this walk deliberately records + * nothing. It reuses each transformation's own `transformSingleAst` with an empty node rather + * than reaching for `fixNodeAddress`, because two transformers do more than shift coordinates: + * `MoveCellsTransformer` only moves an address that lies inside the range being moved, and that + * decision lives in `transformSingleAst`, not in `fixNodeAddress`. + * + * Returns `undefined` when the stamp predates the last compaction. Compaction brings every + * vertex up to date before it discards transformations, but a `CellError` cached inside a + * vertex's value is not a vertex and is never brought up to date, so its version can fall behind + * `versionOffset`. The transformations needed to move the address are gone by then, and naming + * some other cell is worse than naming none. + * + * @param {SimpleCellAddress} address - the address as it was at `version` + * @param {number} version - the version the address was current at + */ + public applyTransformationsToAddress(address: SimpleCellAddress, version: number): Maybe { + if (version < this.versionOffset) { + return undefined + } + + let transformed = address + const currentVersion = this.version() + for (let v = version; v < currentVersion; v++) { + transformed = this.transformations[v - this.versionOffset].transformSingleAst(ADDRESS_ONLY_NODE, transformed)[1] + } + return transformed + } + public* getTransformationsFrom(version: number, filter?: (transformation: FormulaTransformer) => boolean): IterableIterator { const currentVersion = this.version() for (let v = Math.max(version, this.versionOffset); v < currentVersion; v++) { From d8cd6cff59c5aeb8b3ce214703743c7f5162fff3 Mon Sep 17 00:00:00 2001 From: marcin-kordas-hoc Date: Mon, 14 Sep 2026 10:32:49 +0000 Subject: [PATCH 6/7] HF-131: complete the matcher's ignore list, as the file asks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list's own note says to add every new CellError field here once. This commit adds originAddressVersion, which the address fix introduced, and the two fields that were already missing: propagated and originAddress. All three are recorded by the engine as it reads a value, so an expectation built by hand cannot know them — the same reason root and address are already ignored. Co-Authored-By: Claude Opus 5 (1M context) --- test/_setupFiles/matchers/cellErrorComparison.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/_setupFiles/matchers/cellErrorComparison.ts b/test/_setupFiles/matchers/cellErrorComparison.ts index 8d391e833d..2a9dfa15cf 100644 --- a/test/_setupFiles/matchers/cellErrorComparison.ts +++ b/test/_setupFiles/matchers/cellErrorComparison.ts @@ -23,8 +23,9 @@ export type EqualsFn = (actual: unknown, expected: unknown) => boolean * * These are asserted explicitly by the specs that care about them, rather than implicitly by * every error assertion in the suite. `message` is excluded because it is matched by substring - * above; `root` and `address` because an expectation built by the `detailedError` test helper - * cannot know the vertex or address the engine resolved. + * above; `root`, `address`, `propagated`, `originAddress` and `originAddressVersion` because an + * expectation built by the `detailedError` test helper cannot know the vertex, the address, or how + * far the engine had travelled when it recorded them. * * ADDING A FIELD TO `CellError` OR `DetailedCellError`? Add it here, once. Do not add it to a * runner wrapper. @@ -35,6 +36,9 @@ const IGNORED_IN_STRUCTURAL_COMPARE = { address: undefined, originFunction: undefined, argumentIndex: undefined, + propagated: undefined, + originAddress: undefined, + originAddressVersion: undefined, } /** From d53bb739f333c395352e0813cb5794de83a9dbb3 Mon Sep 17 00:00:00 2001 From: marcin-kordas-hoc Date: Thu, 22 Oct 2026 17:07:03 +0000 Subject: [PATCH 7/7] docs: shorten the changelog entries, describe the CellContent.Error comment without a ticket reference Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++-- src/CellContentParser.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94f75d7ec9..3c13d6dd6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - Changed the cell errors thrown by the formula engine to always carry a message describing their cause. [#1547](https://github.com/handsontable/hyperformula/issues/1547) -- Changed the cell errors thrown outside the formula interpreter — invalidated references, references the parser cannot resolve, references exceeding the sheet size, circular references, cleaned-up array results, and error values typed directly into a formula — to also carry a message describing their cause. [#1547](https://github.com/handsontable/hyperformula/issues/1547) +- Changed the cell errors produced outside the formula interpreter, such as invalidated references, circular references and error values typed into a formula, to also carry a message describing their cause. [#1547](https://github.com/handsontable/hyperformula/issues/1547) ### Fixed -- Fixed a cell holding a static error value — a formula the parser rejected, or an error value entered directly — being reported under the address of whichever cell read it first. Such a cell now reports its own address, and keeps reporting it as rows and columns move around it. After a very long run of structural changes the original position can no longer be reconstructed, and the address is omitted rather than guessed. [#1547](https://github.com/handsontable/hyperformula/issues/1547) +- 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 the `AVERAGEIF` function returning a division-by-zero error when the calculated average was `0`. [#1733](https://github.com/handsontable/hyperformula/pull/1733) - Fixed the localized names of `VSTACK` and `HSTACK` in 14 language packs to match Microsoft Excel. [#1748](https://github.com/handsontable/hyperformula/pull/1748) - Fixed the MAXPOOL and MEDIANPOOL functions throwing an uncaught `TypeError` instead of returning the `#VALUE!` error when the range dimensions are not a whole multiple of the window size and the stride. [#1718](https://github.com/handsontable/hyperformula/pull/1718) diff --git a/src/CellContentParser.ts b/src/CellContentParser.ts index 4a14b443d2..03f4612ae7 100644 --- a/src/CellContentParser.ts +++ b/src/CellContentParser.ts @@ -61,11 +61,11 @@ export namespace CellContent { export class Error { public readonly value: CellError - // HF-131: `message` stays optional here on purpose. This constructor's only + // `message` stays optional here on purpose. This constructor's only // no-message caller is the `errorMapping` lookup below (a user typing e.g. // `#REF!` literally into a cell) — the engine has no cause to state beyond // "the user wrote this", so leaving it unset is honest, not an oversight. - // This is also the concrete case the HF-131 ESLint rule (.eslintrc.js) cannot + // This is also the concrete case the cell-error ESLint rule (.eslintrc.js) cannot // catch: the call `new CellError(errorType, message)` is syntactically a // two-argument construction and passes the rule cleanly even when the // caller passes no message, because the rule only inspects this call site,