CellError fields should be immutable. Fixed CYCLE address.

handsontable · Oct 8, 2020 · 1f112c6 · 1f112c6
1 parent 8c4cff4
commit 1f112c6
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 20 deletions.
diff --git a/src/Cell.ts b/src/Cell.ts
@@ -113,16 +113,16 @@ export class CellError {
   constructor(
     public readonly type: ErrorType,
     public readonly message?: string,
-    private address?: SimpleCellAddress
+    public readonly address?: SimpleCellAddress
   ) {
   }
 
-  public getOriginAddress(): Maybe<SimpleCellAddress> {
-    return this.address
-  }
-
-  public attachAddress(address: SimpleCellAddress): void {
-    this.address = this.address ?? address
+  public attachAddress(address: SimpleCellAddress): CellError {
+    if(this.address === undefined) {
+      return new CellError(this.type, this.message, address)
+    } else {
+      return this
+    }
   }
 
   public static parsingError() {

diff --git a/src/Evaluator.ts b/src/Evaluator.ts
@@ -96,7 +96,7 @@ export class Evaluator {
           } else if (vertex instanceof FormulaCellVertex) {
             const address = vertex.getAddress(this.dependencyGraph.lazilyTransformingAstService)
             this.columnSearch.remove(vertex.valueOrNull(), address)
-            const error = new CellError(ErrorType.CYCLE)
+            const error = new CellError(ErrorType.CYCLE, undefined, vertex.address)
             vertex.setCellValue(error)
             changes.addChange(error, vertex.address)
           }
@@ -137,7 +137,7 @@ export class Evaluator {
   private recomputeFormulas(cycled: Vertex[], sorted: Vertex[]): void {
     cycled.forEach((vertex: Vertex) => {
       if (vertex instanceof FormulaCellVertex) {
-        vertex.setCellValue(new CellError(ErrorType.CYCLE))
+        vertex.setCellValue(new CellError(ErrorType.CYCLE, undefined, vertex.address))
       }
     })
     sorted.forEach((vertex: Vertex) => {

diff --git a/src/Exporter.ts b/src/Exporter.ts
@@ -94,7 +94,7 @@ export class Exporter {
 
   private detailedError(error: CellError): DetailedCellError {
     let address = undefined
-    const originAddress = error.getOriginAddress()
+    const originAddress = error.address
     if(originAddress !== undefined) {
       if (originAddress.sheet === NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS) {
         address = this.namedExpressions.namedExpressionInAddress(originAddress.row)?.displayName

diff --git a/src/interpreter/Interpreter.ts b/src/interpreter/Interpreter.ts
@@ -323,7 +323,7 @@ function wrapperBinary<T extends InterpreterValue>(op: (a: T, b: T) => Interpret
 
 function wrapperForAddress(val: InterpreterValue, adr: SimpleCellAddress): InterpreterValue {
   if(val instanceof CellError) {
-    val.attachAddress(adr)
+    return val.attachAddress(adr)
   }
   return val
 }
diff --git a/test/error-address-preservation.spec.ts b/test/error-address-preservation.spec.ts
@@ -1,44 +1,70 @@
 import {ErrorType, HyperFormula} from '../src'
-import {adr, detailedErrorWithOrigin} from './testUtils'
+import {adr, detailedError, detailedErrorWithOrigin} from './testUtils'
 
 describe('Address preservation.', () => {
   it('Should work in the basic case.', () => {
     const engine = HyperFormula.buildFromArray([
       ['=NA()', '=A1']
     ])
-    expect(engine.getCellValue(adr('A1'))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
-    expect(engine.getCellValue(adr('B1'))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('A1'))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('B1'))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
   })
 
   it('Should work with named expressions.', () => {
     const engine = HyperFormula.buildFromArray([
       ['=NAMEDEXPRESSION', '=A1']
     ])
     engine.addNamedExpression('NAMEDEXPRESSION', '=NA()')
-    expect(engine.getCellValue(adr('A1'))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
-    expect(engine.getCellValue(adr('B1'))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('A1'))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'NAMEDEXPRESSION'))
+    expect(engine.getCellValue(adr('B1'))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'NAMEDEXPRESSION'))
   })
 
   it('Should work with operators.', () => {
     const engine = HyperFormula.buildFromArray([
       ['=NA()', '=NA()', '=A1+B1']
     ])
-    expect(engine.getCellValue(adr('C1'))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('C1'))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
   })
 
   it('Should work between sheets.', () => {
     const engine = HyperFormula.buildFromSheets({
       sheet1: [['=NA()']],
       sheet2: [['=sheet1!A1']]
     })
-    expect(engine.getCellValue(adr('A1', 0))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'sheet1!A1'))
-    expect(engine.getCellValue(adr('A1', 1))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'sheet1!A1'))
+    expect(engine.getCellValue(adr('A1', 0))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'sheet1!A1'))
+    expect(engine.getCellValue(adr('A1', 1))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'sheet1!A1'))
   })
 
   it('Should work with function calls.', () => {
     const engine = HyperFormula.buildFromArray([
       ['=NA()', '=DATE(1,1,A1)']
     ])
-    expect(engine.getCellValue(adr('B1'))).toEqualError(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('B1'))).toEqual(detailedErrorWithOrigin(ErrorType.NA, 'Sheet1!A1'))
+  })
+
+  it('Should work with CYCLE.', () => {
+    const engine = HyperFormula.buildFromArray([
+      ['=B1', '=A1'],
+      ['=A1', '=B1'],
+      ['=A1', '=B1']
+    ])
+    expect(engine.getCellValue(adr('A1'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('B1'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!B1'))
+    expect(engine.getCellValue(adr('A2'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('B2'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!B1'))
+    expect(engine.getCellValue(adr('A3'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('B3'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!B1'))
+  })
+
+  it('Should work with CYCLE #2.', () => {
+    const engine = HyperFormula.buildFromArray([
+      ['=B1', '=A1'],
+      ['=A1'],
+      ['=A1']
+    ])
+    expect(engine.getCellValue(adr('A1'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('B1'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!B1'))
+    expect(engine.getCellValue(adr('A2'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!A1'))
+    expect(engine.getCellValue(adr('A3'))).toEqual(detailedErrorWithOrigin(ErrorType.CYCLE, 'Sheet1!A1'))
   })
 })