diff --git a/CHANGELOG.md b/CHANGELOG.md index af942bd4b..e895b633f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Added a new function: `XIRR`. [#1701](https://github.com/handsontable/hyperformula/pull/1701) - Added the UNIQUE function. [#1708](https://github.com/handsontable/hyperformula/pull/1708) - Added the SORT function. [#1707](https://github.com/handsontable/hyperformula/pull/1707) +- Added the `arrayFunctionResultOverwritesData` configuration option (default `false`). When enabled, an array function whose result spills onto occupied cells overwrites them instead of returning a `#SPILL!` error. This is an opt-in, destructive behavior; a collision with another array still yields `#SPILL!`. - Added an Indonesian (Bahasa Indonesia) language pack. [#1674](https://github.com/handsontable/hyperformula/pull/1674) - Added a `stringifyCurrency` config option that lets you plug in a custom currency formatter for the `TEXT` function. [#1145](https://github.com/handsontable/hyperformula/issues/1145) diff --git a/src/Config.ts b/src/Config.ts index 8e8a924ee..0f11c80e9 100644 --- a/src/Config.ts +++ b/src/Config.ts @@ -69,11 +69,14 @@ export class Config implements ConfigParams, ParserConfig { useColumnIndex: false, useStats: false, useArrayArithmetic: false, + arrayFunctionResultOverwritesData: false, } /** @inheritDoc */ public readonly useArrayArithmetic: boolean /** @inheritDoc */ + public readonly arrayFunctionResultOverwritesData: boolean + /** @inheritDoc */ public readonly caseSensitive: boolean /** @inheritDoc */ public readonly chooseAddressMappingPolicy: ChooseAddressMapping @@ -203,6 +206,7 @@ export class Config implements ConfigParams, ParserConfig { timeFormats, thousandSeparator, useArrayArithmetic, + arrayFunctionResultOverwritesData, useStats, undoLimit, maxPendingLazyTransformations, @@ -216,6 +220,7 @@ export class Config implements ConfigParams, ParserConfig { } this.useArrayArithmetic = configValueFromParam(useArrayArithmetic, 'boolean', 'useArrayArithmetic') + this.arrayFunctionResultOverwritesData = configValueFromParam(arrayFunctionResultOverwritesData, 'boolean', 'arrayFunctionResultOverwritesData') this.accentSensitive = configValueFromParam(accentSensitive, 'boolean', 'accentSensitive') this.caseSensitive = configValueFromParam(caseSensitive, 'boolean', 'caseSensitive') this.caseFirst = configValueFromParam(caseFirst, ['upper', 'lower', 'false'], 'caseFirst') diff --git a/src/ConfigParams.ts b/src/ConfigParams.ts index 71aeb0bb7..2cd2dbcac 100644 --- a/src/ConfigParams.ts +++ b/src/ConfigParams.ts @@ -390,6 +390,25 @@ export interface ConfigParams { * @category Engine */ useArrayArithmetic: boolean, + /** + * When set to `true`, an array function whose result spills onto already-occupied cells + * overwrites those cells (clearing their previous content, spilling the array, and rerouting + * any dependents to the spilled values) instead of returning a `#SPILL!` error. + * + * **Warning:** this is a destructive, opt-in behavior. Enabling it clears whatever data + * happens to sit in the spill range on the live sheet, so use it only when overwriting is the + * intended outcome. The cleared cells are restored by `undo()`. + * + * The overwrite is applied when the array formula is evaluated (e.g. via `setCellContents`). + * When set to `false`, an array spill onto an occupied cell yields `#SPILL!` and leaves the + * occupant intact (the default, Excel-compatible behavior). + * + * Even when set to `true`, a spill that would collide with *another array* still yields + * `#SPILL!` and leaves that array intact — overwrite mode never clobbers another array. + * @default false + * @category Engine + */ + arrayFunctionResultOverwritesData: boolean, /** * When set to `true`, switches column search strategy from binary search to column index. * diff --git a/src/CrudOperations.ts b/src/CrudOperations.ts index bf0238dbc..b6fd98ac8 100644 --- a/src/CrudOperations.ts +++ b/src/CrudOperations.ts @@ -262,6 +262,7 @@ export class CrudOperations { this.undoRedo.clearRedoStack() const oldContents: { address: SimpleCellAddress, newContent: RawCellContent, oldContent: [SimpleCellAddress, ClipboardCell] }[] = [] + const overwrittenCells: [SimpleCellAddress, ClipboardCell][] = [] for (let i = 0; i < cellContents.length; i++) { for (let j = 0; j < cellContents[i].length; j++) { @@ -272,12 +273,13 @@ export class CrudOperations { } const newContent = cellContents[i][j] this.clipboardOperations.abortCut() - const oldContent = this.operations.setCellContent(address, newContent) + const {oldContent, overwrittenCells: overwritten} = this.operations.setCellContent(address, newContent) oldContents.push({address, newContent, oldContent}) + overwrittenCells.push(...overwritten) } } - this.undoRedo.saveOperation(new SetCellContentsUndoEntry(oldContents)) + this.undoRedo.saveOperation(new SetCellContentsUndoEntry(oldContents, overwrittenCells)) } public setSheetContent(sheetId: number, values: RawCellContent[][]): void { diff --git a/src/DependencyGraph/DependencyGraph.ts b/src/DependencyGraph/DependencyGraph.ts index 962dedff1..535a1407d 100644 --- a/src/DependencyGraph/DependencyGraph.ts +++ b/src/DependencyGraph/DependencyGraph.ts @@ -63,6 +63,7 @@ export class DependencyGraph { public readonly lazilyTransformingAstService: LazilyTransformingAstService, public readonly functionRegistry: FunctionRegistry, public readonly namedExpressions: NamedExpressions, + public readonly config: Config, ) { this.graph = new Graph(this.dependencyQueryVertices) this.sheetReferenceRegistrar = new SheetReferenceRegistrar(sheetMapping, addressMapping) @@ -82,7 +83,8 @@ export class DependencyGraph { stats, lazilyTransformingAstService, functionRegistry, - namedExpressions + namedExpressions, + config ) } @@ -480,6 +482,30 @@ export class DependencyGraph { return true } + /** + * True when an array spill collision may be resolved by overwriting the occupants + * (i.e. `arrayFunctionResultOverwritesData` is on) AND doing so would not clobber + * another array. Array-vs-array collisions always keep `#SPILL!` (matches Excel and + * avoids corrupting the pre-existing array), even in overwrite mode. + */ + public canOverwriteArrayResult(arrayVertex: ArrayFormulaVertex): boolean { + return this.config.arrayFunctionResultOverwritesData && !this.overwriteWouldHitArray(arrayVertex) + } + + private overwriteWouldHitArray(arrayVertex: ArrayFormulaVertex): boolean { + const range = arrayVertex.getRangeOrUndef() + if (range === undefined) { + return false + } + for (const address of range.addresses(this)) { + const vertexUnderAddress = this.addressMapping.getCell(address) + if (vertexUnderAddress instanceof ArrayFormulaVertex && vertexUnderAddress !== arrayVertex) { + return true + } + } + return false + } + public moveCells(sourceRange: AbsoluteCellRange, toRight: number, toBottom: number, toSheet: number) { for (const sourceAddress of sourceRange.addressesWithDirection(toRight, toBottom, this)) { const targetAddress = simpleCellAddress(toSheet, sourceAddress.col + toRight, sourceAddress.row + toBottom) @@ -1119,14 +1145,35 @@ export class DependencyGraph { this.addressMapping.setCell(address, vertex) if (vertex instanceof ArrayFormulaVertex) { - if (!this.isThereSpaceForArray(vertex)) { + const spaceForArray = this.isThereSpaceForArray(vertex) + if (!spaceForArray && !this.canOverwriteArrayResult(vertex)) { return } + // We reach the loop either because the array spills into free space, or because there is no + // free space but `arrayFunctionResultOverwritesData` lets it overwrite the occupants. Only + // the latter actually claims occupied cells, so only then do we record the overwrite. + const isOverwritingOccupants = !spaceForArray for (const cellAddress of range.addresses(this)) { if (vertex.isLeftCorner(cellAddress)) { continue } const old = this.getCell(cellAddress) + // Record each overwritten occupant's previous value as a content change (new value + // `EmptyValue`, carrying the old value) BEFORE dropping the vertex, so every caller — + // `setCellContents`, array replace/expand, restore-from-cache — uniformly drops the stale + // value from the column index via `ColumnSearch.applyChanges`. Gated on overwrite mode so + // the free-spill path (empty occupants, and array re-placement during row/column ops) is + // untouched. + if (isOverwritingOccupants && old !== undefined && !(old instanceof EmptyCellVertex)) { + // Read the occupant's previous value WITHOUT going through `getCellValue`/`addressMapping`: + // a `ScalarFormulaVertex` that hasn't been computed yet (e.g. a sibling cell set earlier in + // the same `batch()`/`suspendEvaluation()` block) throws from `getCellValue()`. `valueOrUndef` + // is the non-throwing accessor every `FormulaVertex` exposes for exactly this case; an + // uncomputed occupant is treated as `EmptyValue`, matching what it would evaluate to before + // its formula runs. + const previousValue = old instanceof FormulaVertex ? (old.valueOrUndef() ?? EmptyValue) : old.getCellValue() + this.changes.addChange(EmptyValue, cellAddress, previousValue) + } this.exchangeOrAddGraphNode(old, vertex) } } @@ -1149,7 +1196,7 @@ export class DependencyGraph { } this.setArray(range, vertex) - if (!this.isThereSpaceForArray(vertex)) { + if (!this.isThereSpaceForArray(vertex) && !this.canOverwriteArrayResult(vertex)) { return } diff --git a/src/Evaluator.ts b/src/Evaluator.ts index f810bee36..2a132cb90 100644 --- a/src/Evaluator.ts +++ b/src/Evaluator.ts @@ -133,7 +133,7 @@ export class Evaluator { private recomputeFormulaVertexValue(vertex: FormulaVertex): InterpreterValue { const address = vertex.getAddress(this.lazilyTransformingAstService) - if (vertex instanceof ArrayFormulaVertex && (vertex.array.size.isRef || !this.dependencyGraph.isThereSpaceForArray(vertex))) { + if (vertex instanceof ArrayFormulaVertex && (vertex.array.size.isRef || (!this.dependencyGraph.isThereSpaceForArray(vertex) && !this.dependencyGraph.canOverwriteArrayResult(vertex)))) { return vertex.setNoSpace() } else { const formula = vertex.getFormula(this.lazilyTransformingAstService) diff --git a/src/Operations.ts b/src/Operations.ts index 6a0e5ccdd..e83162793 100644 --- a/src/Operations.ts +++ b/src/Operations.ts @@ -154,6 +154,17 @@ export interface MoveCellsResult { addedGlobalNamedExpressions: string[], } +export interface SetCellContentResult { + /** Previous content of the anchor cell (the cell the new content is written to). */ + oldContent: [SimpleCellAddress, ClipboardCell], + /** + * Content of the non-anchor cells that an array formula overwrote while spilling + * (only populated when `arrayFunctionResultOverwritesData` is on and the spill actually + * overwrites static occupants). Empty otherwise. Used to make the overwrite undoable. + */ + overwrittenCells: [SimpleCellAddress, ClipboardCell][], +} + export class Operations { private changes: ContentChanges = ContentChanges.empty() private readonly maxRows: number @@ -513,7 +524,12 @@ export class Operations { break } case ClipboardCellType.FORMULA: { - this.setFormulaToCellFromCache(clipboardCell.hash, address) + // Apply the resulting content changes to the column index so that when an array + // formula shrinks on restore (e.g. undoing an overwrite-expand), the vacated spill + // values are dropped from the index (HF-305). The removeRows / version-restore + // callers deliberately ignore the return value and keep their own index bookkeeping. + const changes = this.setFormulaToCellFromCache(clipboardCell.hash, address) + this.columnSearch.applyChanges(changes.getChanges()) break } case ClipboardCellType.EMPTY: { @@ -595,9 +611,10 @@ export class Operations { return result } - public setCellContent(address: SimpleCellAddress, newCellContent: RawCellContent): [SimpleCellAddress, ClipboardCell] { + public setCellContent(address: SimpleCellAddress, newCellContent: RawCellContent): SetCellContentResult { const parsedCellContent = this.cellContentParser.parse(newCellContent) const oldContent = this.getOldContent(address) + let overwrittenCells: [SimpleCellAddress, ClipboardCell][] = [] if (parsedCellContent instanceof CellContent.Formula) { const parserResult = this.parser.parse(parsedCellContent.formula, address) @@ -612,6 +629,7 @@ export class Operations { throw Error('Incorrect array size') } + overwrittenCells = this.snapshotOverwrittenOccupants(address, size) this.setFormulaToCell(address, size, parserResult) } catch (error) { if (!(error as Error).message) { @@ -628,7 +646,67 @@ export class Operations { this.setValueToCell({ parsedValue: parsedCellContent.value, rawValue: newCellContent }, address) } - return oldContent + return { oldContent, overwrittenCells } + } + + /** + * Snapshots the cells that an array formula is about to overwrite while spilling, so the + * overwrite can be undone. Returns an empty list unless `arrayFunctionResultOverwritesData` + * is on and the array is non-scalar. Mirrors `DependencyGraph.canOverwriteArrayResult`: + * if a DIFFERENT pre-existing array sits in the spill range, the spill will be blocked + * (`#SPILL!`) and nothing is overwritten, so nothing is captured. The array currently anchored + * at `anchorAddress` (the one being replaced/expanded) does not block and is not snapshotted — + * its cells are re-created when the old anchor formula is restored on undo. The anchor cell + * itself is excluded because its previous content is captured separately as `oldContent`. + */ + private snapshotOverwrittenOccupants(anchorAddress: SimpleCellAddress, size: ArraySize): [SimpleCellAddress, ClipboardCell][] { + return this.overwrittenOccupantAddresses(anchorAddress, size) + .map(occupantAddress => [occupantAddress, this.getClipboardCell(occupantAddress)] as [SimpleCellAddress, ClipboardCell]) + } + + /** + * The occupied, non-array cells (excluding the anchor) that an array formula will overwrite while + * spilling. Empty unless `arrayFunctionResultOverwritesData` is on and the array is non-scalar. + * Mirrors `DependencyGraph.canOverwriteArrayResult`: if a DIFFERENT pre-existing array sits in + * the spill range, the spill is blocked (`#SPILL!`) and nothing is overwritten, so the list is + * empty. The array currently anchored at `anchorAddress` (being replaced/expanded) neither blocks + * nor is reported as an occupant. + */ + private overwrittenOccupantAddresses(anchorAddress: SimpleCellAddress, size: ArraySize): SimpleCellAddress[] { + if (!this.dependencyGraph.config.arrayFunctionResultOverwritesData || size.width * size.height <= 1) { + return [] + } + + const spillRange = AbsoluteCellRange.spanFromOrUndef(anchorAddress, size.width, size.height) + if (spillRange === undefined) { + return [] + } + + const occupants: SimpleCellAddress[] = [] + for (const occupantAddress of spillRange.addresses(this.dependencyGraph)) { + const vertex = this.dependencyGraph.getCell(occupantAddress) + + if (vertex instanceof ArrayFormulaVertex) { + // The array currently anchored at `anchorAddress` is the one being replaced/expanded. + // Its cells (corner + internal) are re-created when the old anchor formula is restored + // on undo, so they must be neither snapshotted nor treated as a blocking collision. + // A DIFFERENT pre-existing array still blocks the overwrite (the spill stays #SPILL! + // and overwrites nothing), matching DependencyGraph.canOverwriteArrayResult / + // overwriteWouldHitArray. + if (!vertex.isLeftCorner(anchorAddress)) { + return [] + } + continue + } + + if (equalSimpleCellAddress(occupantAddress, anchorAddress) || vertex === undefined || vertex instanceof EmptyCellVertex) { + continue + } + + occupants.push(occupantAddress) + } + + return occupants } public setSheetContent(sheetId: number, newSheetContent: RawCellContent[][]) { @@ -670,6 +748,11 @@ export class Operations { const arrayChanges = this.dependencyGraph.setFormulaToCell(address, ast, absolutizeDependencies(dependencies, address), size, hasVolatileFunction, hasStructuralChangeFunction) + // In overwrite mode the spill clears occupied cells; their stale values are dropped from the + // column index here uniformly, because `exchangeOrAddFormulaVertex` records a content change + // (new value EmptyValue, carrying the old value) for every occupant it claims and + // `applyChanges` removes any change whose `oldValue` is set. Applies to the free-spill and + // non-overwrite paths too, where no occupants are claimed and this is a no-op. this.columnSearch.applyChanges(arrayChanges.getChanges()) this.changes.addAll(arrayChanges) } @@ -706,7 +789,7 @@ export class Operations { this.changes.addChange(EmptyValue, address) } - public setFormulaToCellFromCache(formulaHash: string, address: SimpleCellAddress) { + public setFormulaToCellFromCache(formulaHash: string, address: SimpleCellAddress): ContentChanges { const { ast, hasVolatileFunction, @@ -718,7 +801,7 @@ export class Operations { this.parser.rememberNewAst(cleanedAst) const cleanedDependencies = filterDependenciesOutOfScope(absoluteDependencies) const size = this.arraySizePredictor.checkArraySize(ast, address) - this.dependencyGraph.setFormulaToCell(address, cleanedAst, cleanedDependencies, size, hasVolatileFunction, hasStructuralChangeFunction) + return this.dependencyGraph.setFormulaToCell(address, cleanedAst, cleanedDependencies, size, hasVolatileFunction, hasStructuralChangeFunction) } /** @@ -840,10 +923,23 @@ export class Operations { if (arrayVertex.array.size.isRef) { continue } + // Capture the array's footprint BEFORE re-placing it, to tell apart two kinds of + // ContentChanges that setFormulaToCellFromCache returns below: + // - genuinely NEW occupants a grown array just claimed (HF-305 overwrite, outside this range) + // - the routine shrink-then-recreate artifact of re-placing an array in the same spot + // (cleanAddressMappingUnderArray clears the array's OWN previous cells every time an array + // vertex is re-set, overwrite flag or not) -- these are already reconciled by the normal + // recompute cycle right after this method returns, so re-applying them to the column index + // here double-touches entries the row/column-shift machinery already moved and corrupts it + // (this is what broke the column-index removeRows canary in an earlier attempt). + const oldRange = arrayVertex.getRangeOrUndef() const ast = arrayVertex.getFormula(this.lazilyTransformingAstService) const address = arrayVertex.getAddress(this.lazilyTransformingAstService) const hash = this.parser.computeHashFromAst(ast) - this.setFormulaToCellFromCache(hash, address) + const changes = this.setFormulaToCellFromCache(hash, address) + const overwriteChanges = changes.getChanges() + .filter(change => oldRange === undefined || !oldRange.addressInRange(change.address)) + this.columnSearch.applyChanges(overwriteChanges) } } diff --git a/src/UndoRedo.ts b/src/UndoRedo.ts index 87ab06439..d2700df69 100644 --- a/src/UndoRedo.ts +++ b/src/UndoRedo.ts @@ -350,6 +350,7 @@ export class SetCellContentsUndoEntry extends BaseUndoEntry { newContent: RawCellContent, oldContent: [SimpleCellAddress, ClipboardCell], }[], + public readonly overwrittenCells: [SimpleCellAddress, ClipboardCell][] = [], ) { super() } @@ -640,6 +641,12 @@ export class UndoRedo { } this.operations.restoreCell(oldContentAddress, oldContent) } + // Restore any cells that an array formula overwrote while spilling. This must run after + // the anchor formulas above are undone, so the spill (and its array-internal cells) is + // gone and the overwritten addresses are free to restore. + for (const [address, clipboardCell] of operation.overwrittenCells) { + this.operations.restoreCell(address, clipboardCell) + } } public undoPaste(operation: PasteUndoEntry) { diff --git a/test/hf-305-overwrite.spec.ts b/test/hf-305-overwrite.spec.ts new file mode 100644 index 000000000..576e5f6fa --- /dev/null +++ b/test/hf-305-overwrite.spec.ts @@ -0,0 +1,445 @@ +import {DetailedCellError, ErrorType, HyperFormula} from '../src' +import {SimpleCellAddress, simpleCellAddress} from '../src/Cell' + +const adr = (stringAddress: string, sheet: number = 0): SimpleCellAddress => { + const result = /^(\$([A-Za-z0-9_]+)\.)?(\$?)([A-Za-z]+)(\$?)([0-9]+)$/.exec(stringAddress)! + const row = Number(result[6]) - 1 + return simpleCellAddress(sheet, colNumber(result[4]), row) +} + +const colNumber = (input: string): number => { + if (input.length === 1) { + return input.toUpperCase().charCodeAt(0) - 65 + } else { + return input.split('').reduce((currentColumn, nextLetter) => { + return currentColumn * 26 + (nextLetter.toUpperCase().charCodeAt(0) - 64) + }, 0) - 1 + } +} + +describe('HF-305 arrayFunctionResultOverwritesData', () => { + it('OFF (flag omitted, default): array spilling onto an occupied cell yields #SPILL! and leaves the occupant intact', () => { + const data = [ + ['=TRANSPOSE(C1:E1)', null, 1, 2, 3], + ['x'], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3'}) + + const a1 = hf.getCellValue(adr('A1')) + expect(a1 instanceof DetailedCellError).toBe(true) + expect((a1 as DetailedCellError).type).toBe(ErrorType.SPILL) + expect(hf.getCellValue(adr('A2'))).toBe('x') + + hf.destroy() + }) + + it('OFF (explicit false): array spilling onto an occupied cell yields #SPILL! and leaves the occupant intact', () => { + const data = [ + ['=TRANSPOSE(C1:E1)', null, 1, 2, 3], + ['x'], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: false}) + + const a1 = hf.getCellValue(adr('A1')) + expect((a1 as DetailedCellError).type).toBe(ErrorType.SPILL) + expect(hf.getCellValue(adr('A2'))).toBe('x') + + hf.destroy() + }) + + it('OFF: setting an array formula that collides with an occupied cell yields #SPILL! (recalc parity)', () => { + const data = [ + [null, null, 1, 2, 3], + ['x'], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: false}) + + hf.setCellContents(adr('A1'), [['=TRANSPOSE(C1:E1)']]) + + const a1 = hf.getCellValue(adr('A1')) + expect((a1 as DetailedCellError).type).toBe(ErrorType.SPILL) + expect(hf.getCellValue(adr('A2'))).toBe('x') + + hf.destroy() + }) + + it('ON: setting an array formula that collides with an occupied cell overwrites it and the spill lands', () => { + const data = [ + [null, null, 1, 2, 3], + ['x'], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + + hf.setCellContents(adr('A1'), [['=TRANSPOSE(C1:E1)']]) + + expect(hf.getCellValue(adr('A1'))).toBe(1) + expect(hf.getCellValue(adr('A2'))).toBe(2) + expect(hf.getCellValue(adr('A3'))).toBe(3) + + hf.destroy() + }) + + it('ON: a cell referencing the overwritten occupant reflects the new spilled value (dependent reroute)', () => { + const data = [ + [null, '=A2', 1, 2, 3], + ['x'], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + + hf.setCellContents(adr('A1'), [['=TRANSPOSE(C1:E1)']]) + + expect(hf.getCellValue(adr('A2'))).toBe(2) + expect(hf.getCellValue(adr('B1'))).toBe(2) + + hf.destroy() + }) + + it('OFF: free spill into empty cells still works (no regression)', () => { + const data = [ + ['=TRANSPOSE(C1:E1)', null, 1, 2, 3], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: false}) + + expect(hf.getCellValue(adr('A1'))).toBe(1) + expect(hf.getCellValue(adr('A2'))).toBe(2) + expect(hf.getCellValue(adr('A3'))).toBe(3) + + hf.destroy() + }) + + it('ON: free spill into empty cells still works (no regression when the flag is on but there is no collision)', () => { + const data = [ + ['=TRANSPOSE(C1:E1)', null, 1, 2, 3], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + + expect(hf.getCellValue(adr('A1'))).toBe(1) + expect(hf.getCellValue(adr('A2'))).toBe(2) + expect(hf.getCellValue(adr('A3'))).toBe(3) + + hf.destroy() + }) + + // OUT OF SCOPE (documented current behavior): the overwrite primitive lives on the + // setFormulaToCell / setCellContents path (`exchangeOrAddFormulaVertex`). When an array + // formula AND a conflicting occupant are declared *inline in the same buildFromArray call*, + // the occupant is processed after the array and GraphBuilder.shrinkArrayIfNeeded shrinks the + // array back to its corner, so the inline occupant wins at build time even with the flag ON. + // Asserting current behavior so a future change to this edge is a conscious decision. + it('ON (out of scope): an occupant declared inline in the same buildFromArray still wins at build time', () => { + const data = [ + ['=TRANSPOSE(C1:E1)', null, 1, 2, 3], + ['x'], + ] + + const hf = HyperFormula.buildFromArray(data, {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + + expect(hf.getCellValue(adr('A2'))).toBe('x') + expect(hf.getCellValue(adr('A3'))).toBe(null) + + hf.destroy() + }) + + // Array-vs-array collisions ALWAYS keep #SPILL!, even in overwrite mode: the flag clears + // static occupants but must never clobber another array (matches Excel, and avoids corrupting + // the pre-existing array). Guarded by DependencyGraph.canOverwriteArrayResult / overwriteWouldHitArray. + it('ON: array-vs-array collision stays #SPILL! and leaves the pre-existing array intact', () => { + // First array at B2 spills B2:B4 = 1,2,3 into free space. + const hf = HyperFormula.buildFromArray([ + [null, null, 1, 2, 3], + [null, '=TRANSPOSE(C1:E1)'], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + expect(hf.getCellValue(adr('B2'))).toBe(1) + + // A second array at B1 would spill B1:B2, hitting the first array's anchor at B2. + hf.setCellContents(adr('B1'), [['=TRANSPOSE(C1:D1)']]) + + const b1 = hf.getCellValue(adr('B1')) + expect(b1 instanceof DetailedCellError).toBe(true) + expect((b1 as DetailedCellError).type).toBe(ErrorType.SPILL) + // the pre-existing array is left untouched (no corruption) + expect(hf.getCellValue(adr('B2'))).toBe(1) + expect(hf.getCellValue(adr('B3'))).toBe(2) + expect(hf.getCellValue(adr('B4'))).toBe(3) + + hf.destroy() + }) + + it('ON: overwrites a formula occupant (recalc path)', () => { + const hf = HyperFormula.buildFromArray([ + [null, null, 1, 2, 3], + ['=C1+100'], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + expect(hf.getCellValue(adr('A2'))).toBe(101) + + hf.setCellContents(adr('A1'), [['=TRANSPOSE(C1:E1)']]) + + expect(hf.getCellValue(adr('A1'))).toBe(1) + expect(hf.getCellValue(adr('A2'))).toBe(2) // formula occupant overwritten + expect(hf.getCellValue(adr('A3'))).toBe(3) + + hf.destroy() + }) + + it('ON: overwrites a 2-D block of occupants (MMULT 2x2)', () => { + const hf = HyperFormula.buildFromArray([ + [null, null, null, 1, 0, null, 1, 0], + ['x', 'y', null, 0, 1, null, 0, 1], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + + hf.setCellContents(adr('A1'), [['=MMULT(D1:E2,G1:H2)']]) // 2x2 spill A1:B2 over A2='x', B2='y' + + expect(hf.getCellValue(adr('A1'))).toBe(1) + expect(hf.getCellValue(adr('B1'))).toBe(0) + expect(hf.getCellValue(adr('A2'))).toBe(0) // occupant 'x' overwritten + expect(hf.getCellValue(adr('B2'))).toBe(1) // occupant 'y' overwritten + + hf.destroy() + }) + + // The overwrite is destructive on the live sheet, but the cleared occupants are recorded on + // the undo stack, so undo restores them. + it('ON: undo of an overwrite restores the overwritten occupant', () => { + const hf = HyperFormula.buildFromArray([ + [null, null, 1, 2, 3], + ['x'], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + hf.setCellContents(adr('A1'), [['=TRANSPOSE(C1:E1)']]) // overwrites A2='x' + expect(hf.getCellValue(adr('A2'))).toBe(2) + + hf.undo() + + expect(hf.getCellValue(adr('A1'))).toBe(null) // formula removed + expect(hf.getCellValue(adr('A2'))).toBe('x') // overwritten occupant restored + + hf.destroy() + }) + + it('ON: redo after undo re-applies the overwrite', () => { + const hf = HyperFormula.buildFromArray([ + [null, null, 1, 2, 3], + ['x'], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + hf.setCellContents(adr('A1'), [['=TRANSPOSE(C1:E1)']]) // overwrites A2='x' + hf.undo() + expect(hf.getCellValue(adr('A2'))).toBe('x') // restored + + hf.redo() + + expect(hf.getCellValue(adr('A1'))).toBe(1) + expect(hf.getCellValue(adr('A2'))).toBe(2) // overwrite re-applied + + hf.destroy() + }) + + it('ON: undo of a blocked array-vs-array spill leaves the pre-existing array intact', () => { + // First array at B2 spills B2:B4 = 1,2,3 into free space. + const hf = HyperFormula.buildFromArray([ + [null, null, 1, 2, 3], + [null, '=TRANSPOSE(C1:E1)'], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + expect(hf.getCellValue(adr('B2'))).toBe(1) + + // A second array at B1 would spill B1:B2, hitting the first array's anchor at B2 -> #SPILL!. + hf.setCellContents(adr('B1'), [['=TRANSPOSE(C1:D1)']]) + expect((hf.getCellValue(adr('B1')) as DetailedCellError).type).toBe(ErrorType.SPILL) + + hf.undo() + + expect(hf.getCellValue(adr('B1'))).toBe(null) // failed spill removed + // pre-existing array is untouched by the undo (nothing was overwritten to restore) + expect(hf.getCellValue(adr('B2'))).toBe(1) + expect(hf.getCellValue(adr('B3'))).toBe(2) + expect(hf.getCellValue(adr('B4'))).toBe(3) + + hf.destroy() + }) + + // With useColumnIndex, overwriting a cell must drop its old value from the column index, + // otherwise VLOOKUP/MATCH could still match an overwritten value (stale index). + it('ON + useColumnIndex: overwrite drops the stale value from the column index', () => { + // A1:A3 = 10,20,30; G1:I1 = 100,200,300; K1 = MATCH(20, A1:A3); L1 = MATCH(200, A1:A3) + const hf = HyperFormula.buildFromArray([ + [10, null, null, null, null, null, 100, 200, 300, null, '=MATCH(20,A1:A3,0)', '=MATCH(200,A1:A3,0)'], + [20], + [30], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true, useColumnIndex: true}) + expect(hf.getCellValue(adr('K1'))).toBe(2) // 20 initially at A2 + + hf.setCellContents(adr('A1'), [['=TRANSPOSE(G1:I1)']]) // A1:A3 -> 100,200,300 + + expect(hf.getCellValue(adr('A2'))).toBe(200) + const match20 = hf.getCellValue(adr('K1')) + expect(match20 instanceof DetailedCellError).toBe(true) // 20 is gone -> #N/A (not stale) + expect((match20 as DetailedCellError).type).toBe(ErrorType.NA) + expect(hf.getCellValue(adr('L1'))).toBe(2) // 200 now at A2 + + hf.destroy() + }) + + // Finding #1 (Bugbot): replacing/expanding an EXISTING array so it overwrites a NEW static cell + // must snapshot that static cell for undo. Previously overwrittenOccupantAddresses bailed out on + // ANY array in the spill range — including the array being replaced at the anchor — so nothing + // was captured and undo lost the static occupant. The anchor's own array is now excluded from + // the block, so only a genuinely new static occupant (A3) is snapshotted. + it('ON: undo of an array EXPANDED over a new static cell restores that cell', () => { + // A1=TRANSPOSE(D1:E1) spills A1:A2 = 10,20 into free space; A3 = 99 (static). + const hf = HyperFormula.buildFromArray([ + ['=TRANSPOSE(D1:E1)', null, null, 10, 20, 30], + [null], + [99], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + expect(hf.getCellValue(adr('A2'))).toBe(20) + expect(hf.getCellValue(adr('A3'))).toBe(99) + + // Expand the array to A1:A3 = 10,20,30, overwriting the static A3 = 99. + hf.setCellContents(adr('A1'), [['=TRANSPOSE(D1:F1)']]) + expect(hf.getCellValue(adr('A3'))).toBe(30) // 99 overwritten by the expanded spill + + hf.undo() + + expect(hf.getCellValue(adr('A1'))).toBe(10) // original array re-spilled + expect(hf.getCellValue(adr('A2'))).toBe(20) + expect(hf.getCellValue(adr('A3'))).toBe(99) // overwritten static cell restored (finding #1) + + hf.destroy() + }) + + // Finding #3 on the array-replace path: expanding an array over a static cell with useColumnIndex + // must drop the overwritten value from the column index (not just the plain overwrite path). + it('ON + useColumnIndex: expanding an array over a static cell drops the stale value from the column index', () => { + // A1=TRANSPOSE(D1:E1) spills A1:A2 = 10,20; A3 = 99; H1 = MATCH(99, A1:A3). + const hf = HyperFormula.buildFromArray([ + ['=TRANSPOSE(D1:E1)', null, null, 10, 20, 30, null, '=MATCH(99,A1:A3,0)'], + [null], + [99], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true, useColumnIndex: true}) + expect(hf.getCellValue(adr('H1'))).toBe(3) // 99 at A3 + + hf.setCellContents(adr('A1'), [['=TRANSPOSE(D1:F1)']]) // expand to A1:A3 = 10,20,30 + + expect(hf.getCellValue(adr('A3'))).toBe(30) + const match99 = hf.getCellValue(adr('H1')) + expect(match99 instanceof DetailedCellError).toBe(true) // 99 gone -> #N/A (not stale) + expect((match99 as DetailedCellError).type).toBe(ErrorType.NA) + + hf.destroy() + }) + + // Undoing an expand-overwrite must also drop the vacated spill values from the column index — + // restore goes through restoreCell -> setFormulaToCellFromCache, which now applies the shrink's + // content changes to the column search (otherwise a lookup could still match a value undo removed). + it('ON + useColumnIndex: undo of an expand-overwrite drops the vacated value from the column index', () => { + // A1=TRANSPOSE(D1:E1) spills A1:A2 = 10,20; A3 = 99; K1 = MATCH(30, A1:A3). + const hf = HyperFormula.buildFromArray([ + ['=TRANSPOSE(D1:E1)', null, null, 10, 20, 30, null, null, null, null, '=MATCH(30,A1:A3,0)'], + [null], + [99], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true, useColumnIndex: true}) + + hf.setCellContents(adr('A1'), [['=TRANSPOSE(D1:F1)']]) // expand to A1:A3 = 10,20,30 (overwrites 99) + expect(hf.getCellValue(adr('K1'))).toBe(3) // 30 now at A3 + + hf.undo() // back to A1:A2 = 10,20, A3 = 99 + + expect(hf.getCellValue(adr('A3'))).toBe(99) // value restored + const match30 = hf.getCellValue(adr('K1')) + expect(match30 instanceof DetailedCellError).toBe(true) // 30 vacated -> #N/A (not stale) + expect((match30 as DetailedCellError).type).toBe(ErrorType.NA) + + hf.destroy() + }) + + // Documents the copy/paste boundary relevant to Bugbot finding #2. Copying an array cell captures + // its VALUE (getClipboardCell materializes ArrayFormulaVertex cells), NOT the array formula, so a + // paste writes a plain value that never spills. There is therefore no public copy/paste path that + // spills an array over occupied cells, and the flag has no effect on paste. Pinned so a future + // change to copy semantics (preserving array formulas) is a conscious decision that would also + // need paste-undo of the overwritten occupants. + it('ON: pasting a copied array anchor writes its value and does NOT overwrite occupants (copy materializes arrays)', () => { + const hf = HyperFormula.buildFromArray([ + ['=TRANSPOSE(G1:I1)', null, null, 'x', null, null, 100, 200, 300], + [null, null, null, 'y'], + [null, null, null, 'z'], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + + hf.copy({start: adr('A1'), end: adr('A1')}) // captures the VALUE 100, not =TRANSPOSE(...) + hf.paste(adr('D1')) + + expect(hf.getCellValue(adr('D1'))).toBe(100) // pasted value, not a spill + expect(hf.getCellValue(adr('D2'))).toBe('y') // occupant intact (no spill, no overwrite) + expect(hf.getCellValue(adr('D3'))).toBe('z') + + hf.destroy() + }) + + // Bugbot (PR #1714, round 1): overwriting an occupant recorded its previous value by calling + // `getCellValue`, which throws for a `ScalarFormulaVertex` that hasn't been computed yet -- the + // normal state of a sibling cell set earlier in the same batch()/suspendEvaluation() block, since + // evaluation only runs once the batch is committed. That throw aborted the whole operation before + // the spill landed (and could even corrupt graph state, since `batch()`'s catch handler calls + // `resumeEvaluation()` before rethrowing, and that call could itself throw against the half-applied + // exchange, masking the original error). Fixed by reading the occupant via the non-throwing + // `valueOrUndef()` accessor every `FormulaVertex` exposes, treating an uncomputed occupant as + // `EmptyValue`. + it('ON: array spilling over an occupant formula that is not yet computed (inside batch) does not throw', () => { + const hf = HyperFormula.buildFromArray([ + [0, 1, 10], + [null, null, 20], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true}) + + expect(() => { + hf.batch(() => { + // B1 becomes a formula whose value is not computed yet (batch suspends evaluation). + hf.setCellContents(adr('B1'), '=A1+1') + // A1's spill claims B1 while it is still uncomputed. + hf.setCellContents(adr('A1'), [['=TRANSPOSE(C1:C2)']]) + }) + }).not.toThrow() + + expect(hf.getCellValue(adr('A1'))).toBe(10) + expect(hf.getCellValue(adr('B1'))).toBe(20) + + hf.destroy() + }) + + // Bugbot (PR #1714, round 1): `setFormulaToCellFromCache` returns overwrite `ContentChanges`, but + // `rewriteAffectedArrays` (the structural caller used by doAddRows/doRemoveRows/doAddColumns/ + // doRemoveColumns to re-place arrays whose dependencies shifted) ignored that return value. When a + // structural insert/delete grows an array so it overwrites a previously-static cell under the + // overwrite flag, the stale value was never dropped from the column index, so MATCH/VLOOKUP could + // still match it. Fixed by applying the returned changes to the column index at this call site only + // (mirroring the existing `restoreCell` pattern), without threading extra undo entries through + // insert/delete -- that half is a documented, deliberately out-of-scope limitation. + it('ON + useColumnIndex: addColumns growing an array over a static cell drops the stale value from the column index', () => { + // A1 = TRANSPOSE(D1:E1) spills A1:A2 = 10,20. A3 = 99 (static, not yet overlapped). + // C1 = MATCH(99, A1:A3, 0) sits left of D1:E1 so the column insertion never shifts it. + const hf = HyperFormula.buildFromArray([ + ['=TRANSPOSE(D1:E1)', null, '=MATCH(99,A1:A3,0)', 10, 20], + [null], + [99], + ], {licenseKey: 'gpl-v3', arrayFunctionResultOverwritesData: true, useColumnIndex: true}) + + expect(hf.getCellValue(adr('A3'))).toBe(99) + expect(hf.getCellValue(adr('C1'))).toBe(3) // 99 found at row 3 + + // Insert a column inside D1:E1 (at E's position), extending the reference to D1:F1 and growing + // the array from 1x2 to 1x3 -- it now spills over A3, overwriting the static 99 there. + hf.addColumns(0, [4, 1]) + + expect(hf.getCellValue(adr('A3'))).toBe(20) // spilled value, 99 is gone + + const match99 = hf.getCellValue(adr('C1')) + expect(match99 instanceof DetailedCellError).toBe(true) // must be #N/A, not a stale match + expect((match99 as DetailedCellError).type).toBe(ErrorType.NA) + + hf.destroy() + }) +})