diff options
7 files changed, 108 insertions, 71 deletions
diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 65fe1e25c4..f6f5f8a620 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -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> diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 28019e68fe..2b5196f962 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -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> diff --git a/src/java/org/apache/poi/hssf/model/FormulaParser.java b/src/java/org/apache/poi/hssf/model/FormulaParser.java index f16ab45129..2819786ef6 100644 --- a/src/java/org/apache/poi/hssf/model/FormulaParser.java +++ b/src/java/org/apache/poi/hssf/model/FormulaParser.java @@ -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; diff --git a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java b/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java index 351d44a421..4aee71fbd4 100644 --- a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java @@ -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); } } 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); |