Bug 44306 - fixed reading/writing of AttrPtg and rendering of CHOOSE formulas

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

3 files changed, 40 insertions, 50 deletions

diff --git a/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java b/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java
index 714c0358ff..7a9acb087b 100644
--- a/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java
+++ b/src/testcases/org/apache/poi/hssf/record/TestExternalNameRecord.java
@@ -38,6 +38,8 @@ public final class TestExternalNameRecord extends TestCase {
 	// data taken from bugzilla 44774 att 21790

 	private static final byte[] dataPlainName = {

 		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

+		// TODO - the last 2 bytes of formula data (8,0) seem weird.  They encode to ConcatPtg, UnknownPtg

+		// UnknownPtg is otherwise not created by any other test cases

 	};

 	

 	private static ExternalNameRecord createSimpleENR(byte[] data) {

diff --git a/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java b/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java
index abe6943c79..4f5e390873 100644
--- a/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java
+++ b/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java
@@ -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());
 	}
 	
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
index 1a62af4356..53e06ab292 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
@@ -17,17 +17,18 @@
 
 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);