From 102c0d2173985b96966600b5b01d9980199ae1a0 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 31 Dec 2016 16:53:16 +0000 Subject: [PATCH] Bug 60219: FormulaParser can't parse external references when sheet name is quoted git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1776796 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/ss/formula/FormulaParser.java | 51 ++++++++++--------- .../poi/ss/formula/TestFormulaParser.java | 2 - .../poi/hssf/model/TestFormulaParser.java | 22 ++++---- .../apache/poi/hssf/model/TestWorkbook.java | 2 +- .../apache/poi/hssf/usermodel/TestBugs.java | 16 ++++-- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/src/java/org/apache/poi/ss/formula/FormulaParser.java b/src/java/org/apache/poi/ss/formula/FormulaParser.java index 6caf8e1eb..745836ef7 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaParser.java +++ b/src/java/org/apache/poi/ss/formula/FormulaParser.java @@ -115,7 +115,7 @@ public final class FormulaParser { * Tracks whether the run of whitespace preceding "look" could be an * intersection operator. See GetChar. */ - private boolean _inIntersection = false; + private boolean _inIntersection; private final FormulaParsingWorkbook _book; private final SpreadsheetVersion _ssVersion; @@ -133,7 +133,7 @@ public final class FormulaParser { * This class is recommended only for single threaded use. * * If you have a {@link org.apache.poi.hssf.usermodel.HSSFWorkbook}, and not a - * {@link org.apache.poi.hssf.model.Workbook}, then use the convenience method on + * {@link org.apache.poi.ss.usermodel.Workbook}, then use the convenience method on * {@link org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator} */ private FormulaParser(String formula, FormulaParsingWorkbook book, int sheetIndex, int rowIndex) { @@ -298,7 +298,7 @@ public final class FormulaParser { /** Get a Number */ private String GetNum() { - StringBuffer value = new StringBuffer(); + StringBuilder value = new StringBuilder(); while (IsDigit(this.look)){ value.append(this.look); @@ -670,7 +670,7 @@ public final class FormulaParser { GetChar(); } // parse column quantifier - String startColumnName = null; + String startColumnName; String endColumnName = null; int nColQuantifiers = 0; int savePtr1 = _pointer; @@ -1112,13 +1112,20 @@ public final class FormulaParser { @Override public String toString() { - StringBuilder sb = new StringBuilder(64); - sb.append(getClass().getName()).append(" ["); - sb.append(_rep); - sb.append("]"); - return sb.toString(); + return getClass().getName() + " [" + _rep + "]"; } } + + private String getBookName() { + StringBuilder sb = new StringBuilder(); + GetChar(); + while (look != ']') { + sb.append(look); + GetChar(); + } + GetChar(); + return sb.toString(); + } /** * Note - caller should reset {@link #_pointer} upon null result @@ -1127,22 +1134,18 @@ public final class FormulaParser { private SheetIdentifier parseSheetName() { String bookName; if (look == '[') { - StringBuilder sb = new StringBuilder(); - GetChar(); - while (look != ']') { - sb.append(look); - GetChar(); - } - GetChar(); - bookName = sb.toString(); + bookName = getBookName(); } else { bookName = null; } if (look == '\'') { - StringBuffer sb = new StringBuffer(); - Match('\''); + + if (look == '[') + bookName = getBookName(); + + StringBuilder sb = new StringBuilder(); boolean done = look == '\''; while(!done) { sb.append(look); @@ -1232,7 +1235,7 @@ public final class FormulaParser { boolean result = CellReference.classifyCellReference(str, _ssVersion) == NameType.CELL; if(result){ - /** + /* * Check if the argument is a function. Certain names can be either a cell reference or a function name * depending on the contenxt. Compare the following examples in Excel 2007: * (a) LOG10(100) + 1 @@ -1323,7 +1326,7 @@ public final class FormulaParser { * Adds a name (named range or user defined function) to underlying workbook's names table * @param functionName */ - private final void addName(String functionName) { + private void addName(String functionName) { final Name name = _book.createName(); name.setFunction(true); name.setNameName(functionName); @@ -1766,7 +1769,7 @@ public final class FormulaParser { * return Int or Number Ptg based on size of input */ private static Ptg getNumberPtgFromString(String number1, String number2, String exponent) { - StringBuffer number = new StringBuffer(); + StringBuilder number = new StringBuilder(); if (number2 == null) { number.append(number1); @@ -1808,7 +1811,7 @@ public final class FormulaParser { private String parseStringLiteral() { Match('"'); - StringBuffer token = new StringBuffer(); + StringBuilder token = new StringBuilder(); while (true) { if (look == '"') { GetChar(); @@ -1975,7 +1978,7 @@ public final class FormulaParser { //{--------------------------------------------------------------} //{ Parse and Translate an Assignment Statement } - /** + /* procedure Assignment; var Name: string[8]; begin diff --git a/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java b/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java index dfbbd4281..421913fe7 100644 --- a/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java +++ b/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java @@ -33,7 +33,6 @@ import org.apache.poi.ss.formula.ptg.StringPtg; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.usermodel.XSSFEvaluationWorkbook; import org.apache.poi.xssf.usermodel.XSSFWorkbook; -import org.junit.Ignore; import org.junit.Test; /** @@ -189,7 +188,6 @@ public class TestFormulaParser { } // bug 60219: FormulaParser can't parse external references when sheet name is quoted - @Ignore @Test public void testParseExternalReferencesWithQuotedSheetName() throws Exception { XSSFWorkbook wb = new XSSFWorkbook(); diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java index 82153fe09..a6cb27343 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java +++ b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java @@ -218,7 +218,7 @@ public final class TestFormulaParser { } } - private final static void assertEqualsIgnoreCase(String expected, String actual) { + private static void assertEqualsIgnoreCase(String expected, String actual) { assertEquals(expected.toLowerCase(Locale.ROOT), actual.toLowerCase(Locale.ROOT)); } @@ -414,7 +414,7 @@ public final class TestFormulaParser { HSSFSheet sheet = wb.createSheet("Test"); HSSFRow row = sheet.createRow(0); HSSFCell cell = row.createCell(0); - String formula = null; + String formula; cell.setCellFormula("1.3E21/3"); formula = cell.getCellFormula(); @@ -488,7 +488,7 @@ public final class TestFormulaParser { HSSFSheet sheet = wb.createSheet("Test"); HSSFRow row = sheet.createRow(0); HSSFCell cell = row.createCell(0); - String formula = null; + String formula; // starts from decimal point @@ -530,7 +530,7 @@ public final class TestFormulaParser { HSSFSheet sheet = wb.createSheet("Test"); HSSFRow row = sheet.createRow(0); HSSFCell cell = row.createCell(0); - String formula = null; + String formula; cell.setCellFormula("A1.A2"); formula = cell.getCellFormula(); @@ -557,7 +557,7 @@ public final class TestFormulaParser { HSSFSheet sheet = wb.createSheet("Test"); HSSFRow row = sheet.createRow(0); HSSFCell cell = row.createCell(0); - String formula = null; + String formula; // References to a single cell: @@ -718,8 +718,8 @@ public final class TestFormulaParser { assertEquals(65534.6, np.getValue(), 0); } + @Test public void testMissingArgs() { - confirmTokenClasses("if(A1, ,C1)", RefPtg.class, AttrPtg.class, // tAttrIf @@ -1327,8 +1327,7 @@ public final class TestFormulaParser { } /** - * TODO - delete equiv test: - * {@link BaseTestBugzillaIssues#test42448()} + * See the related/similar test: {@link BaseTestBugzillaIssues#bug42448()} */ @Test public void testParseAbnormalSheetNamesAndRanges_bug42448() throws IOException { @@ -1373,11 +1372,10 @@ public final class TestFormulaParser { @Test public void testUnionOfFullCollFullRowRef() throws IOException { - Ptg[] ptgs; - ptgs = parseFormula("3:4"); - ptgs = parseFormula("$Z:$AC"); + parseFormula("3:4"); + Ptg[] ptgs = parseFormula("$Z:$AC"); confirmTokenClasses(ptgs, AreaPtg.class); - ptgs = parseFormula("B:B"); + parseFormula("B:B"); ptgs = parseFormula("$11:$13"); confirmTokenClasses(ptgs, AreaPtg.class); diff --git a/src/testcases/org/apache/poi/hssf/model/TestWorkbook.java b/src/testcases/org/apache/poi/hssf/model/TestWorkbook.java index 6147358bc..558a5ec0f 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestWorkbook.java +++ b/src/testcases/org/apache/poi/hssf/model/TestWorkbook.java @@ -115,7 +115,7 @@ public final class TestWorkbook { } }; - /** + /* * register the two test UDFs in a UDF finder, to be passed to the evaluator */ UDFFinder udff1 = new DefaultUDFFinder(new String[] { "myFunc", }, diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 1308b2c11..862b6cec7 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -1836,17 +1836,26 @@ public final class TestBugs extends BaseTestBugzillaIssues { assertEquals("'[$http://gagravarr.org/FormulaRefs.xls]Sheet1'!B1", row.getCell(1).getCellFormula()); assertEquals(112.0, row.getCell(1).getNumericCellValue(), 0); + // Link our new workbook + Workbook externalWb1 = new HSSFWorkbook(); + externalWb1.createSheet("Sheet1"); + wb1.linkExternalWorkbook("$http://gagravarr.org/FormulaRefs2.xls", externalWb1); + // Change 4 row.getCell(1).setCellFormula("'[$http://gagravarr.org/FormulaRefs2.xls]Sheet1'!B2"); row.getCell(1).setCellValue(123.0); + // Link our new workbook + Workbook externalWb2 = new HSSFWorkbook(); + externalWb2.createSheet("Sheet1"); + wb1.linkExternalWorkbook("$http://example.com/FormulaRefs.xls", externalWb2); + // Add 5 row = s.createRow(5); row.createCell(1, CellType.FORMULA); row.getCell(1).setCellFormula("'[$http://example.com/FormulaRefs.xls]Sheet1'!B1"); row.getCell(1).setCellValue(234.0); - // Re-test HSSFWorkbook wb2 = writeOutAndReadBack(wb1); wb1.close(); @@ -1871,8 +1880,7 @@ public final class TestBugs extends BaseTestBugzillaIssues { assertEquals("[Formulas2.xls]Sheet1!B2", row.getCell(1).getCellFormula()); assertEquals(112.0, row.getCell(1).getNumericCellValue(), 0); - // TODO - Fix these so they work... - /*row = s.getRow(4); + row = s.getRow(4); assertEquals(CellType.FORMULA, row.getCell(1).getCellTypeEnum()); assertEquals("'[$http://gagravarr.org/FormulaRefs2.xls]Sheet1'!B2", row.getCell(1).getCellFormula()); assertEquals(123.0, row.getCell(1).getNumericCellValue(), 0); @@ -1880,7 +1888,7 @@ public final class TestBugs extends BaseTestBugzillaIssues { row = s.getRow(5); assertEquals(CellType.FORMULA, row.getCell(1).getCellTypeEnum()); assertEquals("'[$http://example.com/FormulaRefs.xls]Sheet1'!B1", row.getCell(1).getCellFormula()); - assertEquals(234.0, row.getCell(1).getNumericCellValue(), 0);*/ + assertEquals(234.0, row.getCell(1).getNumericCellValue(), 0); wb2.close(); }