diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/OperandResolver.java b/src/java/org/apache/poi/hssf/record/formula/eval/OperandResolver.java index f18abce008..0b9116db3e 100644 --- a/src/java/org/apache/poi/hssf/record/formula/eval/OperandResolver.java +++ b/src/java/org/apache/poi/hssf/record/formula/eval/OperandResolver.java @@ -103,14 +103,8 @@ public final class OperandResolver { public static ValueEval chooseSingleElementFromArea(AreaEval ae, int srcCellRow, int srcCellCol) throws EvaluationException { ValueEval result = chooseSingleElementFromAreaInternal(ae, srcCellRow, srcCellCol); - if(result == null) { - // This seems to be required because AreaEval.values() array may contain nulls. - // perhaps that should not be allowed. - result = BlankEval.instance; - } if (result instanceof ErrorEval) { throw new EvaluationException((ErrorEval) result); - } return result; } diff --git a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java index 69e1446d44..0e63016250 100644 --- a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java @@ -57,6 +57,7 @@ import org.apache.poi.hssf.record.formula.eval.MissingArgEval; import org.apache.poi.hssf.record.formula.eval.NameEval; import org.apache.poi.hssf.record.formula.eval.NameXEval; import org.apache.poi.hssf.record.formula.eval.NumberEval; +import org.apache.poi.hssf.record.formula.eval.OperandResolver; import org.apache.poi.hssf.record.formula.eval.RefEval; import org.apache.poi.hssf.record.formula.eval.StringEval; import org.apache.poi.hssf.record.formula.eval.ValueEval; @@ -449,14 +450,7 @@ public final class WorkbookEvaluator { if (!stack.isEmpty()) { throw new IllegalStateException("evaluation stack not empty"); } - value = dereferenceValue(value, ec.getRowIndex(), ec.getColumnIndex()); - if (value == BlankEval.instance) { - // Note Excel behaviour here. A blank final final value is converted to zero. - return NumberEval.ZERO; - // Formulas _never_ evaluate to blank. If a formula appears to have evaluated to - // blank, the actual value is empty string. This can be verified with ISBLANK(). - } - return value; + return dereferenceResult(value, ec.getRowIndex(), ec.getColumnIndex()); } /** @@ -480,31 +474,30 @@ public final class WorkbookEvaluator { } return index-startIndex; } + /** - * Dereferences a single value from any AreaEval or RefEval evaluation result. - * If the supplied evaluationResult is just a plain value, it is returned as-is. - * @return a NumberEval, StringEval, BoolEval, - * BlankEval or ErrorEval. Never null. + * Dereferences a single value from any AreaEval or RefEval evaluation + * result. If the supplied evaluationResult is just a plain value, it is + * returned as-is. + * + * @return a {@link NumberEval}, {@link StringEval}, {@link BoolEval}, or + * {@link ErrorEval}. Never null. {@link BlankEval} is + * converted to {@link NumberEval#ZERO} */ - private static ValueEval dereferenceValue(ValueEval evaluationResult, int srcRowNum, int srcColNum) { - if (evaluationResult instanceof RefEval) { - RefEval rv = (RefEval) evaluationResult; - return rv.getInnerValueEval(); + public static ValueEval dereferenceResult(ValueEval evaluationResult, int srcRowNum, int srcColNum) { + ValueEval value; + try { + value = OperandResolver.getSingleValue(evaluationResult, srcRowNum, srcColNum); + } catch (EvaluationException e) { + return e.getErrorEval(); } - if (evaluationResult instanceof AreaEval) { - AreaEval ae = (AreaEval) evaluationResult; - if (ae.isRow()) { - if(ae.isColumn()) { - return ae.getRelativeValue(0, 0); - } - return ae.getAbsoluteValue(ae.getFirstRow(), srcColNum); - } - if (ae.isColumn()) { - return ae.getAbsoluteValue(srcRowNum, ae.getFirstColumn()); - } - return ErrorEval.VALUE_INVALID; + if (value == BlankEval.instance) { + // Note Excel behaviour here. A blank final final value is converted to zero. + return NumberEval.ZERO; + // Formulas _never_ evaluate to blank. If a formula appears to have evaluated to + // blank, the actual value is empty string. This can be verified with ISBLANK(). } - return evaluationResult; + return value; } diff --git a/src/testcases/org/apache/poi/ss/formula/TestWorkbookEvaluator.java b/src/testcases/org/apache/poi/ss/formula/TestWorkbookEvaluator.java index 7e9149adfa..5e59933cf6 100644 --- a/src/testcases/org/apache/poi/ss/formula/TestWorkbookEvaluator.java +++ b/src/testcases/org/apache/poi/ss/formula/TestWorkbookEvaluator.java @@ -38,7 +38,11 @@ import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator; import org.apache.poi.hssf.usermodel.HSSFRow; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CellValue; +import org.apache.poi.ss.usermodel.ErrorConstants; +import org.apache.poi.ss.usermodel.FormulaEvaluator; +import org.apache.poi.ss.usermodel.Workbook; /** * Tests {@link WorkbookEvaluator}. @@ -193,7 +197,7 @@ public class TestWorkbookEvaluator extends TestCase { assertEquals(HSSFCell.CELL_TYPE_STRING, cv.getCellType()); // adding blank to "abc" gives "abc" assertEquals("abc", cv.getStringValue()); - + // check CHOOSE() cell.setCellFormula("\"abc\"&CHOOSE(2,5,,9)"); fe.notifySetFormula(cell); @@ -202,4 +206,33 @@ public class TestWorkbookEvaluator extends TestCase { // adding blank to "abc" gives "abc" assertEquals("abc", cv.getStringValue()); } + + /** + * Functions like IF, INDIRECT, INDEX, OFFSET etc can return AreaEvals which + * should be dereferenced by the evaluator + */ + public void testResultOutsideRange() { + Workbook wb = new HSSFWorkbook(); + Cell cell = wb.createSheet("Sheet1").createRow(0).createCell(0); + cell.setCellFormula("D2:D5"); // IF(TRUE,D2:D5,D2) or OFFSET(D2:D5,0,0) would work too + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + CellValue cv; + try { + cv = fe.evaluate(cell); + } catch (IllegalArgumentException e) { + if ("Specified row index (0) is outside the allowed range (1..4)".equals(e.getMessage())) { + throw new AssertionFailedError("Identified bug in result dereferencing"); + } + throw new RuntimeException(e); + } + assertEquals(Cell.CELL_TYPE_ERROR, cv.getCellType()); + assertEquals(ErrorConstants.ERROR_VALUE, cv.getErrorValue()); + + // verify circular refs are still detected properly + fe.clearAllCachedResultValues(); + cell.setCellFormula("OFFSET(A1,0,0)"); + cv = fe.evaluate(cell); + assertEquals(Cell.CELL_TYPE_ERROR, cv.getCellType()); + assertEquals(ErrorEval.CIRCULAR_REF_ERROR.getErrorCode(), cv.getErrorValue()); + } }