]> source.dussan.org Git - poi.git/commitdiff
Bug 44306 - fixed reading/writing of AttrPtg and rendering of CHOOSE formulas
authorJosh Micich <josh@apache.org>
Mon, 19 May 2008 01:06:54 +0000 (01:06 +0000)
committerJosh Micich <josh@apache.org>
Mon, 19 May 2008 01:06:54 +0000 (01:06 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@657702 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml
src/documentation/content/xdocs/status.xml
src/java/org/apache/poi/hssf/model/FormulaParser.java
src/java/org/apache/poi/hssf/record/formula/AttrPtg.java
src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java
src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java
src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java

index 65fe1e25c4919e1213c44660fee7f7002a0ec11b..f6f5f8a6208361a6fbc17d8e826127861832b4dc 100644 (file)
@@ -37,6 +37,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">44306 - fixed reading/writing of AttrPtg(type=choose) and method toFormulaString() for CHOOSE formulas</action>
            <action dev="POI-DEVELOPERS" type="fix">24207 - added HSSFName.isDeleted() to check if the name points to cell that no longer exists</action>
            <action dev="POI-DEVELOPERS" type="fix">40414 - fixed selected/active sheet after removing sheet from workbook</action>
            <action dev="POI-DEVELOPERS" type="fix">44523 - fixed workbook sheet selection and focus</action>
index 28019e68feef91e3fb78fe39b987620e2f3b3e86..2b5196f9624313df3b380291c073fdd5503da01f 100644 (file)
@@ -34,6 +34,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">44306 - fixed reading/writing of AttrPtg(type=choose) and method toFormulaString() for CHOOSE formulas</action>
            <action dev="POI-DEVELOPERS" type="fix">24207 - added HSSFName.isDeleted() to check if the name points to cell that no longer exists</action>
            <action dev="POI-DEVELOPERS" type="fix">40414 - fixed selected/active sheet after removing sheet from workbook</action>
            <action dev="POI-DEVELOPERS" type="fix">44523 - fixed workbook sheet selection and focus</action>
index f16ab451294805967c0f8d4c3691f69c00b8b3b2..2819786ef6826b141830deccf2c2ed6079c4231e 100644 (file)
@@ -1000,7 +1000,7 @@ end;
 
             if (ptg instanceof AttrPtg) {
                 AttrPtg attrPtg = ((AttrPtg) ptg);
-                if (attrPtg.isOptimizedIf()) {
+                if (attrPtg.isOptimizedIf() || attrPtg.isOptimizedChoose() || attrPtg.isGoto()) {
                     continue;
                 }
                 if (attrPtg.isSpace()) {
@@ -1014,6 +1014,9 @@ end;
                     // similar to tAttrSpace - RPN is violated
                     continue;
                 }
+                if (!attrPtg.isSum()) {
+                    throw new RuntimeException("Unexpected tAttr: " + attrPtg.toString());
+                }
             }
 
             final OperationPtg o = (OperationPtg) ptg;
index 351d44a421a5f4e920e119687250b7b2cd616b47..4aee71fbd444f4aec0b9edf010800fb7ab42d8d0 100644 (file)
@@ -39,6 +39,11 @@ public final class AttrPtg extends OperationPtg {
     private byte              field_1_options;
     private short             field_2_data;
     
+    /** only used for tAttrChoose: table of offsets to starts of args */
+    private final int[] _jumpTable;
+    /** only used for tAttrChoose: offset to the tFuncVar for CHOOSE() */
+    private final int   _chooseFuncOffset;
+    
     // flags 'volatile' and 'space', can be combined.  
     // OOO spec says other combinations are theoretically possible but not likely to occur.
     private static final BitField semiVolatile = BitFieldFactory.getInstance(0x01);
@@ -63,7 +68,7 @@ public final class AttrPtg extends OperationPtg {
         /** 03H = Carriage returns before opening parenthesis (only allowed before tParen token) */
         public static final int CR_BEFORE_OPEN_PAREN = 0x03;
         /** 04H = Spaces before closing parenthesis (only allowed before tParen, tFunc, and tFuncVar tokens) */
-        public static final int SPACE_BEFORE_CLOSE_PAERN = 0x04;
+        public static final int SPACE_BEFORE_CLOSE_PAREN = 0x04;
         /** 05H = Carriage returns before closing parenthesis (only allowed before tParen, tFunc, and tFuncVar tokens) */
         public static final int CR_BEFORE_CLOSE_PAREN = 0x05;
         /** 06H = Spaces following the equality sign (only in macro sheets) */
@@ -71,16 +76,33 @@ public final class AttrPtg extends OperationPtg {
     }
 
     public AttrPtg() {
+        _jumpTable = null;
+        _chooseFuncOffset = -1;
     }
     
     public AttrPtg(RecordInputStream in)
     {
         field_1_options = in.readByte();
         field_2_data    = in.readShort();
+        if (isOptimizedChoose()) {
+            int nCases = field_2_data;
+            int[] jumpTable = new int[nCases];
+            for (int i = 0; i < jumpTable.length; i++) {
+                jumpTable[i] = in.readUShort();
+            }
+            _jumpTable = jumpTable;
+            _chooseFuncOffset = in.readUShort();
+        } else {
+            _jumpTable = null;
+            _chooseFuncOffset = -1;
+        }
+        
     }
-    private AttrPtg(int options, int data) {
+    private AttrPtg(int options, int data, int[] jt, int chooseFuncOffset) {
         field_1_options = (byte) options;
         field_2_data = (short) data;
+        _jumpTable = jt;
+        _chooseFuncOffset = chooseFuncOffset;
     }
     
     /**
@@ -89,7 +111,7 @@ public final class AttrPtg extends OperationPtg {
      */
     public static AttrPtg createSpace(int type, int count) {
         int data = type & 0x00FF | (count << 8) & 0x00FFFF;
-        return new AttrPtg(space.set(0), data);
+        return new AttrPtg(space.set(0), data, null, -1);
     }
 
     public void setOptions(byte options)
@@ -136,14 +158,14 @@ public final class AttrPtg extends OperationPtg {
         field_1_options=optiIf.setByteBoolean(field_1_options,bif);
     }
 
-       /**
-        * Flags this ptg as a goto/jump 
-        * @param isGoto
-        */
-       public void setGoto(boolean isGoto) {
-               field_1_options=optGoto.setByteBoolean(field_1_options, isGoto);
-       }
-       
+    /**
+     * Flags this ptg as a goto/jump 
+     * @param isGoto
+     */
+    public void setGoto(boolean isGoto) {
+        field_1_options=optGoto.setByteBoolean(field_1_options, isGoto);
+    }
+    
     // lets hope no one uses this anymore
     public boolean isBaxcel()
     {
@@ -181,7 +203,7 @@ public final class AttrPtg extends OperationPtg {
         if(isOptimizedIf()) {
             sb.append("if dist=").append(getData());
         } else if(isOptimizedChoose()) {
-            sb.append("choose dist=").append(getData());
+            sb.append("choose nCases=").append(getData());
         } else if(isGoto()) {
             sb.append("skip dist=").append(getData()); 
         } else if(isSum()) {
@@ -195,13 +217,28 @@ public final class AttrPtg extends OperationPtg {
 
     public void writeBytes(byte [] array, int offset)
     {
-        array[offset]=sid;
-        array[offset+1]=field_1_options;
-        LittleEndian.putShort(array,offset+2,field_2_data);                
+        LittleEndian.putByte(array, offset+0, sid);
+        LittleEndian.putByte(array, offset+1, field_1_options);
+        LittleEndian.putShort(array,offset+2, field_2_data);
+        int[] jt = _jumpTable;
+        if (jt != null) {
+            int joff = offset+4; 
+            LittleEndian.putUShort(array, joff, _chooseFuncOffset);
+            joff+=2;
+            for (int i = 0; i < jt.length; i++) {
+                LittleEndian.putUShort(array, joff, jt[i]);
+                joff+=2;
+            }
+            LittleEndian.putUShort(array, joff, _chooseFuncOffset);
+        }
+        
     }
 
     public int getSize()
     {
+        if (_jumpTable != null) {
+            return SIZE + (_jumpTable.length + 1) * LittleEndian.SHORT_SIZE;
+        }
         return SIZE;
     }
 
@@ -255,12 +292,17 @@ public final class AttrPtg extends OperationPtg {
     
     
  
-    public byte getDefaultOperandClass() {return Ptg.CLASS_VALUE;}
+    public byte getDefaultOperandClass() {
+        return Ptg.CLASS_VALUE;
+    }
 
     public Object clone() {
-      AttrPtg ptg = new AttrPtg();
-      ptg.field_1_options = field_1_options;
-      ptg.field_2_data = field_2_data;
-      return ptg;
+        int[] jt;
+        if (_jumpTable == null) {
+            jt = null;
+        } else {
+            jt = (int[]) _jumpTable.clone();
+        }
+        return new AttrPtg(field_1_options, field_2_data, jt, _chooseFuncOffset);
     }
 }
index 714c0358ff7d9665bfbe437cc2cd9b12a64a34f4..7a9acb087b2f231ac8c625904cbdc4d55eec31db 100644 (file)
@@ -38,6 +38,8 @@ public final class TestExternalNameRecord extends TestCase {
        // data taken from bugzilla 44774 att 21790\r
        private static final byte[] dataPlainName = {\r
                0, 0, 0, 0, 0, 0, 9, 0, 82, 97, 116, 101, 95, 68, 97, 116, 101, 9, 0, 58, 0, 0, 0, 0, 4, 0, 8, 0\r
+               // TODO - the last 2 bytes of formula data (8,0) seem weird.  They encode to ConcatPtg, UnknownPtg\r
+               // UnknownPtg is otherwise not created by any other test cases\r
        };\r
        \r
        private static ExternalNameRecord createSimpleENR(byte[] data) {\r
index abe6943c799927b2396fab1e4a4a6ce7338a0d82..4f5e390873082dec5276edb73414153127434bf3 100644 (file)
@@ -19,16 +19,14 @@ package org.apache.poi.hssf.record;
 
 
 import java.io.ByteArrayInputStream;
+import java.util.List;
+
+import junit.framework.TestCase;
 
 import org.apache.poi.hssf.record.formula.AttrPtg;
-import org.apache.poi.hssf.record.formula.ConcatPtg;
 import org.apache.poi.hssf.record.formula.FuncVarPtg;
 import org.apache.poi.hssf.record.formula.IntPtg;
-import org.apache.poi.hssf.record.formula.RangePtg;
 import org.apache.poi.hssf.record.formula.ReferencePtg;
-import org.apache.poi.hssf.record.formula.UnknownPtg;
-
-import junit.framework.TestCase;
 
 /**
  * Tests the serialization and deserialization of the FormulaRecord
@@ -57,7 +55,7 @@ public final class TestFormulaRecord extends TestCase {
         */
        public void testCheckNanPreserve() {
                byte[] formulaByte = new byte[29];
-               for (int i = 0; i < formulaByte.length; i++) formulaByte[i] = (byte)0;
+
                formulaByte[4] = (byte)0x0F;
                formulaByte[6] = (byte)0x02;
                formulaByte[8] = (byte)0x07;
@@ -91,8 +89,6 @@ public final class TestFormulaRecord extends TestCase {
        public void testExpFormula() {
                byte[] formulaByte = new byte[27];
                
-               for (int i = 0; i < formulaByte.length; i++) formulaByte[i] = (byte)0;
-               
                formulaByte[4] =(byte)0x0F;
                formulaByte[14]=(byte)0x08;
                formulaByte[18]=(byte)0xE0;
@@ -109,15 +105,14 @@ public final class TestFormulaRecord extends TestCase {
        
        public void testWithConcat()  throws Exception {
                // =CHOOSE(2,A2,A3,A4)
-               byte[] data = new byte[] {
+               byte[] data = {
                                6, 0, 68, 0,
                                1, 0, 1, 0, 15, 0, 0, 0, 0, 0, 0, 0, 57,
                                64, 0, 0, 12, 0, 12, -4, 46, 0, 
                                30, 2, 0,       // Int - 2
                                25, 4, 3, 0, // Attr
-                               8, 0,           // Concat 
-                               17, 0,     // Range 
-                               26, 0, 35, 0, // Bit like an attr
+                                       8, 0, 17, 0, 26, 0, // jumpTable
+                                       35, 0, // chooseOffset
                                36, 1, 0, 0, -64, // Ref - A2
                                25, 8, 21, 0, // Attr
                                36, 2, 0, 0, -64, // Ref - A3
@@ -126,30 +121,24 @@ public final class TestFormulaRecord extends TestCase {
                                25, 8, 3, 0,  // Attr 
                                66, 4, 100, 0 // CHOOSE
                };
-               RecordInputStream inp = new RecordInputStream(
-                               new ByteArrayInputStream(data)
-               );
+               RecordInputStream inp = new RecordInputStream( new ByteArrayInputStream(data));
                inp.nextRecord();
                
                FormulaRecord fr = new FormulaRecord(inp);
                
-               assertEquals(14, fr.getNumberOfExpressionTokens());
-               assertEquals(IntPtg.class,         fr.getParsedExpression().get(0).getClass());
-               assertEquals(AttrPtg.class,       fr.getParsedExpression().get(1).getClass());
-               assertEquals(ConcatPtg.class,   fr.getParsedExpression().get(2).getClass());
-               assertEquals(UnknownPtg.class,   fr.getParsedExpression().get(3).getClass());
-               assertEquals(RangePtg.class,     fr.getParsedExpression().get(4).getClass());
-               assertEquals(UnknownPtg.class,   fr.getParsedExpression().get(5).getClass());
-               assertEquals(AttrPtg.class,       fr.getParsedExpression().get(6).getClass());
-               assertEquals(ReferencePtg.class, fr.getParsedExpression().get(7).getClass());
-               assertEquals(AttrPtg.class,       fr.getParsedExpression().get(8).getClass());
-               assertEquals(ReferencePtg.class, fr.getParsedExpression().get(9).getClass());
-               assertEquals(AttrPtg.class,       fr.getParsedExpression().get(10).getClass());
-               assertEquals(ReferencePtg.class, fr.getParsedExpression().get(11).getClass());
-               assertEquals(AttrPtg.class,       fr.getParsedExpression().get(12).getClass());
-               assertEquals(FuncVarPtg.class,   fr.getParsedExpression().get(13).getClass());
+               List ptgs = fr.getParsedExpression();
+               assertEquals(9, ptgs.size());
+               assertEquals(IntPtg.class,         ptgs.get(0).getClass());
+               assertEquals(AttrPtg.class,       ptgs.get(1).getClass());
+               assertEquals(ReferencePtg.class, ptgs.get(2).getClass());
+               assertEquals(AttrPtg.class,       ptgs.get(3).getClass());
+               assertEquals(ReferencePtg.class, ptgs.get(4).getClass());
+               assertEquals(AttrPtg.class,       ptgs.get(5).getClass());
+               assertEquals(ReferencePtg.class, ptgs.get(6).getClass());
+               assertEquals(AttrPtg.class,       ptgs.get(7).getClass());
+               assertEquals(FuncVarPtg.class,   ptgs.get(8).getClass());
                
-               FuncVarPtg choose = (FuncVarPtg)fr.getParsedExpression().get(13);
+               FuncVarPtg choose = (FuncVarPtg)ptgs.get(8);
                assertEquals("CHOOSE", choose.getName());
        }
        
index 1a62af43566ee20ae940629d56a09789ae3149c6..53e06ab292128d11c162dff5cd9d744d10aec397 100644 (file)
 
 package org.apache.poi.hssf.usermodel;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.Iterator;
+
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
-import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.util.Region;
 import org.apache.poi.util.TempFile;
 
-import java.io.*;
-import java.util.Iterator;
-
 /**
  * Testcases for bugs entered in bugzilla
  * the Test name contains the bugzilla bug id
@@ -80,13 +81,7 @@ public final class TestBugs extends TestCase {
         HSSFRow r = s.createRow(0);
         HSSFCell c = r.createCell((short)0);
         c.setCellValue(10);
-        try {
-            writeOutAndReadBack(wb);
-        } catch (RecordFormatException e) {
-            if (false) { // TODO (Apr-2008) this file does not read back ok.  create bugzilla bug & fix.
-                throw new AssertionFailedError("Identified bug XXXX");
-            }
-        }
+        writeOutAndReadBack(wb);
     }
     /**Test writing a hyperlink
      * Open resulting sheet in Excel and check that A1 contains a hyperlink*/
@@ -757,9 +752,13 @@ public final class TestBugs extends TestCase {
         HSSFCell c2 = r2.getCell((short)1);
         assertEquals(25, (int)c2.getNumericCellValue());
 
-        if (false) { // TODO (Apr-2008) This will blow up with IllegalStateException (stack underflow)
-            // excel function "CHOOSE" probably needs some special handling in FormulaParser.toFormulaString()
-            assertEquals("=CHOOSE(2,A2,A3,A4)", c2.getCellFormula());
+        try {
+            assertEquals("CHOOSE(2,A2,A3,A4)", c2.getCellFormula());
+        } catch (IllegalStateException e) {
+            if (e.getMessage().startsWith("Too few arguments")
+                    && e.getMessage().indexOf("ConcatPtg") > 0) {
+                throw new AssertionFailedError("identified bug 44306");
+            }
         }
     }
 
@@ -887,13 +886,13 @@ public final class TestBugs extends TestCase {
         writeOutAndReadBack(wb);
         assertTrue("no errors writing sample xls", true);
     }
-    
+
     /**
      * Had a problem apparently, not sure what as it
      *  works just fine...
      */
     public void test44891() throws Exception {
-       HSSFWorkbook wb = openSample("44891.xls");
+        HSSFWorkbook wb = openSample("44891.xls");
         assertTrue("no errors reading sample xls", true);
         writeOutAndReadBack(wb);
         assertTrue("no errors writing sample xls", true);
@@ -905,7 +904,7 @@ public final class TestBugs extends TestCase {
      * Works fine with poi-3.1-beta1.
      */
     public void test44235() throws Exception {
-       HSSFWorkbook wb = openSample("44235.xls");
+        HSSFWorkbook wb = openSample("44235.xls");
         assertTrue("no errors reading sample xls", true);
         writeOutAndReadBack(wb);
         assertTrue("no errors writing sample xls", true);
@@ -929,7 +928,7 @@ public final class TestBugs extends TestCase {
     }
 
     public void test36947() throws Exception {
-       HSSFWorkbook wb = openSample("36947.xls");
+        HSSFWorkbook wb = openSample("36947.xls");
         assertTrue("no errors reading sample xls", true);
         writeOutAndReadBack(wb);
         assertTrue("no errors writing sample xls", true);
@@ -946,7 +945,7 @@ public final class TestBugs extends TestCase {
     }
 
     public void test39634() throws Exception {
-       HSSFWorkbook wb = openSample("39634.xls");
+        HSSFWorkbook wb = openSample("39634.xls");
         assertTrue("no errors reading sample xls", true);
         writeOutAndReadBack(wb);
         assertTrue("no errors writing sample xls", true);