From 28411ce98d7e21821b10f4c2b4dcf62db8a92b3d Mon Sep 17 00:00:00 2001 From: Evan Hynes Date: Tue, 9 Sep 2025 13:51:18 +0100 Subject: [PATCH 1/2] feat: Implement new config param, allowCircularReferences --- src/Config.ts | 5 + src/ConfigParams.ts | 6 + src/Evaluator.ts | 158 ++++++++++++++- test/circular-dependencies.spec.ts | 306 +++++++++++++++++++++++++++++ 4 files changed, 465 insertions(+), 10 deletions(-) create mode 100644 test/circular-dependencies.spec.ts diff --git a/src/Config.ts b/src/Config.ts index c345c97e9..855f4d7f6 100644 --- a/src/Config.ts +++ b/src/Config.ts @@ -30,6 +30,7 @@ export class Config implements ConfigParams, ParserConfig { public static defaultConfig: ConfigParams = { accentSensitive: false, + allowCircularReferences: false, currencySymbol: ['$'], caseSensitive: false, caseFirst: 'lower', @@ -78,6 +79,8 @@ export class Config implements ConfigParams, ParserConfig { /** @inheritDoc */ public readonly accentSensitive: boolean /** @inheritDoc */ + public readonly allowCircularReferences: boolean + /** @inheritDoc */ public readonly caseFirst: 'upper' | 'lower' | 'false' /** @inheritDoc */ public readonly dateFormats: string[] @@ -164,6 +167,7 @@ export class Config implements ConfigParams, ParserConfig { constructor(options: Partial = {}, showDeprecatedWarns: boolean = true) { const { accentSensitive, + allowCircularReferences, caseSensitive, caseFirst, chooseAddressMappingPolicy, @@ -209,6 +213,7 @@ export class Config implements ConfigParams, ParserConfig { this.useArrayArithmetic = configValueFromParam(useArrayArithmetic, 'boolean', 'useArrayArithmetic') this.accentSensitive = configValueFromParam(accentSensitive, 'boolean', 'accentSensitive') + this.allowCircularReferences = configValueFromParam(options.allowCircularReferences, 'boolean', 'allowCircularReferences') this.caseSensitive = configValueFromParam(caseSensitive, 'boolean', 'caseSensitive') this.caseFirst = configValueFromParam(caseFirst, ['upper', 'lower', 'false'], 'caseFirst') this.ignorePunctuation = configValueFromParam(ignorePunctuation, 'boolean', 'ignorePunctuation') diff --git a/src/ConfigParams.ts b/src/ConfigParams.ts index 165e86189..f22b8c4a5 100644 --- a/src/ConfigParams.ts +++ b/src/ConfigParams.ts @@ -16,6 +16,12 @@ export interface ConfigParams { * @category String */ accentSensitive: boolean, + /** + * When set to `true`, allows circular references in formulas (up to a fixed iteration limit). + * @default false + * @category Engine + */ + allowCircularReferences: boolean, /** * When set to `true`, makes string comparison case-sensitive. * diff --git a/src/Evaluator.ts b/src/Evaluator.ts index f47272b9b..9f23f4629 100644 --- a/src/Evaluator.ts +++ b/src/Evaluator.ts @@ -20,6 +20,7 @@ import {Ast, RelativeDependency} from './parser' import {Statistics, StatType} from './statistics' export class Evaluator { + private readonly iterationCount = 100 constructor( private readonly config: Config, @@ -43,6 +44,7 @@ export class Evaluator { public partialRun(vertices: Vertex[]): ContentChanges { const changes = ContentChanges.empty() + const cycled: Vertex[] = [] this.stats.measure(StatType.EVALUATION, () => { this.dependencyGraph.graph.getTopSortedWithSccSubgraphFrom(vertices, @@ -68,15 +70,17 @@ export class Evaluator { if (vertex instanceof RangeVertex) { vertex.clearCache() } 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) - vertex.setCellValue(error) - changes.addChange(error, address) + const firstCycleChanges = this.iterateCircularDependencies([vertex], 1) + changes.addAll(firstCycleChanges) + cycled.push(vertex) } }, ) }) + + const cycledChanges = this.iterateCircularDependencies(cycled, this.iterationCount - 1) + changes.addAll(cycledChanges) + return changes } @@ -105,11 +109,8 @@ export class Evaluator { * Recalculates formulas in the topological sort order */ private recomputeFormulas(cycled: Vertex[], sorted: Vertex[]): void { - cycled.forEach((vertex: Vertex) => { - if (vertex instanceof FormulaVertex) { - vertex.setCellValue(new CellError(ErrorType.CYCLE, undefined, vertex)) - } - }) + this.iterateCircularDependencies(cycled) + sorted.forEach((vertex: Vertex) => { if (vertex instanceof FormulaVertex) { const newCellValue = this.recomputeFormulaVertexValue(vertex) @@ -121,6 +122,143 @@ export class Evaluator { }) } + private blockCircularDependencies(cycled: Vertex[]): ContentChanges { + const changes = ContentChanges.empty() + + cycled.forEach((vertex: Vertex) => { + if (vertex instanceof RangeVertex) { + vertex.clearCache() + } 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) + vertex.setCellValue(error) + changes.addChange(error, address) + } + }) + + return changes + } + + /** + * Iterates over all circular dependencies (cycled vertices) for 100 iterations + * Handles cascading dependencies by processing cycles in dependency order + */ + private iterateCircularDependencies(cycled: Vertex[], cycles = this.iterationCount): ContentChanges { + if (!this.config.allowCircularReferences) { + return this.blockCircularDependencies(cycled) + } + + const changes = ContentChanges.empty() + cycled.forEach((vertex: Vertex) => { + if (vertex instanceof FormulaVertex && !vertex.isComputed()) { + vertex.setCellValue(0) + } + }) + + for (let i = 0; i < cycles; i++) { + this.clearCachesForCyclicRanges(cycled) + + cycled.forEach((vertex: Vertex) => { + if (!(vertex instanceof FormulaVertex)) { + return + } + + const address = vertex.getAddress(this.lazilyTransformingAstService) + const newCellValue = this.recomputeFormulaVertexValue(vertex) + + if (i < cycles - 1) { + return + } + + this.columnSearch.add(getRawValue(newCellValue), address) + changes.addChange(newCellValue, address) + }) + } + + const dependentChanges = this.updateNonCyclicDependents(cycled) + changes.addAll(dependentChanges) + + return changes + } + + /** + * Updates all non-cyclic cells that depend on the given cycled vertices + * Uses topological sorting to ensure correct dependency order + */ + private updateNonCyclicDependents(cycled: Vertex[]): ContentChanges { + const changes = ContentChanges.empty() + const cyclicSet = new Set(cycled) + + const dependents = new Set() + cycled.forEach(vertex => { + this.dependencyGraph.graph.adjacentNodes(vertex).forEach(dependent => { + if (!cyclicSet.has(dependent) && dependent instanceof FormulaVertex) { + dependents.add(dependent) + } + }) + }) + + if (dependents.size === 0) { + return changes + } + + const {sorted} = this.dependencyGraph.topSortWithScc() + const orderedDependents = sorted.filter(vertex => dependents.has(vertex)) + + orderedDependents.forEach(vertex => { + if (vertex instanceof FormulaVertex) { + const newCellValue = this.recomputeFormulaVertexValue(vertex) + const address = vertex.getAddress(this.lazilyTransformingAstService) + this.columnSearch.add(getRawValue(newCellValue), address) + changes.addChange(newCellValue, address) + } + }) + + return changes + } + + /** + * Clears function caches for ranges that contain any of the given cyclic vertices + * This ensures fresh computation during circular dependency iteration + */ + private clearCachesForCyclicRanges(cycled: Vertex[]): void { + const cyclicAddresses = new Set() + cycled.forEach((vertex: Vertex) => { + if (vertex instanceof FormulaVertex) { + const address = vertex.getAddress(this.lazilyTransformingAstService) + cyclicAddresses.add(`${address.sheet}:${address.col}:${address.row}`) + } + }) + + const sheetsWithCycles = new Set() + cycled.forEach((vertex: Vertex) => { + if (vertex instanceof FormulaVertex) { + const address = vertex.getAddress(this.lazilyTransformingAstService) + sheetsWithCycles.add(address.sheet) + } + }) + + sheetsWithCycles.forEach(sheet => { + for (const rangeVertex of this.dependencyGraph.rangeMapping.rangesInSheet(sheet)) { + const range = rangeVertex.range + let containsCyclicCell = false + + for (const address of range.addresses(this.dependencyGraph)) { + const addressKey = `${address.sheet}:${address.col}:${address.row}` + if (cyclicAddresses.has(addressKey)) { + containsCyclicCell = true + break + } + } + + if (containsCyclicCell) { + rangeVertex.clearCache() + } + } + }) + } + private recomputeFormulaVertexValue(vertex: FormulaVertex): InterpreterValue { const address = vertex.getAddress(this.lazilyTransformingAstService) if (vertex instanceof ArrayVertex && (vertex.array.size.isRef || !this.dependencyGraph.isThereSpaceForArray(vertex))) { diff --git a/test/circular-dependencies.spec.ts b/test/circular-dependencies.spec.ts new file mode 100644 index 000000000..ef7e08a60 --- /dev/null +++ b/test/circular-dependencies.spec.ts @@ -0,0 +1,306 @@ +import {ErrorType, HyperFormula} from '../src' +import {Config} from '../src/Config' +import {adr, detailedError} from './testUtils' + +describe('Circular Dependencies', () => { + describe('with allowCircularReferences disabled (default)', () => { + it('simple cycle should return CYCLE error', () => { + const engine = HyperFormula.buildFromArray([['=B1', '=A1']]) + + expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.CYCLE)) + expect(engine.getCellValue(adr('B1'))).toEqualError(detailedError(ErrorType.CYCLE)) + }) + + it('three-cell cycle should return CYCLE error', () => { + const engine = HyperFormula.buildFromArray([['=B1', '=C1', '=A1']]) + + expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.CYCLE)) + expect(engine.getCellValue(adr('B1'))).toEqualError(detailedError(ErrorType.CYCLE)) + expect(engine.getCellValue(adr('C1'))).toEqualError(detailedError(ErrorType.CYCLE)) + }) + + it('cycle with formula should return CYCLE error', () => { + const engine = HyperFormula.buildFromArray([['5', '=A1+B1']]) + expect(engine.getCellValue(adr('B1'))).toEqualError(detailedError(ErrorType.CYCLE)) + }) + }) + + describe('with allowCircularReferences enabled', () => { + it('should handle simple two-cell cycle', () => { + const engine = HyperFormula.buildFromArray([['=B1+1', '=A1+1']], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + + expect(valueA).toBe(200) + expect(valueB).toBe(199) + }) + + it('should handle three-cell cycle', () => { + const engine = HyperFormula.buildFromArray([['=B1+1', '=C1+1', '=A1+1']], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + const valueC = engine.getCellValue(adr('C1')) + + expect(valueA).toBe(300) + expect(valueB).toBe(299) + expect(valueC).toBe(298) + }) + + it('should converge to stable values for self-referencing formula', () => { + const engine = HyperFormula.buildFromArray([['=A1*0.9 + 10']], { + allowCircularReferences: true + }) + + const value = engine.getCellValue(adr('A1')) + expect(value).toBe(99.99734386) + }) + + it('should handle cycles with non-cyclic dependencies', () => { + const engine = HyperFormula.buildFromArray([ + ['=B1+1', '=A1+1', '10'], + ['=C1*2', '', ''] + ], { + allowCircularReferences: true + }) + + const valueA1 = engine.getCellValue(adr('A1')) + const valueA2 = engine.getCellValue(adr('A2')) + const valueB1 = engine.getCellValue(adr('B1')) + const valueC1 = engine.getCellValue(adr('C1')) + + expect(valueA1).toBe(200) + expect(valueA2).toBe(20) + expect(valueB1).toBe(199) + expect(valueC1).toBe(10) + }) + + it('should handle multiple independent cycles', () => { + const engine = HyperFormula.buildFromArray([ + ['=B1+1', '=A1+1'], + ['=B2+2', '=A2+2'] + ], { + allowCircularReferences: true + }) + + const valueA1 = engine.getCellValue(adr('A1')) + const valueA2 = engine.getCellValue(adr('A2')) + const valueB1 = engine.getCellValue(adr('B1')) + const valueB2 = engine.getCellValue(adr('B2')) + + expect(valueA1).toBe(200) + expect(valueA2).toBe(400) + expect(valueB1).toBe(199) + expect(valueB2).toBe(398) + }) + + it('should propagate changes to dependent cells after cycle resolution', () => { + const engine = HyperFormula.buildFromArray([ + ['=B1+1', '=A1+1', '=A1+B1'] + ], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + const valueC = engine.getCellValue(adr('C1')) + + expect(valueA).toBe(200) + expect(valueB).toBe(199) + expect(valueC).toBe(399) + }) + + it('should handle self-cycles', () => { + const engine = HyperFormula.buildFromArray([['5']], { + allowCircularReferences: true + }) + + engine.setCellContents(adr('A1'), [['=A1*2']]) + + const value = engine.getCellValue(adr('A1')) + expect(value).toBe(0) + }) + + it('should handle complex formula cycles', () => { + const engine = HyperFormula.buildFromArray([ + ['=SUM(B1:C1)', '=A1/2', '=A1/3'] + ], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + const valueC = engine.getCellValue(adr('C1')) + + expect(valueA).toBe(0) + expect(valueB).toBe(0) + expect(valueC).toBe(0) + }) + + it('should handle range references in cycles', () => { + const engine = HyperFormula.buildFromArray([ + ['=SUM(A1:A2)', '=A1'], + ['5', '=A1'] + ], { + allowCircularReferences: true + }) + + const valueA1 = engine.getCellValue(adr('A1')) + const valueB1 = engine.getCellValue(adr('B1')) + const valueA2 = engine.getCellValue(adr('A2')) + const valueB2 = engine.getCellValue(adr('B2')) + + + expect(valueA1).toBe(500) + expect(valueB1).toBe(500) + expect(valueA2).toBe(5) + expect(valueB2).toBe(500) + }) + + it('should work with partialRun operations', () => { + const engine = HyperFormula.buildFromArray([['=B1+1', '=A1+1']], { + allowCircularReferences: true + }) + + engine.setCellContents(adr('C1'), [['=A1*2']]) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + const valueC = engine.getCellValue(adr('C1')) + + expect(valueA).toBe(200) + expect(valueB).toBe(199) + expect(valueC).toBe(400) + }) + + it('should handle cascading cycles', () => { + const engine = HyperFormula.buildFromArray([ + ['=B1+1', '=A1+1', '=D1+1', '=C1+1'] + ], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + const valueC = engine.getCellValue(adr('C1')) + const valueD = engine.getCellValue(adr('D1')) + + expect(valueA).toBe(200) + expect(valueB).toBe(199) + expect(valueC).toBe(200) + expect(valueD).toBe(199) + }) + }) + + describe('configuration validation', () => { + it('should validate allowCircularReferences as boolean', () => { + // eslint-disable-next-line + // @ts-ignore + expect(() => new Config({allowCircularReferences: 'true'})) + .toThrowError('Expected value of type: boolean for config parameter: allowCircularReferences') + + // eslint-disable-next-line + // @ts-ignore + expect(() => new Config({allowCircularReferences: 1})) + .toThrowError('Expected value of type: boolean for config parameter: allowCircularReferences') + + // eslint-disable-next-line + // @ts-ignore + expect(() => new Config({allowCircularReferences: {}})) + .toThrowError('Expected value of type: boolean for config parameter: allowCircularReferences') + }) + + it('should accept valid boolean values', () => { + expect(() => new Config({allowCircularReferences: true})).not.toThrow() + expect(() => new Config({allowCircularReferences: false})).not.toThrow() + }) + + it('should default to false', () => { + const config = new Config() + expect(config.allowCircularReferences).toBe(false) + }) + + it('should preserve configured value', () => { + const configTrue = new Config({allowCircularReferences: true}) + const configFalse = new Config({allowCircularReferences: false}) + + expect(configTrue.allowCircularReferences).toBe(true) + expect(configFalse.allowCircularReferences).toBe(false) + }) + }) + + describe('edge cases', () => { + it('should handle empty cells in cycles', () => { + const engine = HyperFormula.buildFromArray([['=B1', '']], { + allowCircularReferences: true + }) + + engine.setCellContents(adr('B1'), [['=A1']]) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + + expect(valueA).toBe('') + expect(valueB).toBe('') + }) + + it('should handle error values in cycles', () => { + const engine = HyperFormula.buildFromArray([['=B1+1', '=1/0']], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + + expect(valueB).toEqualError(detailedError(ErrorType.DIV_BY_ZERO)) + expect(valueA).toEqualError(detailedError(ErrorType.DIV_BY_ZERO)) + }) + + it('should handle string values in cycles', () => { + const engine = HyperFormula.buildFromArray([['=B1&"a"', '=A1&"b"']], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + + expect(valueA).toBe('0babababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababa') + expect(valueB).toBe('0bababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababab') + }) + + it('should handle very large cycles', () => { + const engine = HyperFormula.buildFromArray([[ + '=B1+1', '=C1+1', '=D1+1', '=E1+1', '=F1+1', '=G1+1', '=H1+1', '=I1+1', '=J1+1', '=A1+1' + ]], { + allowCircularReferences: true + }) + + const valueA = engine.getCellValue(adr('A1')) + const valueB = engine.getCellValue(adr('B1')) + const valueC = engine.getCellValue(adr('C1')) + const valueD = engine.getCellValue(adr('D1')) + const valueE = engine.getCellValue(adr('E1')) + const valueF = engine.getCellValue(adr('F1')) + const valueG = engine.getCellValue(adr('G1')) + const valueH = engine.getCellValue(adr('H1')) + const valueI = engine.getCellValue(adr('I1')) + const valueJ = engine.getCellValue(adr('J1')) + + expect(valueA).toBe(1000) + expect(valueB).toBe(999) + expect(valueC).toBe(998) + expect(valueD).toBe(997) + expect(valueE).toBe(996) + expect(valueF).toBe(995) + expect(valueG).toBe(994) + expect(valueH).toBe(993) + expect(valueI).toBe(992) + expect(valueJ).toBe(991) + }) + }) +}) From be9b00f481d738754b798e78ccba89acde46d8f7 Mon Sep 17 00:00:00 2001 From: Evan Hynes Date: Tue, 16 Sep 2025 10:39:05 +0100 Subject: [PATCH 2/2] fix: Use destructured allowCircularReferences var in Config constructor --- src/Config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config.ts b/src/Config.ts index 855f4d7f6..25f6246cb 100644 --- a/src/Config.ts +++ b/src/Config.ts @@ -213,7 +213,7 @@ export class Config implements ConfigParams, ParserConfig { this.useArrayArithmetic = configValueFromParam(useArrayArithmetic, 'boolean', 'useArrayArithmetic') this.accentSensitive = configValueFromParam(accentSensitive, 'boolean', 'accentSensitive') - this.allowCircularReferences = configValueFromParam(options.allowCircularReferences, 'boolean', 'allowCircularReferences') + this.allowCircularReferences = configValueFromParam(allowCircularReferences, 'boolean', 'allowCircularReferences') this.caseSensitive = configValueFromParam(caseSensitive, 'boolean', 'caseSensitive') this.caseFirst = configValueFromParam(caseFirst, ['upper', 'lower', 'false'], 'caseFirst') this.ignorePunctuation = configValueFromParam(ignorePunctuation, 'boolean', 'ignorePunctuation')