diff --git a/src/java/org/apache/poi/hssf/record/formula/functions/Index.java b/src/java/org/apache/poi/hssf/record/formula/functions/Index.java index c005adffa..badb84640 100644 --- a/src/java/org/apache/poi/hssf/record/formula/functions/Index.java +++ b/src/java/org/apache/poi/hssf/record/formula/functions/Index.java @@ -50,11 +50,23 @@ public final class Index implements Function2Arg, Function3Arg, Function4Arg { public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0, ValueEval arg1) { TwoDEval reference = convertFirstArg(arg0); - boolean colArgWasPassed = false; int columnIx = 0; try { int rowIx = resolveIndexArg(arg1, srcRowIndex, srcColumnIndex); - return getValueFromArea(reference, rowIx, columnIx, colArgWasPassed, srcRowIndex, srcColumnIndex); + + if (!reference.isColumn()) { + if (!reference.isRow()) { + // always an error with 2-D area refs + // Note - the type of error changes if the pRowArg is negative + return ErrorEval.REF_INVALID; + } + // When the two-arg version of INDEX() has been invoked and the reference + // is a single column ref, the row arg seems to get used as the column index + columnIx = rowIx; + rowIx = 0; + } + + return getValueFromArea(reference, rowIx, columnIx); } catch (EvaluationException e) { return e.getErrorEval(); } @@ -63,11 +75,10 @@ public final class Index implements Function2Arg, Function3Arg, Function4Arg { ValueEval arg2) { TwoDEval reference = convertFirstArg(arg0); - boolean colArgWasPassed = true; try { int columnIx = resolveIndexArg(arg2, srcRowIndex, srcColumnIndex); int rowIx = resolveIndexArg(arg1, srcRowIndex, srcColumnIndex); - return getValueFromArea(reference, rowIx, columnIx, colArgWasPassed, srcRowIndex, srcColumnIndex); + return getValueFromArea(reference, rowIx, columnIx); } catch (EvaluationException e) { return e.getErrorEval(); } @@ -110,101 +121,49 @@ public final class Index implements Function2Arg, Function3Arg, Function4Arg { return ErrorEval.VALUE_INVALID; } - - /** - * @param colArgWasPassed false if the INDEX argument list had just 2 items - * (exactly 1 comma). If anything is passed for the column_num argument - * (including {@link BlankEval} or {@link MissingArgEval}) this parameter will be - * true. This parameter is needed because error codes are slightly - * different when only 2 args are passed. - */ - private static ValueEval getValueFromArea(TwoDEval ae, int pRowIx, int pColumnIx, - boolean colArgWasPassed, int srcRowIx, int srcColIx) throws EvaluationException { - boolean rowArgWasEmpty = pRowIx == 0; - boolean colArgWasEmpty = pColumnIx == 0; - int rowIx; - int columnIx; - - // when the area ref is a single row or a single column, - // there are special rules for conversion of rowIx and columnIx - if (ae.isRow()) { - if (ae.isColumn()) { - // single cell ref - rowIx = rowArgWasEmpty ? 0 : pRowIx-1; - columnIx = colArgWasEmpty ? 0 : pColumnIx-1; - } else { - if (colArgWasPassed) { - rowIx = rowArgWasEmpty ? 0 : pRowIx-1; - columnIx = pColumnIx-1; - } else { - // special case - row arg seems to get used as the column index - rowIx = 0; - // transfer both the index value and the empty flag from 'row' to 'column': - columnIx = pRowIx-1; - colArgWasEmpty = rowArgWasEmpty; - } - } - } else if (ae.isColumn()) { - if (rowArgWasEmpty) { - if (ae instanceof AreaEval) { - rowIx = srcRowIx - ((AreaEval) ae).getFirstRow(); - } else { - // TODO - ArrayEval - // rowIx = relative row of evaluating cell in its array formula cell group - throw new RuntimeException("incomplete code - "); - } - } else { - rowIx = pRowIx-1; - } - if (colArgWasEmpty) { - columnIx = 0; - } else { - columnIx = colArgWasEmpty ? 0 : pColumnIx-1; - } - } else { - // ae is an area (not single row or column) - if (!colArgWasPassed) { - // always an error with 2-D area refs - // Note - the type of error changes if the pRowArg is negative - throw new EvaluationException(pRowIx < 0 ? ErrorEval.VALUE_INVALID : ErrorEval.REF_INVALID); - } - // Normal case - area ref is 2-D, and both index args were provided - // if either arg is missing (or blank) the logic is similar to OperandResolver.getSingleValue() - if (rowArgWasEmpty) { - if (ae instanceof AreaEval) { - rowIx = srcRowIx - ((AreaEval) ae).getFirstRow(); - } else { - // TODO - ArrayEval - // rowIx = relative row of evaluating cell in its array formula cell group - throw new RuntimeException("incomplete code - "); - } - } else { - rowIx = pRowIx-1; - } - if (colArgWasEmpty) { - if (ae instanceof AreaEval) { - columnIx = srcColIx - ((AreaEval) ae).getFirstColumn(); - } else { - // TODO - ArrayEval - // colIx = relative col of evaluating cell in its array formula cell group - throw new RuntimeException("incomplete code - "); - } - } else { - columnIx = pColumnIx-1; - } - } - + private static ValueEval getValueFromArea(TwoDEval ae, int pRowIx, int pColumnIx) + throws EvaluationException { + assert pRowIx >= 0; + assert pColumnIx >= 0; + int width = ae.getWidth(); int height = ae.getHeight(); - // Slightly irregular logic for bounds checking errors - if (!rowArgWasEmpty && rowIx >= height || !colArgWasEmpty && columnIx >= width) { - // high bounds check fail gives #REF! if arg was explicitly passed - throw new EvaluationException(ErrorEval.REF_INVALID); + + int relFirstRowIx; + int relLastRowIx; + + if ((pRowIx == 0)) { + relFirstRowIx = 0; + relLastRowIx = height-1; + } else { + // Slightly irregular logic for bounds checking errors + if (pRowIx > height) { + // high bounds check fail gives #REF! if arg was explicitly passed + throw new EvaluationException(ErrorEval.REF_INVALID); + } + int rowIx = pRowIx-1; + relFirstRowIx = rowIx; + relLastRowIx = rowIx; } - if (rowIx < 0 || columnIx < 0 || rowIx >= height || columnIx >= width) { - throw new EvaluationException(ErrorEval.VALUE_INVALID); + + int relFirstColIx; + int relLastColIx; + if ((pColumnIx == 0)) { + relFirstColIx = 0; + relLastColIx = width-1; + } else { + // Slightly irregular logic for bounds checking errors + if (pColumnIx > width) { + // high bounds check fail gives #REF! if arg was explicitly passed + throw new EvaluationException(ErrorEval.REF_INVALID); + } + int columnIx = pColumnIx-1; + relFirstColIx = columnIx; + relLastColIx = columnIx; } - return ae.getValue(rowIx, columnIx); + + AreaEval x = ((AreaEval) ae); + return x.offset(relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx); } diff --git a/src/testcases/org/apache/poi/hssf/record/formula/functions/EvalFactory.java b/src/testcases/org/apache/poi/hssf/record/formula/functions/EvalFactory.java index 433ed9df4..57d707262 100644 --- a/src/testcases/org/apache/poi/hssf/record/formula/functions/EvalFactory.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/functions/EvalFactory.java @@ -17,6 +17,7 @@ package org.apache.poi.hssf.record.formula.functions; +import org.apache.poi.hssf.record.formula.AreaI; import org.apache.poi.hssf.record.formula.AreaPtg; import org.apache.poi.hssf.record.formula.Ref3DPtg; import org.apache.poi.hssf.record.formula.RefPtg; @@ -78,10 +79,14 @@ public final class EvalFactory { private static final class MockAreaEval extends AreaEvalBase { private final ValueEval[] _values; - public MockAreaEval(AreaPtg areaPtg, ValueEval[] values) { + public MockAreaEval(AreaI areaPtg, ValueEval[] values) { super(areaPtg); _values = values; } + private MockAreaEval(int firstRow, int firstColumn, int lastRow, int lastColumn, ValueEval[] values) { + super(firstRow, firstColumn, lastRow, lastColumn); + _values = values; + } public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) { if (relativeRowIndex < 0 || relativeRowIndex >=getHeight()) { throw new IllegalArgumentException("row index out of range"); @@ -94,11 +99,35 @@ public final class EvalFactory { return _values[oneDimensionalIndex]; } public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) { + if (relFirstRowIx < 0 || relFirstColIx < 0 + || relLastRowIx >= getHeight() || relLastColIx >= getWidth()) { + throw new RuntimeException("Operation not implemented on this mock object"); + } + if (relFirstRowIx == 0 && relFirstColIx == 0 && relLastRowIx == getHeight()-1 && relLastColIx == getWidth()-1) { return this; } - throw new RuntimeException("Operation not implemented on this mock object"); + ValueEval[] values = transpose(_values, getWidth(), relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx); + return new MockAreaEval(getFirstRow() + relFirstRowIx, getFirstColumn() + relFirstColIx, + getFirstRow() + relLastRowIx, getFirstColumn() + relLastColIx, values); + } + private static ValueEval[] transpose(ValueEval[] srcValues, int srcWidth, + int relFirstRowIx, int relLastRowIx, + int relFirstColIx, int relLastColIx) { + int height = relLastRowIx - relFirstRowIx + 1; + int width = relLastColIx - relFirstColIx + 1; + ValueEval[] result = new ValueEval[height * width]; + for (int r=0; r @@ -83,10 +87,17 @@ public final class TestIndex extends TestCase { args = new ValueEval[] { arg0, new NumberEval(rowNum), }; } - double actual = NumericFunctionInvoker.invoke(FUNC_INST, args); + double actual = invokeAndDereference(args); assertEquals(expectedResult, actual, 0D); } + private static double invokeAndDereference(ValueEval[] args) { + ValueEval ve = FUNC_INST.evaluate(args, -1, -1); + ve = WorkbookEvaluator.dereferenceResult(ve, -1, -1); + assertEquals(NumberEval.class, ve.getClass()); + return ((NumberEval)ve).getNumberValue(); + } + /** * Tests expressions like "INDEX(A1:C1,,2)".
* This problem was found while fixing bug 47048 and is observable up to svn r773441. @@ -101,14 +112,46 @@ public final class TestIndex extends TestCase { ValueEval[] args = new ValueEval[] { arg0, MissingArgEval.instance, new NumberEval(2), }; ValueEval actualResult; try { - actualResult = FUNC_INST.evaluate(args, 1, (short)1); + actualResult = FUNC_INST.evaluate(args, -1, -1); } catch (RuntimeException e) { if (e.getMessage().equals("Unexpected arg eval type (org.apache.poi.hssf.record.formula.eval.MissingArgEval")) { throw new AssertionFailedError("Identified bug 47048b - INDEX() should support missing-arg"); } throw e; } + // result should be an area eval "B10:B10" + AreaEval ae = confirmAreaEval("B10:B10", actualResult); + actualResult = ae.getValue(0, 0); assertEquals(NumberEval.class, actualResult.getClass()); assertEquals(26.0, ((NumberEval)actualResult).getNumberValue(), 0.0); } + + /** + * When the argument to INDEX is a reference, the result should be a reference + * A formula like "OFFSET(INDEX(A1:B2,2,1),1,1,1,1)" should return the value of cell B3. + * This works because the INDEX() function returns a reference to A2 (not the value of A2) + */ + public void testReferenceResult() { + ValueEval[] values = new ValueEval[4]; + Arrays.fill(values, NumberEval.ZERO); + AreaEval arg0 = EvalFactory.createAreaEval("A1:B2", values); + ValueEval[] args = new ValueEval[] { arg0, new NumberEval(2), new NumberEval(1), }; + ValueEval ve = FUNC_INST.evaluate(args, -1, -1); + confirmAreaEval("A2:A2", ve); + } + + /** + * Confirms that the result is an area ref with the specified coordinates + * @return ve cast to {@link AreaEval} if it is valid + */ + private static AreaEval confirmAreaEval(String refText, ValueEval ve) { + CellRangeAddress cra = CellRangeAddress.valueOf(refText); + assertTrue(ve instanceof AreaEval); + AreaEval ae = (AreaEval) ve; + assertEquals(cra.getFirstRow(), ae.getFirstRow()); + assertEquals(cra.getFirstColumn(), ae.getFirstColumn()); + assertEquals(cra.getLastRow(), ae.getLastRow()); + assertEquals(cra.getLastColumn(), ae.getLastColumn()); + return ae; + } } diff --git a/test-data/spreadsheet/IndexFunctionTestCaseData.xls b/test-data/spreadsheet/IndexFunctionTestCaseData.xls index bf0b23acc..d1de30457 100644 Binary files a/test-data/spreadsheet/IndexFunctionTestCaseData.xls and b/test-data/spreadsheet/IndexFunctionTestCaseData.xls differ