diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index deb2890c7..cf143a31f 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 50096 - Fixed evaluation of cell references with column index greater than 255 49761 - Tolerate Double.NaN when reading .xls files 50211 - Use cached formula result when auto-sizing formula cells 50118 - OLE2 does allow a directory with an empty name, so support this in POIFS diff --git a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java index 0b0625855..9e5b9c8b3 100644 --- a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java +++ b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java @@ -20,6 +20,7 @@ package org.apache.poi.hssf.record; import org.apache.poi.hssf.record.formula.*; import org.apache.poi.hssf.util.CellRangeAddress8Bit; import org.apache.poi.ss.formula.Formula; +import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.util.HexDump; import org.apache.poi.util.LittleEndianOutput; @@ -96,51 +97,6 @@ public final class SharedFormulaRecord extends SharedValueRecordBase { return sid; } - /** - * Creates a non shared formula from the shared formula counterpart
- * - * Perhaps this functionality could be implemented in terms of the raw - * byte array inside {@link Formula}. - */ - public static Ptg[] convertSharedFormulas(Ptg[] ptgs, int formulaRow, int formulaColumn) { - - Ptg[] newPtgStack = new Ptg[ptgs.length]; - - for (int k = 0; k < ptgs.length; k++) { - Ptg ptg = ptgs[k]; - byte originalOperandClass = -1; - if (!ptg.isBaseToken()) { - originalOperandClass = ptg.getPtgClass(); - } - if (ptg instanceof RefPtgBase) { - RefPtgBase refNPtg = (RefPtgBase)ptg; - ptg = new RefPtg(fixupRelativeRow(formulaRow,refNPtg.getRow(),refNPtg.isRowRelative()), - fixupRelativeColumn(formulaColumn,refNPtg.getColumn(),refNPtg.isColRelative()), - refNPtg.isRowRelative(), - refNPtg.isColRelative()); - ptg.setClass(originalOperandClass); - } else if (ptg instanceof AreaPtgBase) { - AreaPtgBase areaNPtg = (AreaPtgBase)ptg; - ptg = new AreaPtg(fixupRelativeRow(formulaRow,areaNPtg.getFirstRow(),areaNPtg.isFirstRowRelative()), - fixupRelativeRow(formulaRow,areaNPtg.getLastRow(),areaNPtg.isLastRowRelative()), - fixupRelativeColumn(formulaColumn,areaNPtg.getFirstColumn(),areaNPtg.isFirstColRelative()), - fixupRelativeColumn(formulaColumn,areaNPtg.getLastColumn(),areaNPtg.isLastColRelative()), - areaNPtg.isFirstRowRelative(), - areaNPtg.isLastRowRelative(), - areaNPtg.isFirstColRelative(), - areaNPtg.isLastColRelative()); - ptg.setClass(originalOperandClass); - } else if (ptg instanceof OperandPtg) { - // Any subclass of OperandPtg is mutable, so it's safest to not share these instances. - ptg = ((OperandPtg) ptg).copy(); - } else { - // all other Ptgs are immutable and can be shared - } - newPtgStack[k] = ptg; - } - return newPtgStack; - } - /** * @return the equivalent {@link Ptg} array that the formula would have, were it not shared. */ @@ -152,23 +108,8 @@ public final class SharedFormulaRecord extends SharedValueRecordBase { throw new RuntimeException("Shared Formula Conversion: Coding Error"); } - return convertSharedFormulas(field_7_parsed_expr.getTokens(), formulaRow, formulaColumn); - } - - private static int fixupRelativeColumn(int currentcolumn, int column, boolean relative) { - if(relative) { - // mask out upper bits to produce 'wrapping' at column 256 ("IV") - return (column + currentcolumn) & 0x00FF; - } - return column; - } - - private static int fixupRelativeRow(int currentrow, int row, boolean relative) { - if(relative) { - // mask out upper bits to produce 'wrapping' at row 65536 - return (row+currentrow) & 0x00FFFF; - } - return row; + SharedFormula sf = new SharedFormula(SpreadsheetVersion.EXCEL97); + return sf.convertSharedFormulas(field_7_parsed_expr.getTokens(), formulaRow, formulaColumn); } public Object clone() { diff --git a/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java b/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java index 70ed4da87..23c8f996f 100644 --- a/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java +++ b/src/java/org/apache/poi/hssf/record/formula/RefPtgBase.java @@ -40,7 +40,14 @@ public abstract class RefPtgBase extends OperandPtg { private int field_2_col; private static final BitField rowRelative = BitFieldFactory.getInstance(0x8000); private static final BitField colRelative = BitFieldFactory.getInstance(0x4000); - private static final BitField column = BitFieldFactory.getInstance(0x00FF); + + /** + * YK: subclasses of RefPtgBase are used by the FormulaParser and FormulaEvaluator accross HSSF and XSSF. + * The bit mask should accomodate the maximum number of avaiable columns, i.e. 0x3FFF. + * + * @see org.apache.poi.ss.SpreadsheetVersion + */ + private static final BitField column = BitFieldFactory.getInstance(0x3FFF); protected RefPtgBase() { // Required for clone methods diff --git a/src/java/org/apache/poi/hssf/record/formula/SharedFormula.java b/src/java/org/apache/poi/hssf/record/formula/SharedFormula.java new file mode 100644 index 000000000..db55a7015 --- /dev/null +++ b/src/java/org/apache/poi/hssf/record/formula/SharedFormula.java @@ -0,0 +1,97 @@ +/* ==================================================================== + 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; + +import org.apache.poi.ss.SpreadsheetVersion; + +/** + * Encapsulates logic to convert shared formulaa into non shared equivalent + */ +public class SharedFormula { + + private final int _columnWrappingMask; + private final int _rowWrappingMask; + + public SharedFormula(SpreadsheetVersion ssVersion){ + _columnWrappingMask = ssVersion.getLastColumnIndex(); //"IV" for .xls and "XFD" for .xlsx + _rowWrappingMask = ssVersion.getLastRowIndex(); + } + + /** + * Creates a non shared formula from the shared formula counterpart, i.e. + * Converts the shared formula into the equivalent {@link Ptg} array that it would have, + * were it not shared. + * + * @param ptgs parsed tokens of the shared formula + * @param formulaRow + * @param formulaColumn + */ + public Ptg[] convertSharedFormulas(Ptg[] ptgs, int formulaRow, int formulaColumn) { + + Ptg[] newPtgStack = new Ptg[ptgs.length]; + + for (int k = 0; k < ptgs.length; k++) { + Ptg ptg = ptgs[k]; + byte originalOperandClass = -1; + if (!ptg.isBaseToken()) { + originalOperandClass = ptg.getPtgClass(); + } + if (ptg instanceof RefPtgBase) { + RefPtgBase refNPtg = (RefPtgBase)ptg; + ptg = new RefPtg(fixupRelativeRow(formulaRow,refNPtg.getRow(),refNPtg.isRowRelative()), + fixupRelativeColumn(formulaColumn,refNPtg.getColumn(),refNPtg.isColRelative()), + refNPtg.isRowRelative(), + refNPtg.isColRelative()); + ptg.setClass(originalOperandClass); + } else if (ptg instanceof AreaPtgBase) { + AreaPtgBase areaNPtg = (AreaPtgBase)ptg; + ptg = new AreaPtg(fixupRelativeRow(formulaRow,areaNPtg.getFirstRow(),areaNPtg.isFirstRowRelative()), + fixupRelativeRow(formulaRow,areaNPtg.getLastRow(),areaNPtg.isLastRowRelative()), + fixupRelativeColumn(formulaColumn,areaNPtg.getFirstColumn(),areaNPtg.isFirstColRelative()), + fixupRelativeColumn(formulaColumn,areaNPtg.getLastColumn(),areaNPtg.isLastColRelative()), + areaNPtg.isFirstRowRelative(), + areaNPtg.isLastRowRelative(), + areaNPtg.isFirstColRelative(), + areaNPtg.isLastColRelative()); + ptg.setClass(originalOperandClass); + } else if (ptg instanceof OperandPtg) { + // Any subclass of OperandPtg is mutable, so it's safest to not share these instances. + ptg = ((OperandPtg) ptg).copy(); + } else { + // all other Ptgs are immutable and can be shared + } + newPtgStack[k] = ptg; + } + return newPtgStack; + } + + private int fixupRelativeColumn(int currentcolumn, int column, boolean relative) { + if(relative) { + // mask out upper bits to produce 'wrapping' at the maximum column ("IV" for .xls and "XFD" for .xlsx) + return (column + currentcolumn) & _columnWrappingMask; + } + return column; + } + + private int fixupRelativeRow(int currentrow, int row, boolean relative) { + if(relative) { + return (row+currentrow) & _rowWrappingMask; + } + return row; + } + +} diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java index e0998bc59..3d9d9275e 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java @@ -24,6 +24,7 @@ import java.util.Date; import org.apache.poi.hssf.record.SharedFormulaRecord; import org.apache.poi.hssf.record.formula.Ptg; +import org.apache.poi.hssf.record.formula.SharedFormula; import org.apache.poi.hssf.record.formula.eval.ErrorEval; import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.ss.formula.FormulaParser; @@ -391,8 +392,10 @@ public final class XSSFCell implements Cell { int sheetIndex = sheet.getWorkbook().getSheetIndex(sheet); XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.create(sheet.getWorkbook()); + SharedFormula sf = new SharedFormula(SpreadsheetVersion.EXCEL2007); + Ptg[] ptgs = FormulaParser.parse(sharedFormula, fpb, FormulaType.CELL, sheetIndex); - Ptg[] fmla = SharedFormulaRecord.convertSharedFormulas(ptgs, + Ptg[] fmla = sf.convertSharedFormulas(ptgs, getRowIndex() - ref.getFirstRow(), getColumnIndex() - ref.getFirstColumn()); return FormulaRenderer.toFormulaString(fpb, fmla); } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java index 2c66b957e..a452e5f9c 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java @@ -20,6 +20,7 @@ package org.apache.poi.xssf.usermodel; import junit.framework.TestCase; import org.apache.poi.ss.usermodel.*; +import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.XSSFITestDataProvider; @@ -57,4 +58,35 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator { XSSFCell d3 = sheet.getRow(2).getCell(3); assertEquals(result, evaluator.evaluateInCell(d3).getNumericCellValue()); } + + /** + * Evaluation of cell references with column indexes greater than 255. See bugzilla 50096 + */ + public void testEvaluateColumnGreaterThan255() { + XSSFWorkbook wb = (XSSFWorkbook) _testDataProvider.openSampleWorkbook("50096.xlsx"); + XSSFFormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator(); + + /** + * The first row simply contains the numbers 1 - 300. + * The second row simply refers to the cell value above in the first row by a simple formula. + */ + for (int i = 245; i < 265; i++) { + XSSFCell cell_noformula = wb.getSheetAt(0).getRow(0).getCell(i); + XSSFCell cell_formula = wb.getSheetAt(0).getRow(1).getCell(i); + + CellReference ref_noformula = new CellReference(cell_noformula.getRowIndex(), cell_noformula.getColumnIndex()); + CellReference ref_formula = new CellReference(cell_noformula.getRowIndex(), cell_noformula.getColumnIndex()); + String fmla = cell_formula.getCellFormula(); + // assure that the formula refers to the cell above. + // the check below is 'deep' and involves conversion of the shared formula: + // in the sample file a shared formula in GN1 is spanned in the range GN2:IY2, + assertEquals(ref_noformula.formatAsString(), fmla); + + CellValue cv_noformula = evaluator.evaluate(cell_noformula); + CellValue cv_formula = evaluator.evaluate(cell_formula); + assertEquals("Wrong evaluation result in " + ref_formula.formatAsString(), + cv_noformula.getNumberValue(), cv_formula.getNumberValue()); + } + + } } diff --git a/src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java b/src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java index 10a64813c..1547f9456 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java @@ -24,11 +24,13 @@ import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.record.formula.Ptg; import org.apache.poi.hssf.record.formula.RefPtg; +import org.apache.poi.hssf.record.formula.SharedFormula; import org.apache.poi.hssf.usermodel.*; import org.apache.poi.ss.usermodel.CellValue; import org.apache.poi.ss.formula.FormulaParser; import org.apache.poi.ss.formula.FormulaRenderer; import org.apache.poi.ss.formula.FormulaType; +import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.util.LittleEndianInput; /** @@ -74,7 +76,8 @@ public final class TestSharedFormulaRecord extends TestCase { int encodedLen = in.readUShort(); Ptg[] sharedFormula = Ptg.readTokens(encodedLen, in); - Ptg[] convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 100, 200); + SharedFormula sf = new SharedFormula(SpreadsheetVersion.EXCEL97); + Ptg[] convertedFormula = sf.convertSharedFormulas(sharedFormula, 100, 200); RefPtg refPtg = (RefPtg) convertedFormula[1]; assertEquals("$C101", refPtg.toFormulaString()); @@ -102,32 +105,34 @@ public final class TestSharedFormulaRecord extends TestCase { HSSFEvaluationWorkbook fpb = HSSFEvaluationWorkbook.create(wb); Ptg[] sharedFormula, convertedFormula; + SharedFormula sf = new SharedFormula(SpreadsheetVersion.EXCEL97); + sharedFormula = FormulaParser.parse("A2", fpb, FormulaType.CELL, -1); - convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 0, 0); + convertedFormula = sf.convertSharedFormulas(sharedFormula, 0, 0); confirmOperandClasses(sharedFormula, convertedFormula); //conversion relative to [0,0] should return the original formula assertEquals("A2", FormulaRenderer.toFormulaString(fpb, convertedFormula)); - convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 1, 0); + convertedFormula = sf.convertSharedFormulas(sharedFormula, 1, 0); confirmOperandClasses(sharedFormula, convertedFormula); //one row down assertEquals("A3", FormulaRenderer.toFormulaString(fpb, convertedFormula)); - convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 1, 1); + convertedFormula = sf.convertSharedFormulas(sharedFormula, 1, 1); confirmOperandClasses(sharedFormula, convertedFormula); //one row down and one cell right assertEquals("B3", FormulaRenderer.toFormulaString(fpb, convertedFormula)); sharedFormula = FormulaParser.parse("SUM(A1:C1)", fpb, FormulaType.CELL, -1); - convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 0, 0); + convertedFormula = sf.convertSharedFormulas(sharedFormula, 0, 0); confirmOperandClasses(sharedFormula, convertedFormula); assertEquals("SUM(A1:C1)", FormulaRenderer.toFormulaString(fpb, convertedFormula)); - convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 1, 0); + convertedFormula = sf.convertSharedFormulas(sharedFormula, 1, 0); confirmOperandClasses(sharedFormula, convertedFormula); assertEquals("SUM(A2:C2)", FormulaRenderer.toFormulaString(fpb, convertedFormula)); - convertedFormula = SharedFormulaRecord.convertSharedFormulas(sharedFormula, 1, 1); + convertedFormula = sf.convertSharedFormulas(sharedFormula, 1, 1); confirmOperandClasses(sharedFormula, convertedFormula); assertEquals("SUM(B2:D2)", FormulaRenderer.toFormulaString(fpb, convertedFormula)); } diff --git a/src/testcases/org/apache/poi/hssf/record/formula/TestReferencePtg.java b/src/testcases/org/apache/poi/hssf/record/formula/TestReferencePtg.java index dc8fd6b9b..4eef47b3e 100644 --- a/src/testcases/org/apache/poi/hssf/record/formula/TestReferencePtg.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/TestReferencePtg.java @@ -103,5 +103,20 @@ public final class TestReferencePtg extends TestCase { } assertTrue(Arrays.equals(tRefN_data, outData)); } + + /** + * test that RefPtgBase can handle references with column index greater than 255, + * see Bugzilla 50096 + */ + public void testColumnGreater255() { + RefPtgBase ptg; + ptg = new RefPtg("IW1"); + assertEquals(256, ptg.getColumn()); + assertEquals("IW1", ptg.formatReferenceAsString()); + + ptg = new RefPtg("JA1"); + assertEquals(260, ptg.getColumn()); + assertEquals("JA1", ptg.formatReferenceAsString()); + } } diff --git a/test-data/spreadsheet/50096.xlsx b/test-data/spreadsheet/50096.xlsx new file mode 100644 index 000000000..b88784282 Binary files /dev/null and b/test-data/spreadsheet/50096.xlsx differ