Fixed evaluation of cell references with column index greater than 255, see bugzilla 50096

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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta1" date="2010-??-??">
+           <action dev="poi-developers" type="fix">50096 - Fixed evaluation of cell references with column index greater than 255 </action>
            <action dev="poi-developers" type="fix">49761 - Tolerate Double.NaN when reading .xls files</action>
            <action dev="poi-developers" type="fix">50211 - Use cached formula result when auto-sizing formula cells</action>
            <action dev="poi-developers" type="fix">50118 - OLE2 does allow a directory with an empty name, so support this in POIFS</action>

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<br/>
-     *
-     * 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() {

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

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

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

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

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

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