diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e535ebc2..47a950411 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Added + +- 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, or a spill range that cannot be represented on the sheet, still yields `#SPILL!`. [#1714](https://github.com/handsontable/hyperformula/pull/1714) + ### Fixed - 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/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..f2b6e1c7d 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,37 @@ 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. + * + * An unrepresentable spill range (e.g. an infinite-height array anchored outside row 1) + * can never be overwrite-resolved: overwrite mode bypasses occupancy collisions only, + * never a range that cannot be spanned at all, so those keep `#SPILL!` too. + */ + public canOverwriteArrayResult(arrayVertex: ArrayFormulaVertex): boolean { + if (arrayVertex.getRangeOrUndef() === undefined) { + return false + } + 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 +1152,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,6 +1203,11 @@ export class DependencyGraph { } this.setArray(range, vertex) + // No `canOverwriteArrayResult` bypass here: this is the build path (`GraphBuilder` via + // `addArrayVertex`), which is deliberately outside the `arrayFunctionResultOverwritesData` + // feature — an occupant declared in the same build wins, and the array keeps `#SPILL!`. + // Claiming occupied cells here would silently overwrite them with no undo snapshot and + // no column-index change records. if (!this.isThereSpaceForArray(vertex)) { return } diff --git a/src/Evaluator.ts b/src/Evaluator.ts index f810bee36..235736e82 100644 --- a/src/Evaluator.ts +++ b/src/Evaluator.ts @@ -133,6 +133,12 @@ export class Evaluator { private recomputeFormulaVertexValue(vertex: FormulaVertex): InterpreterValue { const address = vertex.getAddress(this.lazilyTransformingAstService) + // No `canOverwriteArrayResult` bypass here: when `arrayFunctionResultOverwritesData` resolves + // a collision, the overwrite has already claimed the whole spill range in the address mapping + // (`DependencyGraph.exchangeOrAddFormulaVertex`) BEFORE evaluation, so `isThereSpaceForArray` + // is true by the time the array is recomputed. An array that still lacks space at this point + // was never overwrite-resolved (blocked at build time, an unrepresentable spill range, another + // array in the way) and must surface `#SPILL!` instead of silently truncating to its anchor. if (vertex instanceof ArrayFormulaVertex && (vertex.array.size.isRef || !this.dependencyGraph.isThereSpaceForArray(vertex))) { return vertex.setNoSpace() } else { diff --git a/src/Operations.ts b/src/Operations.ts index 6a0e5ccdd..73e6df118 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.isScalar()) { + 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) {