diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d04650d7..75d038cf1 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/), - Fixed an issue where `simpleCellAddressFromString` method was crashing when called with a non-ASCII character in an unquoted sheet name. [#1312](https://github.com/handsontable/hyperformula/issues/1312) - Fixed a bug where adding a row to the very large spreadsheet resulted in `Maximum call stack size exceeded` error. [#1332](https://github.com/handsontable/hyperformula/issues/1332) - Fixed a typo in the JSDoc comment of the `HyperFormula` class. [#1323](https://github.com/handsontable/hyperformula/issues/1323) +- Fixed the `Incorrect array size` error when using column range reference to an empty sheet as a function argument. [#1147](https://github.com/handsontable/hyperformula/issues/1147) - Fixed a bug where function SUBSTITUTE did not work correctly with regexp special characters. [#1289](https://github.com/handsontable/hyperformula/issues/1289) ## [2.6.0] - 2023-09-19 diff --git a/src/ArraySize.ts b/src/ArraySize.ts index 2772956f2..706cd55b1 100644 --- a/src/ArraySize.ts +++ b/src/ArraySize.ts @@ -8,7 +8,7 @@ import {SimpleCellAddress} from './Cell' import {Config} from './Config' import {FunctionRegistry} from './interpreter/FunctionRegistry' import {InterpreterState} from './interpreter/InterpreterState' -import {FunctionArgumentType} from './interpreter/plugin/FunctionPlugin' +import {FunctionArgumentType} from './interpreter' import {Ast, AstNodeType, ProcedureAst} from './parser' export class ArraySize { @@ -16,15 +16,7 @@ export class ArraySize { public width: number, public height: number, public isRef: boolean = false, - ) { - if (width <= 0 || height <= 0) { - throw Error('Incorrect array size') - } - } - - public static fromArray(array: T[][]): ArraySize { - return new ArraySize(array.length > 0 ? array[0].length : 0, array.length) - } + ) {} public static error() { return new ArraySize(1, 1, true) @@ -35,7 +27,7 @@ export class ArraySize { } isScalar(): boolean { - return (this.width <= 1 && this.height <= 1) || this.isRef + return (this.width === 1 && this.height === 1) || this.isRef } } @@ -131,37 +123,53 @@ export class ArraySizePredictor { } private checkArraySizeForFunction(ast: ProcedureAst, state: InterpreterState): ArraySize { - const metadata = this.functionRegistry.getMetadata(ast.procedureName) const pluginArraySizeFunction = this.functionRegistry.getArraySizeFunction(ast.procedureName) + if (pluginArraySizeFunction !== undefined) { return pluginArraySizeFunction(ast, state) } - const subChecks = ast.args.map((arg) => this.checkArraySizeForAst(arg, new InterpreterState(state.formulaAddress, state.arraysFlag || (metadata?.arrayFunction ?? false)))) - if (metadata === undefined || metadata.expandRanges || !state.arraysFlag || metadata.vectorizationForbidden || metadata.parameters === undefined) { + + const metadata = this.functionRegistry.getMetadata(ast.procedureName) + + if ( + metadata === undefined + || metadata.expandRanges + || !state.arraysFlag + || metadata.vectorizationForbidden + || metadata.parameters === undefined + ) { return new ArraySize(1, 1) } + + const subChecks = ast.args.map((arg) => this.checkArraySizeForAst(arg, new InterpreterState(state.formulaAddress, state.arraysFlag || (metadata?.arrayFunction ?? false)))) const argumentDefinitions = [...metadata.parameters] - if (metadata.repeatLastArgs === undefined && argumentDefinitions.length < subChecks.length) { - return ArraySize.error() - } - if (metadata.repeatLastArgs !== undefined && argumentDefinitions.length < subChecks.length && - (subChecks.length - argumentDefinitions.length) % metadata.repeatLastArgs !== 0) { + + if ( + metadata.repeatLastArgs !== undefined + && argumentDefinitions.length < subChecks.length + && (subChecks.length - argumentDefinitions.length) % metadata.repeatLastArgs !== 0 + ) { return ArraySize.error() } while (argumentDefinitions.length < subChecks.length) { - argumentDefinitions.push(...argumentDefinitions.slice(argumentDefinitions.length - metadata.repeatLastArgs!)) + if (metadata.repeatLastArgs === undefined) { + return ArraySize.error() + } + + argumentDefinitions.push(...argumentDefinitions.slice(argumentDefinitions.length - metadata.repeatLastArgs)) } let maxWidth = 1 let maxHeight = 1 + for (let i = 0; i < subChecks.length; i++) { if (argumentDefinitions[i].argumentType !== FunctionArgumentType.RANGE && argumentDefinitions[i].argumentType !== FunctionArgumentType.ANY) { maxHeight = Math.max(maxHeight, subChecks[i].height) maxWidth = Math.max(maxWidth, subChecks[i].width) } } + return new ArraySize(maxWidth, maxHeight) } } - diff --git a/src/ArrayValue.ts b/src/ArrayValue.ts index bd711e963..5838760ee 100644 --- a/src/ArrayValue.ts +++ b/src/ArrayValue.ts @@ -49,6 +49,10 @@ export class ArrayValue implements CellArray { constructor(array: InternalScalarValue[][]) { this.size = new ArraySize(array.length > 0 ? array[0].length : 0, array.length) this.array = array + + if (this.size.width <= 0 || this.size.height <= 0) { + throw Error('Incorrect array size') + } } static fromInterpreterValue(value: InterpreterValue) { diff --git a/src/Operations.ts b/src/Operations.ts index 7ab299ad7..ce2298bfb 100644 --- a/src/Operations.ts +++ b/src/Operations.ts @@ -563,6 +563,11 @@ export class Operations { } else { try { const size = this.arraySizePredictor.checkArraySize(ast, address) + + if (size.width <= 0 || size.height <= 0) { + throw Error('Incorrect array size') + } + this.setFormulaToCell(address, size, parserResult) } catch (error) { diff --git a/test/interpreter/function-countif.spec.ts b/test/interpreter/function-countif.spec.ts index 0af0359f4..39c9a6b26 100644 --- a/test/interpreter/function-countif.spec.ts +++ b/test/interpreter/function-countif.spec.ts @@ -135,4 +135,13 @@ describe('Function COUNTIF', () => { expect(engine.getCellValue(adr('A4'))).toEqual(2) }) + + it('works with a column range reference to an empty sheet', () => { + const hf = HyperFormula.buildFromSheets({ + table1: [], + table2: [['=COUNTIF(table1!A:C, ">1")']], + }) + + expect(hf.getCellValue(adr('A1', 1))).toEqual(0) + }) }) diff --git a/test/interpreter/function-ifs.spec.ts b/test/interpreter/function-ifs.spec.ts index 152167409..c63b23f50 100644 --- a/test/interpreter/function-ifs.spec.ts +++ b/test/interpreter/function-ifs.spec.ts @@ -15,6 +15,18 @@ describe('Function IFS', () => { expect(engine.getCellValue(adr('B3'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) }) + it('Should not work for wrong number of arguments when array arithmetic is on', () => { + const engine = HyperFormula.buildFromArray([ + [10, '=IFS()'], + [20, '=IFS(A1>90)'], + [30, '=IFS(A1>90, "A", A1>80)'], + ], { useArrayArithmetic: true }) + + expect(engine.getCellValue(adr('B1'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) + expect(engine.getCellValue(adr('B2'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) + expect(engine.getCellValue(adr('B3'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) + }) + it('Nominal operation', () => { const engine = HyperFormula.buildFromArray([ [11, '=IFS(A1>30, "A", A1>20, "B", A1>10, "C")'], diff --git a/test/interpreter/function-match.spec.ts b/test/interpreter/function-match.spec.ts index 059b1841d..a2ab4e633 100644 --- a/test/interpreter/function-match.spec.ts +++ b/test/interpreter/function-match.spec.ts @@ -14,6 +14,16 @@ describe('Function MATCH', () => { expect(engine.getCellValue(adr('A2'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) }) + it('validates number of arguments when array arithmetics is on', () => { + const engine = HyperFormula.buildFromArray([ + ['=MATCH(1)'], + ['=MATCH(1, B1:B3, 0, 42)'], + ], { useArrayArithmetic: true }) + + expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) + expect(engine.getCellValue(adr('A2'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) + }) + it('validates that 1st argument is number, string or boolean', () => { const engine = HyperFormula.buildFromArray([ ['=MATCH(C2:C3, B1:B1)'], @@ -704,4 +714,45 @@ describe('Function MATCH', () => { expect(engine.getCellValue(adr('A3'))).toEqualError(detailedError(ErrorType.NA)) }) + + describe('works with a range reference to an empty sheet', () => { + it('- cell range', () => { + const hf = HyperFormula.buildFromSheets({ + table1: [], + table2: [['=MATCH(0, table1!A1:A10)']], + }) + + expect(hf.getCellValue(adr('A1', 1))).toEqualError(detailedError(ErrorType.NA)) + }) + + it('- column range', () => { + const hf = HyperFormula.buildFromSheets({ + table1: [[]], + table2: [['=MATCH(0, table1!A:A)']], + }) + + expect(hf.getCellValue(adr('A1', 1))).toEqualError(detailedError(ErrorType.NA)) + }) + + it('- row range', () => { + const hf = HyperFormula.buildFromSheets({ + table1: [], + table2: [['=MATCH(0, table1!1:1)']], + }) + + expect(hf.getCellValue(adr('A1', 1))).toEqualError(detailedError(ErrorType.NA)) + }) + + it('- column range (called from calculateFormula)', () => { + const hf = HyperFormula.buildFromArray([]) + + expect(hf.calculateFormula('=MATCH(0, Sheet1!A:A)', 0)).toEqualError(detailedError(ErrorType.NA)) + }) + + it('- column range (called from calculateFormula without explicit sheet reference)', () => { + const hf = HyperFormula.buildFromArray([]) + + expect(hf.calculateFormula('=MATCH(0, A:A)', 0)).toEqualError(detailedError(ErrorType.NA)) + }) + }) }) diff --git a/test/interpreter/function-sum.spec.ts b/test/interpreter/function-sum.spec.ts index d7d860972..ca3b16119 100644 --- a/test/interpreter/function-sum.spec.ts +++ b/test/interpreter/function-sum.spec.ts @@ -120,6 +120,15 @@ describe('SUM', () => { expect(engine.getCellValue(adr('A3'))).toEqualError(detailedError(ErrorType.DIV_BY_ZERO)) }) + it('works with a column range reference to an empty sheet', () => { + const hf = HyperFormula.buildFromSheets({ + table1: [], + table2: [['=SUM(table1!A:C)']], + }) + + expect(hf.getCellValue(adr('A1', 1))).toEqual(0) + }) + describe('works with reversed cell ranges', () => { it('simple case', () => { const engine = HyperFormula.buildFromArray([ diff --git a/test/interpreter/function-vlookup.spec.ts b/test/interpreter/function-vlookup.spec.ts index 379122702..98ca71871 100644 --- a/test/interpreter/function-vlookup.spec.ts +++ b/test/interpreter/function-vlookup.spec.ts @@ -840,4 +840,13 @@ describe('BinarySearchStrategy', () => { expect(engine.getCellValue(adr('A1'))).toEqual('1') }) + + it('works with a column range reference to an empty sheet', () => { + const hf = HyperFormula.buildFromSheets({ + table1: [], + table2: [['=VLOOKUP("42", table1!A:C, 1)']], + }) + + expect(hf.getCellValue(adr('A1', 1))).toEqualError(detailedError(ErrorType.NA)) + }) })