Fixed INDEX function to return reference results properly.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@891516 13f79535-47bb-0310-9956-ffa450edef68

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;
 	}
 
-
+	private static ValueEval getValueFromArea(TwoDEval ae, int pRowIx, int pColumnIx)
-	/**
+			throws EvaluationException {
-	 * @param colArgWasPassed <code>false</code> if the INDEX argument list had just 2 items
+		assert pRowIx >= 0;
-	 *            (exactly 1 comma).  If anything is passed for the <tt>column_num</tt> argument
+		assert pColumnIx >= 0;
-	 *            (including {@link BlankEval} or {@link MissingArgEval}) this parameter will be
-	 *            <code>true</code>.  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;
-			}
-		}
 	
 		int width = ae.getWidth();
 		int height = ae.getHeight();
+		
+		int relFirstRowIx;
+		int relLastRowIx;
+
+		if ((pRowIx == 0)) {
+			relFirstRowIx = 0;
+			relLastRowIx = height-1;
+		} else {
 			// Slightly irregular logic for bounds checking errors
-		if (!rowArgWasEmpty && rowIx >= height || !colArgWasEmpty && columnIx >= width) {
+			if (pRowIx > height) {
 				// high bounds check fail gives #REF! if arg was explicitly passed
 				throw new EvaluationException(ErrorEval.REF_INVALID);
 			}
-		if (rowIx < 0 || columnIx < 0 || rowIx >= height || columnIx >= width) {
+			int rowIx = pRowIx-1;
-			throw new EvaluationException(ErrorEval.VALUE_INVALID);
+			relFirstRowIx = rowIx;
+			relLastRowIx = rowIx;
 		}
-		return ae.getValue(rowIx, columnIx);
+
+		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;
+		}
+
+		AreaEval x = ((AreaEval) ae);
+		return x.offset(relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx);
 	}
 
 

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<height; r++) {
+				int srcRowIx = r + relFirstRowIx; 
+				for (int c=0; c<width; c++) {
+					int srcColIx = c + relFirstColIx;
+					int destIx = r * width + c;
+					int srcIx = srcRowIx * srcWidth + srcColIx;
+					result[destIx] = srcValues[srcIx];
+				}
+			}
+			return result;
 		}
 	}
 

src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java

@@ -17,6 +17,8 @@
 
 package org.apache.poi.hssf.record.formula.functions;
 
+import java.util.Arrays;
+
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
@@ -24,6 +26,8 @@ import org.apache.poi.hssf.record.formula.eval.AreaEval;
 import org.apache.poi.hssf.record.formula.eval.MissingArgEval;
 import org.apache.poi.hssf.record.formula.eval.NumberEval;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
+import org.apache.poi.ss.formula.WorkbookEvaluator;
+import org.apache.poi.ss.util.CellRangeAddress;
 
 /**
  * Tests for the INDEX() function.</p>
@@ -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)".<br/>
 	 * 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 <tt>ve</tt> 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;
+	}
 }