follow-up to Bug 62904. More tests and improved evaluation of IF in array mode

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

7 files changed, 198 insertions, 54 deletions

diff --git a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
index e91c0e1095..bab3e4e66e 100644
--- a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
+++ b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
@@ -398,6 +398,9 @@ public final class WorkbookEvaluator {
             dbgEvaluationOutputIndent++;
         }
 
+        EvaluationSheet evalSheet = ec.getWorkbook().getSheet(ec.getSheetIndex());
+        EvaluationCell evalCell = evalSheet.getCell(ec.getRowIndex(), ec.getColumnIndex());
+
         Stack<ValueEval> stack = new Stack<>();
         for (int i = 0, iSize = ptgs.length; i < iSize; i++) {
             // since we don't know how to handle these yet :(
@@ -436,46 +439,41 @@ public final class WorkbookEvaluator {
                     continue;
                 }
                 if (attrPtg.isOptimizedIf()) {
-                    ValueEval arg0 = stack.pop();
-                    boolean evaluatedPredicate;
-                    try {
-                        evaluatedPredicate = IfFunc.evaluateFirstArg(arg0, ec.getRowIndex(), ec.getColumnIndex());
-                    } catch (EvaluationException e) {
-                        stack.push(e.getErrorEval());
-                        int dist = attrPtg.getData();
-                        i+= countTokensToBeSkipped(ptgs, i, dist);
-                        attrPtg = (AttrPtg) ptgs[i];
-                        dist = attrPtg.getData()+1;
-                        i+= countTokensToBeSkipped(ptgs, i, dist);
-                        continue;
-                    }
-                    if (evaluatedPredicate) {
-                        // nothing to skip - true param follows
-                    } else {
-                        int dist = attrPtg.getData();
-                        Ptg currPtg = ptgs[i+1];
-                        i+= countTokensToBeSkipped(ptgs, i, dist);
-                        Ptg nextPtg = ptgs[i+1];
-
-                        if (ptgs[i] instanceof AttrPtg && nextPtg instanceof FuncVarPtg &&
-                                // in order to verify that there is no third param, we need to check
-                                // if we really have the IF next or some other FuncVarPtg as third param, e.g. ROW()/COLUMN()!
-                                ((FuncVarPtg)nextPtg).getFunctionIndex() == FunctionMetadataRegistry.FUNCTION_INDEX_IF) {
-                            // this is an if statement without a false param (as opposed to MissingArgPtg as the false param)
-                            //i++;
-                            stack.push(arg0);
-                            if(currPtg instanceof AreaPtg){
-                                // IF in array mode. See Bug 62904
-                                ValueEval currEval = getEvalForPtg(currPtg, ec);
-                                stack.push(currEval);
-                            } else {
+                    if(!evalCell.isPartOfArrayFormulaGroup()) {
+                        ValueEval arg0 = stack.pop();
+                        boolean evaluatedPredicate;
+
+                        try {
+                            evaluatedPredicate = IfFunc.evaluateFirstArg(arg0, ec.getRowIndex(), ec.getColumnIndex());
+                        } catch (EvaluationException e) {
+                            stack.push(e.getErrorEval());
+                            int dist = attrPtg.getData();
+                            i += countTokensToBeSkipped(ptgs, i, dist);
+                            attrPtg = (AttrPtg) ptgs[i];
+                            dist = attrPtg.getData() + 1;
+                            i += countTokensToBeSkipped(ptgs, i, dist);
+                            continue;
+                        }
+                        if (evaluatedPredicate) {
+                            // nothing to skip - true param follows
+                        } else {
+                            int dist = attrPtg.getData();
+                            i += countTokensToBeSkipped(ptgs, i, dist);
+                            Ptg nextPtg = ptgs[i + 1];
+                            if (ptgs[i] instanceof AttrPtg && nextPtg instanceof FuncVarPtg &&
+                                    // in order to verify that there is no third param, we need to check
+                                    // if we really have the IF next or some other FuncVarPtg as third param, e.g. ROW()/COLUMN()!
+                                    ((FuncVarPtg) nextPtg).getFunctionIndex() == FunctionMetadataRegistry.FUNCTION_INDEX_IF) {
+                                // this is an if statement without a false param (as opposed to MissingArgPtg as the false param)
+                                //i++;
+                                stack.push(arg0);
                                 stack.push(BoolEval.FALSE);
                             }
                         }
                     }
                     continue;
                 }
-                if (attrPtg.isSkip()) {
+                if (attrPtg.isSkip() && !evalCell.isPartOfArrayFormulaGroup()) {
                     int dist = attrPtg.getData()+1;
                     i+= countTokensToBeSkipped(ptgs, i, dist);
                     if (stack.peek() == MissingArgEval.instance) {
diff --git a/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java b/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java
index ac1ee34118..8107c48544 100644
--- a/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java
+++ b/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java
@@ -48,7 +48,10 @@ public final class UnaryMinusEval extends Fixed1ArgFunction  implements ArrayFun
 
 	@Override
 	public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex){
-		return evaluateOneArrayArg(args, srcRowIndex, srcColumnIndex, (valA) ->
+		if (args.length != 1) {
+			return ErrorEval.VALUE_INVALID;
+		}
+		return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, (valA) ->
 				evaluate(srcRowIndex, srcColumnIndex, valA)
 		);
 	}
diff --git a/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java b/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java
index d5cb586a85..81b36aad40 100644
--- a/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java
+++ b/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java
@@ -52,7 +52,10 @@ public final class UnaryPlusEval extends Fixed1ArgFunction  implements ArrayFunc
 
 	@Override
 	public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex){
-		return evaluateOneArrayArg(args, srcRowIndex, srcColumnIndex, (valA) ->
+		if (args.length != 1) {
+			return ErrorEval.VALUE_INVALID;
+		}
+		return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, (valA) ->
 				evaluate(srcRowIndex, srcColumnIndex, valA)
 		);
 	}
diff --git a/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java b/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java
index 088d7b34fd..7f8a359d3a 100644
--- a/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java
+++ b/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java
@@ -111,6 +111,12 @@ public interface ArrayFunction {
                     vA = ErrorEval.NAME_INVALID;
                 } catch (EvaluationException e) {
                     vA = e.getErrorEval();
+                } catch (RuntimeException e) {
+                    if(e.getMessage().startsWith("Don't now how to evaluate name")){
+                        vA = ErrorEval.NAME_INVALID;
+                    } else {
+                        throw e;
+                    }
                 }
                 ValueEval vB;
                 try {
@@ -119,6 +125,12 @@ public interface ArrayFunction {
                     vB = ErrorEval.NAME_INVALID;
                 } catch (EvaluationException e) {
                     vB = e.getErrorEval();
+                } catch (RuntimeException e) {
+                    if(e.getMessage().startsWith("Don't now how to evaluate name")){
+                        vB = ErrorEval.NAME_INVALID;
+                    } else {
+                        throw e;
+                    }
                 }
                 if(vA instanceof ErrorEval){
                     vals[idx++] = vA;
@@ -138,10 +150,8 @@ public interface ArrayFunction {
         return new CacheAreaEval(srcRowIndex, srcColumnIndex, srcRowIndex + height - 1, srcColumnIndex + width - 1, vals);
     }
 
-    default ValueEval evaluateOneArrayArg(ValueEval[] args, int srcRowIndex, int srcColumnIndex,
+    default ValueEval evaluateOneArrayArg(ValueEval arg0, int srcRowIndex, int srcColumnIndex,
                                           java.util.function.Function<ValueEval, ValueEval> evalFunc){
-        ValueEval arg0 = args[0];
-
         int w1, w2, h1, h2;
         int a1FirstCol = 0, a1FirstRow = 0;
         if (arg0 instanceof AreaEval) {
@@ -178,6 +188,12 @@ public interface ArrayFunction {
                     vA = ErrorEval.NAME_INVALID;
                 } catch (EvaluationException e) {
                     vA = e.getErrorEval();
+                } catch (RuntimeException e) {
+                    if(e.getMessage().startsWith("Don't now how to evaluate name")){
+                        vA = ErrorEval.NAME_INVALID;
+                    } else {
+                        throw e;
+                    }
                 }
                 vals[idx++] = evalFunc.apply(vA);
             }
diff --git a/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java b/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java
index 5d011e3b20..5b5b539fef 100644
--- a/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java
+++ b/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java
@@ -37,7 +37,7 @@ import org.apache.poi.ss.formula.eval.ValueEval;
  *
  * @author Amol S. Deshmukh &lt; amolweb at ya hoo dot com &gt;
  */
-public abstract class BooleanFunction implements Function {
+public abstract class BooleanFunction implements Function,ArrayFunction {
 
 	public final ValueEval evaluate(ValueEval[] args, int srcRow, int srcCol) {
 		if (args.length < 1) {
@@ -142,7 +142,20 @@ public abstract class BooleanFunction implements Function {
 			return BoolEval.TRUE;
 		}
 	};
-	public static final Function NOT = new Fixed1ArgFunction() {
+
+	abstract static class Boolean1ArgFunction extends Fixed1ArgFunction implements ArrayFunction {
+		@Override
+		public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex) {
+			if (args.length != 1) {
+				return ErrorEval.VALUE_INVALID;
+			}
+			return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex,
+					vA -> evaluate(srcRowIndex, srcColumnIndex, vA));
+		}
+
+	}
+
+	public static final Function NOT = new Boolean1ArgFunction() {
 		public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0) {
 			boolean boolArgVal;
 			try {
@@ -156,4 +169,13 @@ public abstract class BooleanFunction implements Function {
 			return BoolEval.valueOf(!boolArgVal);
 		}
 	};
-}
+
+	@Override
+	public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex) {
+		if (args.length != 1) {
+			return ErrorEval.VALUE_INVALID;
+		}
+		return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex,
+				vA -> evaluate(new ValueEval[]{vA}, srcRowIndex, srcColumnIndex));
+	}
+}
\ No newline at end of file
diff --git a/src/java/org/apache/poi/ss/formula/functions/IfFunc.java b/src/java/org/apache/poi/ss/formula/functions/IfFunc.java
index effff27815..7f92881353 100644
--- a/src/java/org/apache/poi/ss/formula/functions/IfFunc.java
+++ b/src/java/org/apache/poi/ss/formula/functions/IfFunc.java
@@ -17,10 +17,14 @@
 
 package org.apache.poi.ss.formula.functions;
 
+import org.apache.poi.ss.formula.CacheAreaEval;
+import org.apache.poi.ss.formula.FormulaParseException;
 import org.apache.poi.ss.formula.eval.*;
 import org.apache.poi.ss.formula.ptg.Ptg;
 import org.apache.poi.ss.formula.ptg.RefPtg;
 
+import java.util.function.BiFunction;
+
 /**
  * Implementation for the Excel function IF
  * <p>
@@ -84,25 +88,120 @@ public final class IfFunc extends Var2or3ArgFunction implements ArrayFunction {
 
  	@Override
 	public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex) {
+		if (args.length < 2 || args.length > 3) {
+			return ErrorEval.VALUE_INVALID;
+		}
+
 		ValueEval arg0 = args[0];
 		ValueEval arg1 = args[1];
-		return evaluateTwoArrayArgs(arg0, arg1, srcRowIndex, srcColumnIndex,
-                (vA, vB) -> {
+		ValueEval arg2 = args.length == 2 ? BoolEval.FALSE : args[2];
+		return evaluateArrayArgs(arg0, arg1, arg2, srcRowIndex, srcColumnIndex);
+	}
+
+	ValueEval evaluateArrayArgs(ValueEval arg0, ValueEval arg1, ValueEval arg2, int srcRowIndex, int srcColumnIndex) {
+		int w1, w2, h1, h2;
+		int a1FirstCol = 0, a1FirstRow = 0;
+		if (arg0 instanceof AreaEval) {
+			AreaEval ae = (AreaEval)arg0;
+			w1 = ae.getWidth();
+			h1 = ae.getHeight();
+			a1FirstCol = ae.getFirstColumn();
+			a1FirstRow = ae.getFirstRow();
+		} else if (arg0 instanceof RefEval){
+			RefEval ref = (RefEval)arg0;
+			w1 = 1;
+			h1 = 1;
+			a1FirstCol = ref.getColumn();
+			a1FirstRow = ref.getRow();
+		} else {
+			w1 = 1;
+			h1 = 1;
+		}
+		int a2FirstCol = 0, a2FirstRow = 0;
+		if (arg1 instanceof AreaEval) {
+			AreaEval ae = (AreaEval)arg1;
+			w2 = ae.getWidth();
+			h2 = ae.getHeight();
+			a2FirstCol = ae.getFirstColumn();
+			a2FirstRow = ae.getFirstRow();
+		} else if (arg1 instanceof RefEval){
+			RefEval ref = (RefEval)arg1;
+			w2 = 1;
+			h2 = 1;
+			a2FirstCol = ref.getColumn();
+			a2FirstRow = ref.getRow();
+		} else {
+			w2 = 1;
+			h2 = 1;
+		}
+
+		int a3FirstCol = 0, a3FirstRow = 0;
+		if (arg2 instanceof AreaEval) {
+			AreaEval ae = (AreaEval)arg2;
+			a3FirstCol = ae.getFirstColumn();
+			a3FirstRow = ae.getFirstRow();
+		} else if (arg2 instanceof RefEval){
+			RefEval ref = (RefEval)arg2;
+			a3FirstCol = ref.getColumn();
+			a3FirstRow = ref.getRow();
+		}
+
+		int width = Math.max(w1, w2);
+		int height = Math.max(h1, h2);
+
+		ValueEval[] vals = new ValueEval[height * width];
+
+		int idx = 0;
+		for(int i = 0; i < height; i++){
+			for(int j = 0; j < width; j++){
+				ValueEval vA;
+				try {
+					vA = OperandResolver.getSingleValue(arg0, a1FirstRow + i, a1FirstCol + j);
+				} catch (FormulaParseException e) {
+					vA = ErrorEval.NAME_INVALID;
+				} catch (EvaluationException e) {
+					vA = e.getErrorEval();
+				}
+				ValueEval vB;
+				try {
+					vB = OperandResolver.getSingleValue(arg1, a2FirstRow + i, a2FirstCol + j);
+				} catch (FormulaParseException e) {
+					vB = ErrorEval.NAME_INVALID;
+				} catch (EvaluationException e) {
+					vB = e.getErrorEval();
+				}
+
+				ValueEval vC;
+				try {
+					vC = OperandResolver.getSingleValue(arg2, a3FirstRow + i, a3FirstCol + j);
+				} catch (FormulaParseException e) {
+					vC = ErrorEval.NAME_INVALID;
+				} catch (EvaluationException e) {
+					vC = e.getErrorEval();
+				}
+
+				if(vA instanceof ErrorEval){
+					vals[idx++] = vA;
+				} else if (vB instanceof ErrorEval) {
+					vals[idx++] = vB;
+				} else {
 					Boolean b;
 					try {
 						b = OperandResolver.coerceValueToBoolean(vA, false);
+						vals[idx++] = b != null && b ? vB : vC;
 					} catch (EvaluationException e) {
-						return e.getErrorEval();
-					}
-					if (b != null && b) {
-						if (vB == MissingArgEval.instance) {
-							return BlankEval.instance;
-						}
-						return vB;
+						vals[idx++] = e.getErrorEval();
 					}
-					return BoolEval.FALSE;
 				}
-        );
+
+			}
+		}
+
+		if (vals.length == 1) {
+			return vals[0];
+		}
+
+		return new CacheAreaEval(srcRowIndex, srcColumnIndex, srcRowIndex + height - 1, srcColumnIndex + width - 1, vals);
 	}
 
 }
diff --git a/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java b/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java
index 5cb7d25505..9df2803c3c 100644
--- a/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java
+++ b/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java
@@ -44,7 +44,10 @@ public abstract class LogicalFunction extends Fixed1ArgFunction implements Array
 
 	@Override
 	public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex){
-		return evaluateOneArrayArg(args, srcRowIndex, srcColumnIndex, (valA) ->
+		if (args.length != 1) {
+			return ErrorEval.VALUE_INVALID;
+		}
+		return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, (valA) ->
 				BoolEval.valueOf(evaluate(valA))
 		);
 	}