diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 4fb704e5d..95cab0e78 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + Fixed special cases of INDEX function (single column/single row, errors) 45761 - Support for Very Hidden excel sheets in HSSF 45738 - Initial HWPF support for Office Art Shapes 45720 - Fixed HSSFWorkbook.cloneSheet to correctly clone sheets with drawings diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 118f81ba7..e7fb447da 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + Fixed special cases of INDEX function (single column/single row, errors) 45761 - Support for Very Hidden excel sheets in HSSF 45738 - Initial HWPF support for Office Art Shapes 45720 - Fixed HSSFWorkbook.cloneSheet to correctly clone sheets with drawings 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 3c93c0846..1c149dbdf 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 @@ -22,6 +22,7 @@ import org.apache.poi.hssf.record.formula.eval.ErrorEval; import org.apache.poi.hssf.record.formula.eval.Eval; import org.apache.poi.hssf.record.formula.eval.EvaluationException; 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.ValueEval; /** @@ -51,6 +52,10 @@ public final class Index implements Function { return ErrorEval.VALUE_INVALID; } Eval firstArg = args[0]; + if (firstArg instanceof RefEval) { + // convert to area ref for simpler code in getValueFromArea() + firstArg = ((RefEval)firstArg).offset(0, 0, 0, 0); + } if(!(firstArg instanceof AreaEval)) { // else the other variation of this function takes an array as the first argument @@ -84,16 +89,63 @@ public final class Index implements Function { // too many arguments return ErrorEval.VALUE_INVALID; } - return getValueFromArea(reference, rowIx, columnIx); + return getValueFromArea(reference, rowIx, columnIx, nArgs); } catch (EvaluationException e) { return e.getErrorEval(); } } - private static ValueEval getValueFromArea(AreaEval ae, int rowIx, int columnIx) throws EvaluationException { + /** + * @param nArgs - needed because error codes are slightly different when only 2 args are passed + */ + private static ValueEval getValueFromArea(AreaEval ae, int pRowIx, int pColumnIx, int nArgs) throws EvaluationException { + 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()) { + rowIx = pRowIx == -1 ? 0 : pRowIx; + columnIx = pColumnIx == -1 ? 0 : pColumnIx; + } else { + if (nArgs == 2) { + rowIx = 0; + columnIx = pRowIx; + } else { + rowIx = pRowIx == -1 ? 0 : pRowIx; + columnIx = pColumnIx; + } + } + if (rowIx < -1 || columnIx < -1) { + throw new EvaluationException(ErrorEval.VALUE_INVALID); + } + } else if (ae.isColumn()) { + if (nArgs == 2) { + rowIx = pRowIx; + columnIx = 0; + } else { + rowIx = pRowIx; + columnIx = pColumnIx == -1 ? 0 : pColumnIx; + } + if (rowIx < -1 || columnIx < -1) { + throw new EvaluationException(ErrorEval.VALUE_INVALID); + } + } else { + if (nArgs == 2) { + // always an error with 2-D area refs + if (pRowIx < -1) { + throw new EvaluationException(ErrorEval.VALUE_INVALID); + } + throw new EvaluationException(ErrorEval.REF_INVALID); + } + // Normal case - area ref is 2-D, and both index args were provided + rowIx = pRowIx; + columnIx = pColumnIx; + } + int width = ae.getWidth(); int height = ae.getHeight(); - // Slightly irregular logic for bounds checking errors if (rowIx >= height || columnIx >= width) { throw new EvaluationException(ErrorEval.REF_INVALID); diff --git a/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls b/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls new file mode 100644 index 000000000..9dcde6177 Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls differ diff --git a/src/testcases/org/apache/poi/hssf/record/formula/functions/AllIndividualFunctionEvaluationTests.java b/src/testcases/org/apache/poi/hssf/record/formula/functions/AllIndividualFunctionEvaluationTests.java index 2ec7ad005..4ae90d067 100755 --- a/src/testcases/org/apache/poi/hssf/record/formula/functions/AllIndividualFunctionEvaluationTests.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/functions/AllIndividualFunctionEvaluationTests.java @@ -34,6 +34,7 @@ public final class AllIndividualFunctionEvaluationTests { result.addTestSuite(TestDate.class); result.addTestSuite(TestFinanceLib.class); result.addTestSuite(TestIndex.class); + result.addTestSuite(TestIndexFunctionFromSpreadsheet.class); result.addTestSuite(TestIsBlank.class); result.addTestSuite(TestLen.class); result.addTestSuite(TestLookupFunctionsFromSpreadsheet.class); diff --git a/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java index 80c154596..95355733d 100755 --- a/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java @@ -48,11 +48,11 @@ public final class TestIndex extends TestCase { double[] values = TEST_VALUES0; confirmAreaEval("C1:D6", values, 4, 1, 7); confirmAreaEval("C1:D6", values, 6, 2, 12); - confirmAreaEval("C1:D6", values, 3, -1, 5); + confirmAreaEval("C1:D6", values, 3, 1, 5); // now treat same data as 3 columns, 4 rows confirmAreaEval("C10:E13", values, 2, 2, 5); - confirmAreaEval("C10:E13", values, 4, -1, 10); + confirmAreaEval("C10:E13", values, 4, 1, 10); } /** diff --git a/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndexFunctionFromSpreadsheet.java b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndexFunctionFromSpreadsheet.java new file mode 100644 index 000000000..b1c5247ff --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndexFunctionFromSpreadsheet.java @@ -0,0 +1,259 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.hssf.record.formula.functions; + +import java.io.PrintStream; + +import junit.framework.Assert; +import junit.framework.AssertionFailedError; +import junit.framework.TestCase; + +import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.record.formula.eval.ErrorEval; +import org.apache.poi.hssf.usermodel.HSSFCell; +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.hssf.usermodel.HSSFFormulaEvaluator.CellValue; +import org.apache.poi.hssf.util.CellReference; + +/** + * Tests INDEX() as loaded from a test data spreadsheet.

+ * + * @author Josh Micich + */ +public final class TestIndexFunctionFromSpreadsheet extends TestCase { + + private static final class Result { + public static final int SOME_EVALUATIONS_FAILED = -1; + public static final int ALL_EVALUATIONS_SUCCEEDED = +1; + public static final int NO_EVALUATIONS_FOUND = 0; + } + + /** + * This class defines constants for navigating around the test data spreadsheet used for these tests. + */ + private static final class SS { + + /** Name of the test spreadsheet (found in the standard test data folder) */ + public final static String FILENAME = "IndexFunctionTestCaseData.xls"; + + public static final int COLUMN_INDEX_EVALUATION = 2; // Column 'C' + public static final int COLUMN_INDEX_EXPECTED_RESULT = 3; // Column 'D' + + } + + // Note - multiple failures are aggregated before ending. + // If one or more functions fail, a single AssertionFailedError is thrown at the end + private int _evaluationFailureCount; + private int _evaluationSuccessCount; + + + + private static void confirmExpectedResult(String msg, HSSFCell expected, HSSFFormulaEvaluator.CellValue actual) { + if (expected == null) { + throw new AssertionFailedError(msg + " - Bad setup data expected value is null"); + } + if(actual == null) { + throw new AssertionFailedError(msg + " - actual value was null"); + } + if(expected.getCellType() == HSSFCell.CELL_TYPE_ERROR) { + confirmErrorResult(msg, expected.getErrorCellValue(), actual); + return; + } + if(actual.getCellType() == HSSFCell.CELL_TYPE_ERROR) { + throw unexpectedError(msg, expected, actual.getErrorValue()); + } + if(actual.getCellType() != expected.getCellType()) { + throw wrongTypeError(msg, expected, actual); + } + + + switch (expected.getCellType()) { + case HSSFCell.CELL_TYPE_BOOLEAN: + assertEquals(msg, expected.getBooleanCellValue(), actual.getBooleanValue()); + break; + case HSSFCell.CELL_TYPE_FORMULA: // will never be used, since we will call method after formula evaluation + throw new AssertionFailedError("Cannot expect formula as result of formula evaluation: " + msg); + case HSSFCell.CELL_TYPE_NUMERIC: + assertEquals(expected.getNumericCellValue(), actual.getNumberValue(), 0.0); + break; + case HSSFCell.CELL_TYPE_STRING: + assertEquals(msg, expected.getRichStringCellValue().getString(), actual.getRichTextStringValue().getString()); + break; + } + } + + + private static AssertionFailedError wrongTypeError(String msgPrefix, HSSFCell expectedCell, CellValue actualValue) { + return new AssertionFailedError(msgPrefix + " Result type mismatch. Evaluated result was " + + formatValue(actualValue) + + " but the expected result was " + + formatValue(expectedCell) + ); + } + private static AssertionFailedError unexpectedError(String msgPrefix, HSSFCell expected, int actualErrorCode) { + return new AssertionFailedError(msgPrefix + " Error code (" + + ErrorEval.getText(actualErrorCode) + + ") was evaluated, but the expected result was " + + formatValue(expected) + ); + } + + + private static void confirmErrorResult(String msgPrefix, int expectedErrorCode, CellValue actual) { + if(actual.getCellType() != HSSFCell.CELL_TYPE_ERROR) { + throw new AssertionFailedError(msgPrefix + " Expected cell error (" + + ErrorEval.getText(expectedErrorCode) + ") but actual value was " + + formatValue(actual)); + } + if(expectedErrorCode != actual.getErrorValue()) { + throw new AssertionFailedError(msgPrefix + " Expected cell error code (" + + ErrorEval.getText(expectedErrorCode) + + ") but actual error code was (" + + ErrorEval.getText(actual.getErrorValue()) + + ")"); + } + } + + + private static String formatValue(HSSFCell expecedCell) { + switch (expecedCell.getCellType()) { + case HSSFCell.CELL_TYPE_BLANK: return ""; + case HSSFCell.CELL_TYPE_BOOLEAN: return String.valueOf(expecedCell.getBooleanCellValue()); + case HSSFCell.CELL_TYPE_NUMERIC: return String.valueOf(expecedCell.getNumericCellValue()); + case HSSFCell.CELL_TYPE_STRING: return expecedCell.getRichStringCellValue().getString(); + } + throw new RuntimeException("Unexpected cell type of expected value (" + expecedCell.getCellType() + ")"); + } + private static String formatValue(CellValue actual) { + switch (actual.getCellType()) { + case HSSFCell.CELL_TYPE_BLANK: return ""; + case HSSFCell.CELL_TYPE_BOOLEAN: return String.valueOf(actual.getBooleanValue()); + case HSSFCell.CELL_TYPE_NUMERIC: return String.valueOf(actual.getNumberValue()); + case HSSFCell.CELL_TYPE_STRING: return actual.getRichTextStringValue().getString(); + } + throw new RuntimeException("Unexpected cell type of evaluated value (" + actual.getCellType() + ")"); + } + + + protected void setUp() { + _evaluationFailureCount = 0; + _evaluationSuccessCount = 0; + } + + public void testFunctionsFromTestSpreadsheet() { + HSSFWorkbook workbook = HSSFTestDataSamples.openSampleWorkbook(SS.FILENAME); + + processTestSheet(workbook, workbook.getSheetName(0)); + + // confirm results + String successMsg = "There were " + + _evaluationSuccessCount + " function(s) without error"; + if(_evaluationFailureCount > 0) { + String msg = _evaluationFailureCount + " evaluation(s) failed. " + successMsg; + throw new AssertionFailedError(msg); + } + if(false) { // normally no output for successful tests + System.out.println(getClass().getName() + ": " + successMsg); + } + } + + private void processTestSheet(HSSFWorkbook workbook, String sheetName) { + HSSFSheet sheet = workbook.getSheetAt(0); + HSSFFormulaEvaluator evaluator = new HSSFFormulaEvaluator(sheet, workbook); + int maxRows = sheet.getLastRowNum()+1; + int result = Result.NO_EVALUATIONS_FOUND; // so far + + for(int rowIndex=0; rowIndex= endIx) { + // something went wrong. just print the whole stack trace + e.printStackTrace(ps); + } + endIx -= 4; // skip 4 frames of reflection invocation + ps.println(e.toString()); + for(int i=startIx; i