fixed special cases of dereferenceResult method

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

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;
 	}

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());
+		return dereferenceResult(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;
 	}
 
 	/**
@@ -480,31 +474,30 @@ public final class WorkbookEvaluator {
 		}
 		return index-startIndex;
 	}
+
 	/**
-	 * Dereferences a single value from any AreaEval or RefEval evaluation result.
+	 * Dereferences a single value from any AreaEval or RefEval evaluation
-	 * If the supplied evaluationResult is just a plain value, it is returned as-is.
+	 * result. If the supplied evaluationResult is just a plain value, it is
-	 * @return a <tt>NumberEval</tt>, <tt>StringEval</tt>, <tt>BoolEval</tt>,
+	 * returned as-is.
-	 *  <tt>BlankEval</tt> or <tt>ErrorEval</tt>. Never <code>null</code>.
+	 *
+	 * @return a {@link NumberEval}, {@link StringEval}, {@link BoolEval}, or
+	 *         {@link ErrorEval}. Never <code>null</code>. {@link BlankEval} is
+	 *         converted to {@link NumberEval#ZERO}
 	 */
-	private static ValueEval dereferenceValue(ValueEval evaluationResult, int srcRowNum, int srcColNum) {
+	public static ValueEval dereferenceResult(ValueEval evaluationResult, int srcRowNum, int srcColNum) {
-		if (evaluationResult instanceof RefEval) {
+		ValueEval value;
-			RefEval rv = (RefEval) evaluationResult;
+		try {
-			return rv.getInnerValueEval();
+			value = OperandResolver.getSingleValue(evaluationResult, srcRowNum, srcColNum);
+		} catch (EvaluationException e) {
+			return e.getErrorEval();
 		}
-		if (evaluationResult instanceof AreaEval) {
+		if (value == BlankEval.instance) {
-			AreaEval ae = (AreaEval) evaluationResult;
+			// Note Excel behaviour here. A blank final final value is converted to zero.
-			if (ae.isRow()) {
+			return NumberEval.ZERO;
-				if(ae.isColumn()) {
+			// Formulas _never_ evaluate to blank.  If a formula appears to have evaluated to
-					return ae.getRelativeValue(0, 0);
+			// blank, the actual value is empty string. This can be verified with ISBLANK().
 		}
-				return ae.getAbsoluteValue(ae.getFirstRow(), srcColNum);
+		return value;
-			}
-			if (ae.isColumn()) {
-				return ae.getAbsoluteValue(srcRowNum, ae.getFirstColumn());
-			}
-			return ErrorEval.VALUE_INVALID;
-		}
-		return evaluationResult;
 	}
 
 

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}.
@@ -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());
+	}
 }