diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index ef0430c73..a279c64ac 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 47048 - Fixed evaluation of defined names with the 'complex' flag set 46953 - More tweaks to PageSettingsBlock parsing logic in Sheet constructor 47089 - Fixed XSSFWorkbook.createSheet to properly increment sheetId 46568 - Fixed XSLFPowerPointExtractor to properly process line breaks diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 7e1027f6d..65e7caa03 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 47048 - Fixed evaluation of defined names with the 'complex' flag set 46953 - More tweaks to PageSettingsBlock parsing logic in Sheet constructor 47089 - Fixed XSSFWorkbook.createSheet to properly increment sheetId 46568 - Fixed XSLFPowerPointExtractor to properly process line breaks diff --git a/src/java/org/apache/poi/hssf/record/NameRecord.java b/src/java/org/apache/poi/hssf/record/NameRecord.java index a93822360..c9b6f6570 100644 --- a/src/java/org/apache/poi/hssf/record/NameRecord.java +++ b/src/java/org/apache/poi/hssf/record/NameRecord.java @@ -72,6 +72,9 @@ public final class NameRecord extends StandardRecord { public static final int OPT_COMPLEX = 0x0010; public static final int OPT_BUILTIN = 0x0020; public static final int OPT_BINDATA = 0x1000; + public static final boolean isFormula(int optValue) { + return (optValue & 0x0F) == 0; + } } private short field_1_option_flag; @@ -239,7 +242,7 @@ public final class NameRecord extends StandardRecord { * @return true if name has a formula (named range or defined value) */ public boolean hasFormula() { - return field_1_option_flag == 0 && field_13_name_definition.getEncodedTokenSize() > 0; + return Option.isFormula(field_1_option_flag) && field_13_name_definition.getEncodedTokenSize() > 0; } /** diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java index a68bb042f..0ccd54153 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java +++ b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java @@ -21,9 +21,41 @@ import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.constant.ErrorConstant; import org.apache.poi.hssf.record.formula.AbstractFunctionPtg; -import org.apache.poi.hssf.record.formula.*; +import org.apache.poi.hssf.record.formula.AddPtg; +import org.apache.poi.hssf.record.formula.Area3DPtg; +import org.apache.poi.hssf.record.formula.AreaI; +import org.apache.poi.hssf.record.formula.AreaPtg; +import org.apache.poi.hssf.record.formula.AreaPtgBase; +import org.apache.poi.hssf.record.formula.ArrayPtg; +import org.apache.poi.hssf.record.formula.AttrPtg; +import org.apache.poi.hssf.record.formula.BoolPtg; +import org.apache.poi.hssf.record.formula.ConcatPtg; +import org.apache.poi.hssf.record.formula.DividePtg; +import org.apache.poi.hssf.record.formula.EqualPtg; +import org.apache.poi.hssf.record.formula.ErrPtg; +import org.apache.poi.hssf.record.formula.FuncPtg; +import org.apache.poi.hssf.record.formula.FuncVarPtg; +import org.apache.poi.hssf.record.formula.IntPtg; +import org.apache.poi.hssf.record.formula.MemAreaPtg; +import org.apache.poi.hssf.record.formula.MemFuncPtg; +import org.apache.poi.hssf.record.formula.MissingArgPtg; +import org.apache.poi.hssf.record.formula.MultiplyPtg; +import org.apache.poi.hssf.record.formula.NamePtg; +import org.apache.poi.hssf.record.formula.NumberPtg; +import org.apache.poi.hssf.record.formula.PercentPtg; +import org.apache.poi.hssf.record.formula.PowerPtg; +import org.apache.poi.hssf.record.formula.Ptg; +import org.apache.poi.hssf.record.formula.RangePtg; +import org.apache.poi.hssf.record.formula.Ref3DPtg; +import org.apache.poi.hssf.record.formula.RefPtg; +import org.apache.poi.hssf.record.formula.StringPtg; +import org.apache.poi.hssf.record.formula.SubtractPtg; +import org.apache.poi.hssf.record.formula.UnaryMinusPtg; +import org.apache.poi.hssf.record.formula.UnaryPlusPtg; +import org.apache.poi.hssf.record.formula.UnionPtg; import org.apache.poi.hssf.usermodel.FormulaExtractor; import org.apache.poi.hssf.usermodel.HSSFCell; import org.apache.poi.hssf.usermodel.HSSFErrorConstants; @@ -32,6 +64,7 @@ import org.apache.poi.hssf.usermodel.HSSFName; import org.apache.poi.hssf.usermodel.HSSFRow; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.hssf.usermodel.TestHSSFName; import org.apache.poi.ss.formula.FormulaParser; import org.apache.poi.ss.formula.FormulaParserTestHelper; import org.apache.poi.ss.usermodel.BaseTestBugzillaIssues; @@ -450,7 +483,7 @@ public final class TestFormulaParser extends TestCase { */ public void testPrecedenceAndAssociativity() { - Class[] expClss; + Class[] expClss; // TRUE=TRUE=2=2 evaluates to FALSE expClss = new Class[] { BoolPtg.class, BoolPtg.class, EqualPtg.class, @@ -484,12 +517,12 @@ public final class TestFormulaParser extends TestCase { confirmTokenClasses("2^200%", expClss); } - /* package */ static Ptg[] confirmTokenClasses(String formula, Class[] expectedClasses) { + /* package */ static Ptg[] confirmTokenClasses(String formula, Class[] expectedClasses) { Ptg[] ptgs = parseFormula(formula); confirmTokenClasses(ptgs, expectedClasses); return ptgs; } - private static void confirmTokenClasses(Ptg[] ptgs, Class[] expectedClasses) { + private static void confirmTokenClasses(Ptg[] ptgs, Class[] expectedClasses) { assertEquals(expectedClasses.length, ptgs.length); for (int i = 0; i < expectedClasses.length; i++) { if(expectedClasses[i] != ptgs[i].getClass()) { @@ -504,7 +537,7 @@ public final class TestFormulaParser extends TestCase { confirmTokenClasses("2^5", new Class[] { IntPtg.class, IntPtg.class, PowerPtg.class, }); } - private static Ptg parseSingleToken(String formula, Class ptgClass) { + private static Ptg parseSingleToken(String formula, Class ptgClass) { Ptg[] ptgs = parseFormula(formula); assertEquals(1, ptgs.length); Ptg result = ptgs[0]; @@ -533,7 +566,7 @@ public final class TestFormulaParser extends TestCase { public void testMissingArgs() { - Class[] expClss; + Class[] expClss; expClss = new Class[] { RefPtg.class, @@ -930,7 +963,7 @@ public final class TestFormulaParser extends TestCase { wb.createSheet("Sheet1"); Ptg[] ptgs = FormulaParser.parse(formula, HSSFEvaluationWorkbook.create(wb)); - Class[] expectedClasses = { + Class[] expectedClasses = { // TODO - AttrPtg.class, // Excel prepends this MemFuncPtg.class, Area3DPtg.class, @@ -958,7 +991,7 @@ public final class TestFormulaParser extends TestCase { throw new AssertionFailedError("Identified bug 46643"); } - Class [] expectedClasses = { + Class [] expectedClasses = { MemFuncPtg.class, Ref3DPtg.class, Ref3DPtg.class, @@ -1026,7 +1059,7 @@ public final class TestFormulaParser extends TestCase { RangePtg.class, // AttrPtg.class, // [sum ] }); - + } public void testUnionOfFullCollFullRowRef() { @@ -1165,4 +1198,35 @@ public final class TestFormulaParser extends TestCase { FormulaParserTestHelper.confirmParseException(e, expectedMessage); } } + + /** + * In bug 47078, POI had trouble evaluating a defined name flagged as 'complex'. + * POI should also be able to parse such defined names. + */ + public void testParseComplexName() { + + // Mock up a spreadsheet to match the critical details of the sample + HSSFWorkbook wb = new HSSFWorkbook(); + wb.createSheet("Sheet1"); + HSSFName definedName = wb.createName(); + definedName.setNameName("foo"); + definedName.setRefersToFormula("Sheet1!B2"); + + // Set the complex flag - POI doesn't usually manipulate this flag + NameRecord nameRec = TestHSSFName.getNameRecord(definedName); + nameRec.setOptionFlag((short)0x10); // 0x10 -> complex + + Ptg[] result; + try { + result = HSSFFormulaParser.parse("1+foo", wb); + } catch (RuntimeException e) { + FormulaParserTestHelper.confirmParseException(e); + if (e.getMessage().equals("Specified name 'foo' is not a range as expected.")) { + throw new AssertionFailedError("Identified bug 47078c"); + } + throw e; + } + assertNotNull("Ptg array should not be null", result); + confirmTokenClasses(result, new Class[] { IntPtg.class, NamePtg.class, AddPtg.class,}); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java index 4aba177bf..082f07a84 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java @@ -21,9 +21,11 @@ import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.record.NameRecord; +import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CellValue; /** - * + * * @author Josh Micich */ public final class TestHSSFFormulaEvaluator extends TestCase { @@ -41,7 +43,7 @@ public final class TestHSSFFormulaEvaluator extends TestCase { assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cv.getCellType()); assertEquals(3.72, cv.getNumberValue(), 0.0); } - + public void testFullColumnRefs() { HSSFWorkbook wb = new HSSFWorkbook(); HSSFSheet sheet = wb.createSheet("Sheet1"); @@ -56,18 +58,18 @@ public final class TestHSSFFormulaEvaluator extends TestCase { setValue(sheet, 2, 3, 6.0); setValue(sheet, 5, 3, 7.0); setValue(sheet, 50, 3, 8.0); - + // some values in column E setValue(sheet, 1, 4, 9.0); setValue(sheet, 2, 4, 10.0); setValue(sheet, 30000, 4, 11.0); - - // some other values + + // some other values setValue(sheet, 1, 2, 100.0); setValue(sheet, 2, 5, 100.0); setValue(sheet, 3, 6, 100.0); - - + + HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb); assertEquals(26.0, fe.evaluate(cell0).getNumberValue(), 0.0); assertEquals(56.0, fe.evaluate(cell1).getNumberValue(), 0.0); @@ -121,4 +123,48 @@ public final class TestHSSFFormulaEvaluator extends TestCase { } assertEquals(3.5, cellB1.getNumericCellValue(), 0.0); } + + /** + * When evaluating defined names, POI has to decide whether it is capable. Currently + * (May2009) POI only supports simple cell and area refs.
+ * The sample spreadsheet (bugzilla attachment 23508) had a name flagged as 'complex' + * which contained a simple area ref. It is not clear what the 'complex' flag is used + * for but POI should look elsewhere to decide whether it can evaluate the name. + */ + public void testDefinedNameWithComplexFlag_bug47048() { + // Mock up a spreadsheet to match the critical details of the sample + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet sheet = wb.createSheet("Input"); + HSSFName definedName = wb.createName(); + definedName.setNameName("Is_Multicar_Vehicle"); + definedName.setRefersToFormula("Input!$B$17:$G$17"); + + // Set up some data and the formula + HSSFRow row17 = sheet.createRow(16); + row17.createCell(0).setCellValue(25.0); + row17.createCell(1).setCellValue(1.33); + row17.createCell(2).setCellValue(4.0); + + HSSFRow row = sheet.createRow(0); + HSSFCell cellA1 = row.createCell(0); + cellA1.setCellFormula("SUM(Is_Multicar_Vehicle)"); + + // Set the complex flag - POI doesn't usually manipulate this flag + NameRecord nameRec = TestHSSFName.getNameRecord(definedName); + nameRec.setOptionFlag((short)0x10); // 0x10 -> complex + + HSSFFormulaEvaluator hsf = new HSSFFormulaEvaluator(wb); + CellValue value; + try { + value = hsf.evaluate(cellA1); + } catch (RuntimeException e) { + if (e.getMessage().equals("Don't now how to evalate name 'Is_Multicar_Vehicle'")) { + throw new AssertionFailedError("Identified bug 47048a"); + } + throw e; + } + + assertEquals(Cell.CELL_TYPE_NUMERIC, value.getCellType()); + assertEquals(5.33, value.getNumberValue(), 0.0); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFName.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFName.java index 291698ec8..88a2c0cf1 100755 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFName.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFName.java @@ -17,28 +17,19 @@ package org.apache.poi.hssf.usermodel; -import java.util.Date; -import java.util.GregorianCalendar; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.lang.reflect.Field; -import junit.framework.AssertionFailedError; import org.apache.poi.hssf.HSSFITestDataProvider; import org.apache.poi.hssf.HSSFTestDataSamples; -import org.apache.poi.hssf.model.Sheet; import org.apache.poi.hssf.model.HSSFFormulaParser; -import org.apache.poi.hssf.record.DBCellRecord; -import org.apache.poi.hssf.record.FormulaRecord; -import org.apache.poi.hssf.record.Record; -import org.apache.poi.hssf.record.StringRecord; +import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.formula.Ptg; -import org.apache.poi.ss.usermodel.ErrorConstants; -import org.apache.poi.ss.usermodel.BaseTestCell; +import org.apache.poi.ss.formula.FormulaType; import org.apache.poi.ss.usermodel.BaseTestNamedRange; import org.apache.poi.ss.util.AreaReference; -import org.apache.poi.ss.util.CellReference; -import org.apache.poi.ss.formula.FormulaType; /** * Tests various functionality having to do with {@link org.apache.poi.ss.usermodel.Name}. @@ -50,6 +41,30 @@ import org.apache.poi.ss.formula.FormulaType; */ public final class TestHSSFName extends BaseTestNamedRange { + /** + * For manipulating the internals of {@link HSSFName} during testing.
+ * Some tests need a {@link NameRecord} with unusual state, not normally producible by POI. + * This method achieves the aims at low cost without augmenting the POI usermodel api. + * @return a reference to the wrapped {@link NameRecord} + */ + public static NameRecord getNameRecord(HSSFName definedName) { + + Field f; + try { + f = HSSFName.class.getDeclaredField("_definedNameRec"); + } catch (NoSuchFieldException e) { + throw new RuntimeException(e); + } + f.setAccessible(true); + try { + return (NameRecord) f.get(definedName); + } catch (IllegalArgumentException e) { + throw new RuntimeException(e); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + @Override protected HSSFITestDataProvider getTestDataProvider(){ return HSSFITestDataProvider.getInstance(); @@ -71,7 +86,7 @@ public final class TestHSSFName extends BaseTestNamedRange { assertEquals("Print_Titles", nr1.getNameName()); if (false) { - // TODO - full column references not rendering properly, absolute markers not present either + // TODO - full column references not rendering properly, absolute markers not present either assertEquals("FirstSheet!$A:$A,FirstSheet!$1:$3", nr1.getRefersToFormula()); } else { assertEquals("FirstSheet!A:A,FirstSheet!$A$1:$IV$3", nr1.getRefersToFormula()); @@ -112,7 +127,6 @@ public final class TestHSSFName extends BaseTestNamedRange { } } - /** Test of TestCase method, of class test.RangeTest. */ public void testNamedRange() { HSSFWorkbook wb = getTestDataProvider().openSampleWorkbook("Simple.xls"); @@ -150,8 +164,8 @@ public final class TestHSSFName extends BaseTestNamedRange { public void testNamedRead() { HSSFWorkbook wb = getTestDataProvider().openSampleWorkbook("namedinput.xls"); - //Get index of the namedrange with the name = "NamedRangeName" , which was defined in input.xls as A1:D10 - int NamedRangeIndex = wb.getNameIndex("NamedRangeName"); + //Get index of the named range with the name = "NamedRangeName" , which was defined in input.xls as A1:D10 + int NamedRangeIndex = wb.getNameIndex("NamedRangeName"); //Getting NAmed Range HSSFName namedRange1 = wb.getNameAt(NamedRangeIndex); @@ -201,7 +215,7 @@ public final class TestHSSFName extends BaseTestNamedRange { } - public void testDeletedReference() throws Exception { + public void testDeletedReference() { HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("24207.xls"); assertEquals(2, wb.getNumberOfNames()); @@ -224,9 +238,9 @@ public final class TestHSSFName extends BaseTestNamedRange { } /** - * When setting A1 type of referencese HSSFName.setRefersToFormula + * When setting A1 type of references with HSSFName.setRefersToFormula * must set the type of operands to Ptg.CLASS_REF, - * otherwise created named don't appear in the dropdown to the left opf formula bar in Excel + * otherwise created named don't appear in the drop-down to the left of formula bar in Excel */ public void testTypeOfRootPtg(){ HSSFWorkbook wb = new HSSFWorkbook();