diff --git a/CHANGELOG.md b/CHANGELOG.md index 376ecfa7bb..b4081da862 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,11 +13,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed -- Changed the cell errors produced while evaluating a formula to carry a message describing their cause. [#1547](https://github.com/handsontable/hyperformula/issues/1547) +- Changed the cell errors the engine produces, including invalidated references, circular references and error values typed into a formula, to carry a message describing their cause. [#1547](https://github.com/handsontable/hyperformula/issues/1547) ### Fixed - 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) ## [3.5.0] - 2026-10-09 diff --git a/hyperformula/.eslintrc.js b/hyperformula/.eslintrc.js index cd8f06cd2d..b0929ffdad 100644 --- a/hyperformula/.eslintrc.js +++ b/hyperformula/.eslintrc.js @@ -146,12 +146,14 @@ module.exports = { } }, { - files: ['**/src/interpreter/**/*.ts'], + files: ['**/src/**/*.ts'], rules: { // 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/hyperformula/src/Cell.ts b/hyperformula/src/Cell.ts index faab24babe..2dbb2b9d71 100644 --- a/hyperformula/src/Cell.ts +++ b/hyperformula/src/Cell.ts @@ -150,10 +150,80 @@ 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, + public readonly originAddressVersion?: number, ) { } + /** + * 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, this.originAddressVersion) + } + + /** + * 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, this.originAddressVersion) + } + + /** + * 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, this.originAddressVersion) + } + + /** + * 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`, 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, 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, version) + } + /** * Returns a CellError with a given message. * @param {string} detailedMessage - message to be displayed @@ -164,7 +234,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, this.originAddressVersion) } else { return this } diff --git a/hyperformula/src/CellContentParser.ts b/hyperformula/src/CellContentParser.ts index 916068ae00..03f4612ae7 100644 --- a/hyperformula/src/CellContentParser.ts +++ b/hyperformula/src/CellContentParser.ts @@ -61,6 +61,15 @@ export namespace CellContent { export class Error { public readonly value: CellError + // `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 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, + // not who calls it. constructor(errorType: ErrorType, message?: string) { this.value = new CellError(errorType, message) } diff --git a/hyperformula/src/DependencyGraph/DependencyGraph.ts b/hyperformula/src/DependencyGraph/DependencyGraph.ts index 962dedff1a..81270bccc5 100644 --- a/hyperformula/src/DependencyGraph/DependencyGraph.ts +++ b/hyperformula/src/DependencyGraph/DependencyGraph.ts @@ -612,7 +612,26 @@ 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, 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, this.lazilyTransformingAstService.version()).asPropagated() + : value.asPropagated() } public getRawValue(address: SimpleCellAddress): RawCellContent { @@ -624,11 +643,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) } @@ -966,7 +983,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/hyperformula/src/DependencyGraph/FormulaVertex.ts b/hyperformula/src/DependencyGraph/FormulaVertex.ts index 5aa448ffd8..84f54499ab 100644 --- a/hyperformula/src/DependencyGraph/FormulaVertex.ts +++ b/hyperformula/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/hyperformula/src/Evaluator.ts b/hyperformula/src/Evaluator.ts index f810bee363..ecf234dfd9 100644 --- a/hyperformula/src/Evaluator.ts +++ b/hyperformula/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/hyperformula/src/Exporter.ts b/hyperformula/src/Exporter.ts index c5ff24f73b..db23718ebc 100644 --- a/hyperformula/src/Exporter.ts +++ b/hyperformula/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) + 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/hyperformula/src/LazilyTransformingAstService.ts b/hyperformula/src/LazilyTransformingAstService.ts index 204905d5c6..b6b21acbc7 100644 --- a/hyperformula/src/LazilyTransformingAstService.ts +++ b/hyperformula/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++) { diff --git a/hyperformula/src/dependencyTransformers/Transformer.ts b/hyperformula/src/dependencyTransformers/Transformer.ts index 487f8bf918..fc937d1c27 100644 --- a/hyperformula/src/dependencyTransformers/Transformer.ts +++ b/hyperformula/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/hyperformula/src/error-message.ts b/hyperformula/src/error-message.ts index fbc530ac45..c0405711d4 100644 --- a/hyperformula/src/error-message.ts +++ b/hyperformula/src/error-message.ts @@ -15,6 +15,8 @@ export class ErrorMessage { public static NoBroadcastValue = 'Operand ranges cannot be broadcast to a common size, so this position has no value.' public static PoolDimensions = 'Range dimensions are not compatible with the window size and the stride.' 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.' @@ -29,6 +31,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.' @@ -59,6 +65,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/hyperformula/src/interpreter/Interpreter.ts b/hyperformula/src/interpreter/Interpreter.ts index cd8907dc5f..a62372a103 100644 --- a/hyperformula/src/interpreter/Interpreter.ts +++ b/hyperformula/src/interpreter/Interpreter.ts @@ -69,7 +69,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))) } @@ -77,6 +79,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) } @@ -498,7 +503,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/hyperformula/src/parser/FormulaParser.ts b/hyperformula/src/parser/FormulaParser.ts index 82c70e1c1d..3bfb977a44 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)) + 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') } diff --git a/hyperformula/test/_setupFiles/jest/toEqualError.ts b/hyperformula/test/_setupFiles/jest/toEqualError.ts index 80f274cfc2..1af55cb5f2 100644 --- a/hyperformula/test/_setupFiles/jest/toEqualError.ts +++ b/hyperformula/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/hyperformula/test/_setupFiles/matchers/cellErrorComparison.ts b/hyperformula/test/_setupFiles/matchers/cellErrorComparison.ts new file mode 100644 index 0000000000..2a9dfa15cf --- /dev/null +++ b/hyperformula/test/_setupFiles/matchers/cellErrorComparison.ts @@ -0,0 +1,78 @@ +/** + * @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`, `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. + */ +const IGNORED_IN_STRUCTURAL_COMPARE = { + message: undefined, + root: undefined, + address: undefined, + originFunction: undefined, + argumentIndex: undefined, + propagated: undefined, + originAddress: undefined, + originAddressVersion: 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/hyperformula/test/_setupFiles/matchers/toEqualError.ts b/hyperformula/test/_setupFiles/matchers/toEqualError.ts index 6c0b87bb56..65e3a171db 100644 --- a/hyperformula/test/_setupFiles/matchers/toEqualError.ts +++ b/hyperformula/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) } }, }