diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 46b55c3ce..f0c5d67ac 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -50,6 +50,11 @@ Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx + 45365 - Handle more excel number formatting rules in FormatTrackingHSSFListener / XLS2CSVmra + 45373 - Improve the performance of HSSFSheet.shiftRows + 45367 - Fixed bug when last row removed from sheet is row zero + 45348 - Tweaks to RVA formula logic + 45354 - Fixed recognition of named ranges within formulas 45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts 45336 - Fix HSSFColor.getTripletHash() 45334 - Fixed formula parser to handle dots in identifiers diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 31427e939..693fc42e2 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -47,6 +47,11 @@ Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx + 45365 - Handle more excel number formatting rules in FormatTrackingHSSFListener / XLS2CSVmra + 45373 - Improve the performance of HSSFSheet.shiftRows + 45367 - Fixed bug when last row removed from sheet is row zero + 45348 - Tweaks to RVA formula logic + 45354 - Fixed recognition of named ranges within formulas 45338 - Fix HSSFWorkbook to give you the same HSSFFont every time, and then fix it to find newly added fonts 45336 - Fix HSSFColor.getTripletHash() 45334 - Fixed formula parser to handle dots in identifiers diff --git a/src/examples/src/org/apache/poi/hssf/eventusermodel/examples/XLS2CSVmra.java b/src/examples/src/org/apache/poi/hssf/eventusermodel/examples/XLS2CSVmra.java index 1c9b22035..632eacf5c 100644 --- a/src/examples/src/org/apache/poi/hssf/eventusermodel/examples/XLS2CSVmra.java +++ b/src/examples/src/org/apache/poi/hssf/eventusermodel/examples/XLS2CSVmra.java @@ -20,10 +20,6 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintStream; -import java.text.DateFormat; -import java.text.DecimalFormat; -import java.text.SimpleDateFormat; -import java.util.Date; import org.apache.poi.hssf.eventusermodel.FormatTrackingHSSFListener; import org.apache.poi.hssf.eventusermodel.HSSFEventFactory; @@ -37,7 +33,6 @@ import org.apache.poi.hssf.model.FormulaParser; import org.apache.poi.hssf.record.BOFRecord; import org.apache.poi.hssf.record.BlankRecord; import org.apache.poi.hssf.record.BoolErrRecord; -import org.apache.poi.hssf.record.CellValueRecordInterface; import org.apache.poi.hssf.record.FormulaRecord; import org.apache.poi.hssf.record.LabelRecord; import org.apache.poi.hssf.record.LabelSSTRecord; @@ -47,7 +42,6 @@ import org.apache.poi.hssf.record.RKRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.SSTRecord; import org.apache.poi.hssf.record.StringRecord; -import org.apache.poi.hssf.usermodel.HSSFDateUtil; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.poifs.filesystem.POIFSFileSystem; @@ -180,7 +174,7 @@ public class XLS2CSVmra implements HSSFListener { nextRow = frec.getRow(); nextColumn = frec.getColumn(); } else { - thisStr = formatNumberDateCell(frec, frec.getValue()); + thisStr = formatListener.formatNumberDateCell(frec); } } else { thisStr = '"' + @@ -231,7 +225,7 @@ public class XLS2CSVmra implements HSSFListener { thisColumn = numrec.getColumn(); // Format - thisStr = formatNumberDateCell(numrec, numrec.getValue()); + thisStr = formatListener.formatNumberDateCell(numrec); break; case RKRecord.sid: RKRecord rkrec = (RKRecord) record; @@ -290,44 +284,6 @@ public class XLS2CSVmra implements HSSFListener { } } - /** - * Formats a number or date cell, be that a real number, or the - * answer to a formula - */ - private String formatNumberDateCell(CellValueRecordInterface cell, double value) { - // Get the built in format, if there is one - int formatIndex = formatListener.getFormatIndex(cell); - String formatString = formatListener.getFormatString(cell); - - if(formatString == null) { - return Double.toString(value); - } else { - // Is it a date? - if(HSSFDateUtil.isADateFormat(formatIndex,formatString) && - HSSFDateUtil.isValidExcelDate(value)) { - // Java wants M not m for month - formatString = formatString.replace('m','M'); - // Change \- into -, if it's there - formatString = formatString.replaceAll("\\\\-","-"); - - // Format as a date - Date d = HSSFDateUtil.getJavaDate(value, false); - DateFormat df = new SimpleDateFormat(formatString); - return df.format(d); - } else { - if(formatString == "General") { - // Some sort of wierd default - return Double.toString(value); - } - - // Format as a number - DecimalFormat df = new DecimalFormat(formatString); - return df.format(value); - } - } - } - - public static void main(String[] args) throws Exception { if(args.length < 1) { System.err.println("Use:"); diff --git a/src/java/org/apache/poi/hssf/eventusermodel/FormatTrackingHSSFListener.java b/src/java/org/apache/poi/hssf/eventusermodel/FormatTrackingHSSFListener.java index b88143713..5a84f4564 100644 --- a/src/java/org/apache/poi/hssf/eventusermodel/FormatTrackingHSSFListener.java +++ b/src/java/org/apache/poi/hssf/eventusermodel/FormatTrackingHSSFListener.java @@ -16,7 +16,11 @@ ==================================================================== */ package org.apache.poi.hssf.eventusermodel; +import java.text.DateFormat; +import java.text.DecimalFormat; +import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Date; import java.util.Hashtable; import java.util.List; import java.util.Map; @@ -24,8 +28,11 @@ import java.util.Map; import org.apache.poi.hssf.record.CellValueRecordInterface; import org.apache.poi.hssf.record.ExtendedFormatRecord; import org.apache.poi.hssf.record.FormatRecord; +import org.apache.poi.hssf.record.FormulaRecord; +import org.apache.poi.hssf.record.NumberRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.usermodel.HSSFDataFormat; +import org.apache.poi.hssf.usermodel.HSSFDateUtil; /** * A proxy HSSFListener that keeps track of the document @@ -69,6 +76,61 @@ public class FormatTrackingHSSFListener implements HSSFListener { } } + /** + * Formats the given numeric of date Cell's contents + * as a String, in as close as we can to the way + * that Excel would do so. + * Uses the various format records to manage this. + * + * TODO - move this to a central class in such a + * way that hssf.usermodel can make use of it too + */ + public String formatNumberDateCell(CellValueRecordInterface cell) { + double value; + if(cell instanceof NumberRecord) { + value = ((NumberRecord)cell).getValue(); + } else if(cell instanceof FormulaRecord) { + value = ((FormulaRecord)cell).getValue(); + } else { + throw new IllegalArgumentException("Unsupported CellValue Record passed in " + cell); + } + + // Get the built in format, if there is one + int formatIndex = getFormatIndex(cell); + String formatString = getFormatString(cell); + + if(formatString == null) { + return Double.toString(value); + } else { + // Is it a date? + if(HSSFDateUtil.isADateFormat(formatIndex,formatString) && + HSSFDateUtil.isValidExcelDate(value)) { + // Java wants M not m for month + formatString = formatString.replace('m','M'); + // Change \- into -, if it's there + formatString = formatString.replaceAll("\\\\-","-"); + + // Format as a date + Date d = HSSFDateUtil.getJavaDate(value, false); + DateFormat df = new SimpleDateFormat(formatString); + return df.format(d); + } else { + if(formatString == "General") { + // Some sort of wierd default + return Double.toString(value); + } + if(formatString == "0.00E+00") { + // This seems to mean output as a normal double + return Double.toString(value); + } + + // Format as a number + DecimalFormat df = new DecimalFormat(formatString); + return df.format(value); + } + } + } + /** * Returns the format string, eg $##.##, for the * given number format index. diff --git a/src/java/org/apache/poi/hssf/model/FormulaParser.java b/src/java/org/apache/poi/hssf/model/FormulaParser.java index 2cc2a8053..afe7de239 100644 --- a/src/java/org/apache/poi/hssf/model/FormulaParser.java +++ b/src/java/org/apache/poi/hssf/model/FormulaParser.java @@ -29,6 +29,7 @@ import org.apache.poi.hssf.record.formula.function.FunctionMetadataRegistry; import org.apache.poi.ss.usermodel.Workbook; import org.apache.poi.hssf.util.AreaReference; import org.apache.poi.hssf.util.CellReference; +import org.apache.poi.hssf.util.CellReference.NameType; /** * This class parses a formula string into a List of tokens in RPN order. @@ -293,9 +294,14 @@ public final class FormulaParser { // This can be either a cell ref or a named range // Try to spot which it is - if (isValidCellReference(name)) { + int nameType = CellReference.classifyCellReference(name); + if (nameType == NameType.CELL) { return new RefPtg(name); } + if (nameType != NameType.NAMED_RANGE) { + new FormulaParseException("Name '" + name + + "' does not look like a cell reference or named range"); + } for(int i = 0; i < book.getNumberOfNames(); i++) { // named range name matching is case insensitive @@ -303,11 +309,12 @@ public final class FormulaParser { return new NamePtg(name, book); } } - throw new FormulaParseException("Found reference to named range \"" - + name + "\", but that named range wasn't defined!"); + throw new FormulaParseException("Specified named range '" + + name + "' does not exist in the current workbook."); } /** + * @param name an 'identifier' like string (i.e. contains alphanums, and dots) * @return null if name cannot be split at a dot */ private AreaReference parseArea(String name) { @@ -323,6 +330,8 @@ public final class FormulaParser { return null; } } + // This expression is only valid as an area ref, if the LHS and RHS of the dot(s) are both + // cell refs. Otherwise, this expression must be a named range name String partA = name.substring(0, dotPos); if (!isValidCellReference(partA)) { return null; @@ -336,12 +345,14 @@ public final class FormulaParser { return new AreaReference(topLeft, bottomRight); } + /** + * @return true if the specified name is a valid cell reference + */ private static boolean isValidCellReference(String str) { - // TODO - exact rules for recognising cell references may be too complicated for regex - return CELL_REFERENCE_PATTERN.matcher(str).matches(); + return CellReference.classifyCellReference(str) == NameType.CELL; } - - + + /** * Note - Excel function names are 'case aware but not case sensitive'. This method may end * up creating a defined name record in the workbook if the specified name is not an internal diff --git a/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java b/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java index 5358324a3..07d2bd2fd 100644 --- a/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java +++ b/src/java/org/apache/poi/hssf/model/OperandClassTransformer.java @@ -71,11 +71,16 @@ final class OperandClassTransformer { + _formulaType + ") not supported yet"); } - transformNode(rootNode, rootNodeOperandClass, false); + transformNode(rootNode, rootNodeOperandClass, false, false); } + /** + * @param callerForceArrayFlag true if one of the current node's parents is a + * function Ptg which has been changed from default 'V' to 'A' type (due to requirements on + * the function return value). + */ private void transformNode(ParseNode node, byte desiredOperandClass, - boolean callerForceArrayFlag) { + boolean callerForceArrayFlag, boolean isDirectChildOfValueOperator) { Ptg token = node.getToken(); ParseNode[] children = node.getChildren(); if (token instanceof ValueOperatorPtg || token instanceof ControlPtg) { @@ -84,7 +89,7 @@ final class OperandClassTransformer { // but any child nodes are processed according to desiredOperandClass and callerForceArrayFlag for (int i = 0; i < children.length; i++) { ParseNode child = children[i]; - transformNode(child, desiredOperandClass, callerForceArrayFlag); + transformNode(child, desiredOperandClass, callerForceArrayFlag, true); } return; } @@ -101,22 +106,34 @@ final class OperandClassTransformer { // nothing to do return; } - if (callerForceArrayFlag) { - switch (desiredOperandClass) { - case Ptg.CLASS_VALUE: - case Ptg.CLASS_ARRAY: - token.setClass(Ptg.CLASS_ARRAY); - break; - case Ptg.CLASS_REF: - token.setClass(Ptg.CLASS_REF); - break; - default: - throw new IllegalStateException("Unexpected operand class (" - + desiredOperandClass + ")"); - } - } else { - token.setClass(desiredOperandClass); - } + if (isDirectChildOfValueOperator) { + // As per OOO documentation Sec 3.2.4 "Token Class Transformation", "Step 1" + // All direct operands of value operators that are initially 'R' type will + // be converted to 'V' type. + if (token.getPtgClass() == Ptg.CLASS_REF) { + token.setClass(Ptg.CLASS_VALUE); + } + } + token.setClass(transformClass(token.getPtgClass(), desiredOperandClass, callerForceArrayFlag)); + } + + private byte transformClass(byte currentOperandClass, byte desiredOperandClass, + boolean callerForceArrayFlag) { + switch (desiredOperandClass) { + case Ptg.CLASS_VALUE: + if (!callerForceArrayFlag) { + return Ptg.CLASS_VALUE; + } + // else fall through + case Ptg.CLASS_ARRAY: + return Ptg.CLASS_ARRAY; + case Ptg.CLASS_REF: + if (!callerForceArrayFlag) { + return currentOperandClass; + } + return Ptg.CLASS_REF; + } + throw new IllegalStateException("Unexpected operand class (" + desiredOperandClass + ")"); } private void transformFunctionNode(AbstractFunctionPtg afp, ParseNode[] children, @@ -200,7 +217,7 @@ final class OperandClassTransformer { for (int i = 0; i < children.length; i++) { ParseNode child = children[i]; byte paramOperandClass = afp.getParameterClass(i); - transformNode(child, paramOperandClass, localForceArrayFlag); + transformNode(child, paramOperandClass, localForceArrayFlag, false); } } } diff --git a/src/java/org/apache/poi/hssf/record/formula/SheetNameFormatter.java b/src/java/org/apache/poi/hssf/record/formula/SheetNameFormatter.java index 8e47cbe7a..ace857da1 100755 --- a/src/java/org/apache/poi/hssf/record/formula/SheetNameFormatter.java +++ b/src/java/org/apache/poi/hssf/record/formula/SheetNameFormatter.java @@ -14,13 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. ==================================================================== */ - package org.apache.poi.hssf.record.formula; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.poi.hssf.util.CellReference; + /** * Formats sheet names for use in formula expressions. * @@ -28,14 +29,12 @@ import java.util.regex.Pattern; */ public final class SheetNameFormatter { - private static final String BIFF8_LAST_COLUMN = "IV"; - private static final int BIFF8_LAST_COLUMN_TEXT_LEN = BIFF8_LAST_COLUMN.length(); - private static final String BIFF8_LAST_ROW = String.valueOf(0x10000); - private static final int BIFF8_LAST_ROW_TEXT_LEN = BIFF8_LAST_ROW.length(); - private static final char DELIMITER = '\''; - private static final Pattern CELL_REF_PATTERN = Pattern.compile("([A-Za-z])+[0-9]+"); + /** + * Matches a single cell ref with no absolute ('$') markers + */ + private static final Pattern CELL_REF_PATTERN = Pattern.compile("([A-Za-z]+)([0-9]+)"); private SheetNameFormatter() { // no instances of this class @@ -105,27 +104,27 @@ public final class SheetNameFormatter { return false; } - /** - * @return true if the presence of the specified character in a sheet name would - * require the sheet name to be delimited in formulas. This includes every non-alphanumeric - * character besides underscore '_'. - */ - /* package */ static boolean isSpecialChar(char ch) { - // note - Character.isJavaIdentifierPart() would allow dollars '$' - if(Character.isLetterOrDigit(ch)) { - return false; - } - switch(ch) { - case '_': // underscore is ok - return false; - case '\n': - case '\r': - case '\t': - throw new RuntimeException("Illegal character (0x" - + Integer.toHexString(ch) + ") found in sheet name"); - } - return true; - } + /** + * @return true if the presence of the specified character in a sheet name would + * require the sheet name to be delimited in formulas. This includes every non-alphanumeric + * character besides underscore '_'. + */ + /* package */ static boolean isSpecialChar(char ch) { + // note - Character.isJavaIdentifierPart() would allow dollars '$' + if(Character.isLetterOrDigit(ch)) { + return false; + } + switch(ch) { + case '_': // underscore is ok + return false; + case '\n': + case '\r': + case '\t': + throw new RuntimeException("Illegal character (0x" + + Integer.toHexString(ch) + ") found in sheet name"); + } + return true; + } /** @@ -149,64 +148,11 @@ public final class SheetNameFormatter { *

* For better or worse this implementation attempts to replicate Excel's formula renderer. * Excel uses range checking on the apparent 'row' and 'column' components. Note however that - * the maximum sheet size varies across versions: - *

- *

- * - * - * - * - *
Version  File Format  Last Column  Last Row
97-2003BIFF8"IV" (2^8)65536 (2^14)
2007BIFF12"XFD" (2^14)1048576 (2^20)
- * POI currently targets BIFF8 (Excel 97-2003), so the following behaviour can be observed for - * this method: - *
- * - * - * - * - * - * - * - * - * - * - * - *
Input           Result 
"A1", 1true
"a111", 1true
"A65536", 1true
"A65537", 1false
"iv1", 2true
"IW1", 2false
"AAA1", 3false
"a111", 1true
"Sheet1", 6false
+ * the maximum sheet size varies across versions. + * @see org.apache.poi.hssf.util.CellReference */ - /* package */ static boolean cellReferenceIsWithinRange(String rawSheetName, int numberOfLetters) { - - if(numberOfLetters > BIFF8_LAST_COLUMN_TEXT_LEN) { - // "Sheet1" case etc - return false; // that was easy - } - int nDigits = rawSheetName.length() - numberOfLetters; - if(nDigits > BIFF8_LAST_ROW_TEXT_LEN) { - return false; - } - if(numberOfLetters == BIFF8_LAST_COLUMN_TEXT_LEN) { - String colStr = rawSheetName.substring(0, BIFF8_LAST_COLUMN_TEXT_LEN).toUpperCase(); - if(colStr.compareTo(BIFF8_LAST_COLUMN) > 0) { - return false; - } - } else { - // apparent column name has less chars than max - // no need to check range - } - - if(nDigits == BIFF8_LAST_ROW_TEXT_LEN) { - String colStr = rawSheetName.substring(numberOfLetters); - // ASCII comparison is valid if digit count is same - if(colStr.compareTo(BIFF8_LAST_ROW) > 0) { - return false; - } - } else { - // apparent row has less chars than max - // no need to check range - } - - return true; + /* package */ static boolean cellReferenceIsWithinRange(String lettersPrefix, String numbersSuffix) { + return CellReference.cellReferenceIsWithinRange(lettersPrefix, numbersSuffix); } /** @@ -239,7 +185,7 @@ public final class SheetNameFormatter { // rawSheetName == "Sheet1" gets this far. String lettersPrefix = matcher.group(1); - return cellReferenceIsWithinRange(rawSheetName, lettersPrefix.length()); + String numbersSuffix = matcher.group(2); + return cellReferenceIsWithinRange(lettersPrefix, numbersSuffix); } - } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java b/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java index 3e95aaee4..0ddac1f6d 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java @@ -187,6 +187,19 @@ public final class HSSFRow implements Comparable, Row { row.setFirstCol(findFirstCell(row.getFirstCol())); } } + + /** + * Removes all the cells from the row, and their + * records too. + */ + protected void removeAllCells() { + for(int i=0; i 0) - { + while (r == null && rownum > 0) { r = getRow(--rownum); } - if (r == null) - return -1; + if (r == null) { + return -1; + } return rownum; } @@ -1224,6 +1225,28 @@ public class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet * @param resetOriginalRowHeight whether to set the original row's height to the default */ public void shiftRows( int startRow, int endRow, int n, boolean copyRowHeight, boolean resetOriginalRowHeight) + { + shiftRows(startRow, endRow, n, copyRowHeight, resetOriginalRowHeight, true); + } + + /** + * Shifts rows between startRow and endRow n number of rows. + * If you use a negative number, it will shift rows up. + * Code ensures that rows don't wrap around + * + *

+ * Additionally shifts merged regions that are completely defined in these + * rows (ie. merged 2 cells on a row to be shifted). + *

+ * TODO Might want to add bounds checking here + * @param startRow the row to start shifting + * @param endRow the row to end shifting + * @param n the number of rows to shift + * @param copyRowHeight whether to copy the row height during the shift + * @param resetOriginalRowHeight whether to set the original row's height to the default + * @param moveComments whether to move comments at the same time as the cells they are attached to + */ + public void shiftRows( int startRow, int endRow, int n, boolean copyRowHeight, boolean resetOriginalRowHeight, boolean moveComments) { int s, e, inc; if ( n < 0 ) @@ -1250,44 +1273,55 @@ public class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet row2Replace = createRow( rowNum + n ); HSSFCell cell; + + // Remove all the old cells from the row we'll + // be writing too, before we start overwriting + // any cells. This avoids issues with cells + // changing type, and records not being correctly + // overwritten + row2Replace.removeAllCells(); + // If this row doesn't exist, nothing needs to + // be done for the now empty destination row + if (row == null) continue; // Nothing to do for this row + // Fetch the first and last columns of the + // row now, so we still have them to hand + // once we start removing cells + short firstCol = row.getFirstCellNum(); + short lastCol = row.getLastCellNum(); - - // Removes the cells before over writting them. - for ( short col = row2Replace.getFirstCellNum(); col <= row2Replace.getLastCellNum(); col++ ) - { - cell = row2Replace.getCell( col ); - if ( cell != null ) - row2Replace.removeCell( cell ); + // Fix up row heights if required + if (copyRowHeight) { + row2Replace.setHeight(row.getHeight()); + } + if (resetOriginalRowHeight) { + row.setHeight((short)0xff); } - if (row == null) continue; // Nothing to do for this row - else { - if (copyRowHeight) { - row2Replace.setHeight(row.getHeight()); - } - if (resetOriginalRowHeight) { - row.setHeight((short)0xff); - } - } - for ( short col = row.getFirstCellNum(); col <= row.getLastCellNum(); col++ ) - { - cell = row.getCell( col ); - if ( cell != null ) - { - row.removeCell( cell ); - CellValueRecordInterface cellRecord = cell.getCellValueRecord(); - cellRecord.setRow( rowNum + n ); - row2Replace.createCellFromRecord( cellRecord ); - sheet.addValueRecord( rowNum + n, cellRecord ); - } - - // move comments if exist (can exist even if cell is null) - HSSFComment comment = getCellComment(rowNum, col); - if (comment != null) { - comment.setRow(rowNum + n); - } + // Copy each cell from the source row to + // the destination row + for(Iterator cells = row.cellIterator(); cells.hasNext(); ) { + cell = (HSSFCell)cells.next(); + row.removeCell( cell ); + CellValueRecordInterface cellRecord = cell.getCellValueRecord(); + cellRecord.setRow( rowNum + n ); + row2Replace.createCellFromRecord( cellRecord ); + sheet.addValueRecord( rowNum + n, cellRecord ); + } + // Now zap all the cells in the source row + row.removeAllCells(); + + // Move comments from the source row to the + // destination row. Note that comments can + // exist for cells which are null + if(moveComments) { + for( short col = firstCol; col <= lastCol; col++ ) { + HSSFComment comment = getCellComment(rowNum, col); + if (comment != null) { + comment.setRow(rowNum + n); + } + } } } if ( endRow == lastrow || endRow + n > lastrow ) lastrow = Math.min( endRow + n, 65535 ); diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index b80ccb790..09d66e1f6 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -376,28 +376,28 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm log.log(POILogger.DEBUG, "convertLabelRecords exit"); } - /** - * Retrieves the current policy on what to do when - * getting missing or blank cells from a row. - * The default is to return blank and null cells. - * {@link MissingCellPolicy} - */ - public MissingCellPolicy getMissingCellPolicy() { - return missingCellPolicy; - } + /** + * Retrieves the current policy on what to do when + * getting missing or blank cells from a row. + * The default is to return blank and null cells. + * {@link MissingCellPolicy} + */ + public MissingCellPolicy getMissingCellPolicy() { + return missingCellPolicy; + } - /** - * Sets the policy on what to do when - * getting missing or blank cells from a row. - * This will then apply to all calls to - * {@link Row.getCell()}. See - * {@link MissingCellPolicy} - */ - public void setMissingCellPolicy(MissingCellPolicy missingCellPolicy) { - this.missingCellPolicy = missingCellPolicy; - } + /** + * Sets the policy on what to do when + * getting missing or blank cells from a row. + * This will then apply to all calls to + * {@link HSSFRow.getCell()}. See + * {@link MissingCellPolicy} + */ + public void setMissingCellPolicy(MissingCellPolicy missingCellPolicy) { + this.missingCellPolicy = missingCellPolicy; + } - /** + /** * sets the order of appearance for a given sheet. * * @param sheetname the name of the sheet to reorder @@ -1041,11 +1041,11 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm String name, boolean italic, boolean strikeout, short typeOffset, byte underline) { - for (short i=0; i<=getNumberOfFonts(); i++) { - // Remember - there is no 4! - if(i == 4) continue; - - HSSFFont hssfFont = getFontAt(i); + for (short i=0; i<=getNumberOfFonts(); i++) { + // Remember - there is no 4! + if(i == 4) continue; + + HSSFFont hssfFont = getFontAt(i); if (hssfFont.getBoldweight() == boldWeight && hssfFont.getColor() == color && hssfFont.getFontHeight() == fontHeight @@ -1077,19 +1077,17 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm * @param idx index number * @return HSSFFont at the index */ + public HSSFFont getFontAt(short idx) { + if(fonts == null) fonts = new Hashtable(); + + // So we don't confuse users, give them back + // the same object every time, but create + // them lazily + Short sIdx = new Short(idx); + if(fonts.containsKey(sIdx)) { + return (HSSFFont)fonts.get(sIdx); + } - public HSSFFont getFontAt(short idx) - { - if(fonts == null) fonts = new Hashtable(); - - // So we don't confuse users, give them back - // the same object every time, but create - // them lazily - Short sIdx = Short.valueOf(idx); - if(fonts.containsKey(sIdx)) { - return (HSSFFont)fonts.get(sIdx); - } - FontRecord font = workbook.getFontRecordAt(idx); HSSFFont retval = new HSSFFont(idx, font); fonts.put(sIdx, retval); diff --git a/src/java/org/apache/poi/hssf/util/CellReference.java b/src/java/org/apache/poi/hssf/util/CellReference.java index c63e7bc99..0ddb0764e 100644 --- a/src/java/org/apache/poi/hssf/util/CellReference.java +++ b/src/java/org/apache/poi/hssf/util/CellReference.java @@ -25,6 +25,15 @@ package org.apache.poi.hssf.util; * @author Dennis Doubleday (patch to seperateRowColumns()) */ public final class CellReference extends org.apache.poi.ss.util.CellReference { + /** + * Used to classify identifiers found in formulas as cell references or not. + */ + public static final class NameType { + public static final int CELL = 1; + public static final int NAMED_RANGE = 2; + public static final int BAD_CELL_OR_NAMED_RANGE = -1; + } + /** * Create an cell ref from a string representation. Sheet names containing special characters should be * delimited and escaped as per normal syntax rules for formulas. @@ -33,8 +42,15 @@ public final class CellReference extends org.apache.poi.ss.util.CellReference { super(cellRef); } + public CellReference(int pRow, int pCol) { + super(pRow, pCol, true, true); + } + public CellReference(int pRow, short pCol) { + super(pRow, (int)pCol, true, true); + } + public CellReference(int pRow, int pCol, boolean pAbsRow, boolean pAbsCol) { - this(null, pRow, pCol, pAbsRow, pAbsCol); + super(null, pRow, pCol, pAbsRow, pAbsCol); } public CellReference(String pSheetName, int pRow, int pCol, boolean pAbsRow, boolean pAbsCol) { super(pSheetName, pRow, pCol, pAbsRow, pAbsCol); diff --git a/src/java/org/apache/poi/ss/util/CellReference.java b/src/java/org/apache/poi/ss/util/CellReference.java index 4f90311f3..b46cd88cc 100644 --- a/src/java/org/apache/poi/ss/util/CellReference.java +++ b/src/java/org/apache/poi/ss/util/CellReference.java @@ -17,17 +17,26 @@ package org.apache.poi.ss.util; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.apache.poi.hssf.record.formula.SheetNameFormatter; /** - * Common convertion functions between Excel style A1, C27 style - * cell references, and POI usermodel style row=0, column=0 - * style references. - * Applys for both HSSF and XSSF. + * * @author Avik Sengupta * @author Dennis Doubleday (patch to seperateRowColumns()) */ public class CellReference { + /** + * Used to classify identifiers found in formulas as cell references or not. + */ + public static final class NameType { + public static final int CELL = 1; + public static final int NAMED_RANGE = 2; + public static final int BAD_CELL_OR_NAMED_RANGE = -1; + } + /** The character ($) that signifies a row or column value is absolute instead of relative */ private static final char ABSOLUTE_REFERENCE_MARKER = '$'; /** The character (!) that separates sheet names from cell references */ @@ -35,6 +44,20 @@ public class CellReference { /** The character (') used to quote sheet names when they contain special characters */ private static final char SPECIAL_NAME_DELIMITER = '\''; + /** + * Matches a run of letters followed by a run of digits. The run of letters is group 1 and the + * run of digits is group 2. Each group may optionally be prefixed with a single '$'. + */ + private static final Pattern CELL_REF_PATTERN = Pattern.compile("\\$?([A-Za-z]+)\\$?([0-9]+)"); + /** + * Named range names must start with a letter or underscore. Subsequent characters may include + * digits or dot. (They can even end in dot). + */ + private static final Pattern NAMED_RANGE_NAME_PATTERN = Pattern.compile("[_A-Za-z][_.A-Za-z0-9]*"); + private static final String BIFF8_LAST_COLUMN = "IV"; + private static final int BIFF8_LAST_COLUMN_TEXT_LEN = BIFF8_LAST_COLUMN.length(); + private static final String BIFF8_LAST_ROW = String.valueOf(0x10000); + private static final int BIFF8_LAST_ROW_TEXT_LEN = BIFF8_LAST_ROW.length(); private final int _rowIndex; private final int _colIndex; @@ -70,13 +93,13 @@ public class CellReference { _rowIndex = Integer.parseInt(rowRef)-1; // -1 to convert 1-based to zero-based } - /** - * Creates a cell reference for the given row and cell. - * Assumes these references are relative - */ - public CellReference(int row, int col) { - this(row, col, false, false); + public CellReference(int pRow, int pCol) { + this(pRow, pCol, false, false); } + public CellReference(int pRow, short pCol) { + this(pRow, (int)pCol, false, false); + } + public CellReference(int pRow, int pCol, boolean pAbsRow, boolean pAbsCol) { this(null, pRow, pCol, pAbsRow, pAbsCol); } @@ -97,7 +120,7 @@ public class CellReference { } public int getRow(){return _rowIndex;} - public int getCol(){return _colIndex;} + public short getCol(){return (short) _colIndex;} public boolean isRowAbsolute(){return _isRowAbs;} public boolean isColAbsolute(){return _isColAbs;} /** @@ -111,27 +134,148 @@ public class CellReference { /** * takes in a column reference portion of a CellRef and converts it from * ALPHA-26 number format to 0-based base 10. - * ALPHA-26 goes A to Z, then AA to AZ, BA to BZ, ..., ZA to ZZ, - * AAA to AAZ, ABA to ABZ, ..., AZA to AZZ, BAA to BAZ etc */ private int convertColStringToNum(String ref) { - int lastIx = ref.length()-1; - int retval=0; - int pos = 0; - - for (int k = lastIx; k > -1; k--) { - char thechar = ref.charAt(k); - // Character.getNumericValue() returns the values - // 10-35 for the letter A-Z - int shift = (int)Math.pow(26, pos); - retval += (Character.getNumericValue(thechar)-9) * shift; - pos++; - } - return retval-1; + int lastIx = ref.length()-1; + int retval=0; + int pos = 0; + + for (int k = lastIx; k > -1; k--) { + char thechar = ref.charAt(k); + // Character.getNumericValue() returns the values + // 10-35 for the letter A-Z + int shift = (int)Math.pow(26, pos); + retval += (Character.getNumericValue(thechar)-9) * shift; + pos++; + } + return retval-1; } - /** + * Classifies an identifier as either a simple (2D) cell reference or a named range name + * @return one of the values from NameType + */ + public static int classifyCellReference(String str) { + int len = str.length(); + if (len < 1) { + throw new IllegalArgumentException("Empty string not allowed"); + } + char firstChar = str.charAt(0); + switch (firstChar) { + case ABSOLUTE_REFERENCE_MARKER: + case '.': + case '_': + break; + default: + if (!Character.isLetter(firstChar)) { + throw new IllegalArgumentException("Invalid first char (" + firstChar + + ") of cell reference or named range. Letter expected"); + } + } + if (!Character.isDigit(str.charAt(len-1))) { + // no digits at end of str + return validateNamedRangeName(str); + } + Matcher cellRefPatternMatcher = CELL_REF_PATTERN.matcher(str); + if (!cellRefPatternMatcher.matches()) { + return validateNamedRangeName(str); + } + String lettersGroup = cellRefPatternMatcher.group(1); + String digitsGroup = cellRefPatternMatcher.group(2); + if (cellReferenceIsWithinRange(lettersGroup, digitsGroup)) { + // valid cell reference + return NameType.CELL; + } + // If str looks like a cell reference, but is out of (row/col) range, it is a valid + // named range name + // This behaviour is a little weird. For example, "IW123" is a valid named range name + // because the column "IW" is beyond the maximum "IV". Note - this behaviour is version + // dependent. In Excel 2007, "IW123" is not a valid named range name. + if (str.indexOf(ABSOLUTE_REFERENCE_MARKER) >= 0) { + // Of course, named range names cannot have '$' + return NameType.BAD_CELL_OR_NAMED_RANGE; + } + return NameType.NAMED_RANGE; + } + + private static int validateNamedRangeName(String str) { + if (!NAMED_RANGE_NAME_PATTERN.matcher(str).matches()) { + return NameType.BAD_CELL_OR_NAMED_RANGE; + } + return NameType.NAMED_RANGE; + + } + + + /** + * Used to decide whether a name of the form "[A-Z]*[0-9]*" that appears in a formula can be + * interpreted as a cell reference. Names of that form can be also used for sheets and/or + * named ranges, and in those circumstances, the question of whether the potential cell + * reference is valid (in range) becomes important. + *

+ * Note - that the maximum sheet size varies across Excel versions: + *

+ *

+ * + * + * + * + *
Version  File Format  Last Column  Last Row
97-2003BIFF8"IV" (2^8)65536 (2^14)
2007BIFF12"XFD" (2^14)1048576 (2^20)
+ * POI currently targets BIFF8 (Excel 97-2003), so the following behaviour can be observed for + * this method: + *
+ * + * + * + * + * + * + * + * + * + * + * + *
Input           Result 
"A", "1"true
"a", "111"true
"A", "65536"true
"A", "65537"false
"iv", "1"true
"IW", "1"false
"AAA", "1"false
"a", "111"true
"Sheet", "1"false
+ * + * @param colStr a string of only letter characters + * @param rowStr a string of only digit characters + * @return true if the row and col parameters are within range of a BIFF8 spreadsheet. + */ + public static boolean cellReferenceIsWithinRange(String colStr, String rowStr) { + int numberOfLetters = colStr.length(); + if(numberOfLetters > BIFF8_LAST_COLUMN_TEXT_LEN) { + // "Sheet1" case etc + return false; // that was easy + } + int nDigits = rowStr.length(); + if(nDigits > BIFF8_LAST_ROW_TEXT_LEN) { + return false; + } + if(numberOfLetters == BIFF8_LAST_COLUMN_TEXT_LEN) { + if(colStr.toUpperCase().compareTo(BIFF8_LAST_COLUMN) > 0) { + return false; + } + } else { + // apparent column name has less chars than max + // no need to check range + } + + if(nDigits == BIFF8_LAST_ROW_TEXT_LEN) { + // ASCII comparison is valid if digit count is same + if(rowStr.compareTo(BIFF8_LAST_ROW) > 0) { + return false; + } + } else { + // apparent row has less chars than max + // no need to check range + } + + return true; + } + + /** * Separates the row from the columns and returns an array of three Strings. The first element * is the sheet name. Only the first element may be null. The second element in is the column * name still in ALPHA-26 number format. The third element is the row. @@ -212,24 +356,24 @@ public class CellReference { * eg column #3 -> D */ protected static String convertNumToColString(int col) { - // Excel counts column A as the 1st column, we - // treat it as the 0th one - int excelColNum = col + 1; - - String colRef = ""; - int colRemain = excelColNum; - - while(colRemain > 0) { - int thisPart = colRemain % 26; - if(thisPart == 0) { thisPart = 26; } - colRemain = (colRemain - thisPart) / 26; - - // The letter A is at 65 - char colChar = (char)(thisPart+64); - colRef = colChar + colRef; - } - - return colRef; + // Excel counts column A as the 1st column, we + // treat it as the 0th one + int excelColNum = col + 1; + + String colRef = ""; + int colRemain = excelColNum; + + while(colRemain > 0) { + int thisPart = colRemain % 26; + if(thisPart == 0) { thisPart = 26; } + colRemain = (colRemain - thisPart) / 26; + + // The letter A is at 65 + char colChar = (char)(thisPart+64); + colRef = colChar + colRef; + } + + return colRef; } /** @@ -260,21 +404,21 @@ public class CellReference { return sb.toString(); } - /** - * Returns the three parts of the cell reference, the - * Sheet name (or null if none supplied), the 1 based - * row number, and the A based column letter. - * This will not include any markers for absolute - * references, so use {@link #formatAsString()} - * to properly turn references into strings. - */ - public String[] getCellRefParts() { - return new String[] { - _sheetName, - Integer.toString(_rowIndex+1), - convertNumToColString(_colIndex) - }; - } + /** + * Returns the three parts of the cell reference, the + * Sheet name (or null if none supplied), the 1 based + * row number, and the A based column letter. + * This will not include any markers for absolute + * references, so use {@link #formatAsString()} + * to properly turn references into strings. + */ + public String[] getCellRefParts() { + return new String[] { + _sheetName, + Integer.toString(_rowIndex+1), + convertNumToColString(_colIndex) + }; + } /** * Appends cell reference with '$' markers for absolute values as required. diff --git a/src/testcases/org/apache/poi/hssf/HSSFTests.java b/src/testcases/org/apache/poi/hssf/HSSFTests.java index 0a4fa66af..ff15491d9 100644 --- a/src/testcases/org/apache/poi/hssf/HSSFTests.java +++ b/src/testcases/org/apache/poi/hssf/HSSFTests.java @@ -22,6 +22,7 @@ import junit.framework.TestSuite; import org.apache.poi.hssf.eventmodel.TestEventRecordFactory; import org.apache.poi.hssf.eventmodel.TestModelFactory; +import org.apache.poi.hssf.eventusermodel.AllEventUserModelTests; import org.apache.poi.hssf.model.AllModelTests; import org.apache.poi.hssf.record.AllRecordTests; import org.apache.poi.hssf.usermodel.AllUserModelTests; @@ -48,6 +49,7 @@ public final class HSSFTests { TestSuite suite = new TestSuite("Tests for org.apache.poi.hssf"); // $JUnit-BEGIN$ + suite.addTest(AllEventUserModelTests.suite()); suite.addTest(AllModelTests.suite()); suite.addTest(AllUserModelTests.suite()); suite.addTest(AllRecordTests.suite()); diff --git a/src/testcases/org/apache/poi/hssf/data/45365.xls b/src/testcases/org/apache/poi/hssf/data/45365.xls new file mode 100644 index 000000000..fbf1d97c5 Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/45365.xls differ diff --git a/src/testcases/org/apache/poi/hssf/data/testRVA.xls b/src/testcases/org/apache/poi/hssf/data/testRVA.xls index f23821117..17aa9fd71 100644 Binary files a/src/testcases/org/apache/poi/hssf/data/testRVA.xls and b/src/testcases/org/apache/poi/hssf/data/testRVA.xls differ diff --git a/src/testcases/org/apache/poi/hssf/eventusermodel/AllEventUserModelTests.java b/src/testcases/org/apache/poi/hssf/eventusermodel/AllEventUserModelTests.java new file mode 100644 index 000000000..41e3b8632 --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/eventusermodel/AllEventUserModelTests.java @@ -0,0 +1,38 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.hssf.eventusermodel; + +import junit.framework.Test; +import junit.framework.TestSuite; + +/** + * Collects all tests for org.apache.poi.hssf.eventusermodel. + * + * @author Josh Micich + */ +public class AllEventUserModelTests { + + public static Test suite() { + TestSuite result = new TestSuite(AllEventUserModelTests.class.getName()); + result.addTestSuite(TestEventWorkbookBuilder.class); + result.addTestSuite(TestFormatTrackingHSSFListener.class); + result.addTestSuite(TestHSSFEventFactory.class); + result.addTestSuite(TestMissingRecordAwareHSSFListener.class); + return result; + } +} diff --git a/src/testcases/org/apache/poi/hssf/eventusermodel/TestEventWorkbookBuilder.java b/src/testcases/org/apache/poi/hssf/eventusermodel/TestEventWorkbookBuilder.java index adf084331..215732deb 100644 --- a/src/testcases/org/apache/poi/hssf/eventusermodel/TestEventWorkbookBuilder.java +++ b/src/testcases/org/apache/poi/hssf/eventusermodel/TestEventWorkbookBuilder.java @@ -134,7 +134,7 @@ public final class TestEventWorkbookBuilder extends TestCase { fr = (FormulaRecord)mockListen._frecs.get(5); assertEquals(6, fr.getRow()); assertEquals(0, fr.getColumn()); - assertEquals("SUM('Sh3'!A1:A4)", FormulaParser.toFormulaString(stubHSSF, fr.getParsedExpression())); + assertEquals("SUM(Sh3!A1:A4)", FormulaParser.toFormulaString(stubHSSF, fr.getParsedExpression())); // Now, load via Usermodel and re-check @@ -142,7 +142,7 @@ public final class TestEventWorkbookBuilder extends TestCase { POIFSFileSystem fs = new POIFSFileSystem(is); HSSFWorkbook wb = new HSSFWorkbook(fs); assertEquals("Sheet1!A1", wb.getSheetAt(0).getRow(1).getCell(0).getCellFormula()); - assertEquals("SUM('Sh3'!A1:A4)", wb.getSheetAt(0).getRow(6).getCell(0).getCellFormula()); + assertEquals("SUM(Sh3!A1:A4)", wb.getSheetAt(0).getRow(6).getCell(0).getCellFormula()); } private static final class MockHSSFListener implements HSSFListener { diff --git a/src/testcases/org/apache/poi/hssf/eventusermodel/TestFormatTrackingHSSFListener.java b/src/testcases/org/apache/poi/hssf/eventusermodel/TestFormatTrackingHSSFListener.java index e52a3bc96..60f5d2ca7 100644 --- a/src/testcases/org/apache/poi/hssf/eventusermodel/TestFormatTrackingHSSFListener.java +++ b/src/testcases/org/apache/poi/hssf/eventusermodel/TestFormatTrackingHSSFListener.java @@ -24,6 +24,9 @@ import java.util.List; import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.record.CellValueRecordInterface; +import org.apache.poi.hssf.record.FormulaRecord; +import org.apache.poi.hssf.record.NumberRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.poifs.filesystem.POIFSFileSystem; /** @@ -31,16 +34,17 @@ import org.apache.poi.poifs.filesystem.POIFSFileSystem; */ public final class TestFormatTrackingHSSFListener extends TestCase { private FormatTrackingHSSFListener listener; - - public void setUp() { + private MockHSSFListener mockListen; + + private void processFile(String filename) throws Exception { HSSFRequest req = new HSSFRequest(); - MockHSSFListener mockListen = new MockHSSFListener(); + mockListen = new MockHSSFListener(); listener = new FormatTrackingHSSFListener(mockListen); req.addListenerForAllRecords(listener); HSSFEventFactory factory = new HSSFEventFactory(); try { - InputStream is = HSSFTestDataSamples.openSampleFileStream("MissingBits.xls"); + InputStream is = HSSFTestDataSamples.openSampleFileStream(filename); POIFSFileSystem fs = new POIFSFileSystem(is); factory.processWorkbookEvents(req, fs); } catch (IOException e) { @@ -49,11 +53,42 @@ public final class TestFormatTrackingHSSFListener extends TestCase { } public void testFormats() throws Exception { + processFile("MissingBits.xls"); + assertEquals("_(*#,##0_);_(*(#,##0);_(* \"-\"_);_(@_)", listener.getFormatString(41)); assertEquals("_($*#,##0_);_($*(#,##0);_($* \"-\"_);_(@_)", listener.getFormatString(42)); assertEquals("_(*#,##0.00_);_(*(#,##0.00);_(*\"-\"??_);_(@_)", listener.getFormatString(43)); } + /** + * Ensure that all number and formula records can be + * turned into strings without problems + */ + public void testTurnToString() throws Exception { + processFile("45365.xls"); + + for(int i=0; i 0); + } + } + + // TODO - test some specific format strings + } + private static final class MockHSSFListener implements HSSFListener { public MockHSSFListener() {} private final List _records = new ArrayList(); diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java index 115a7e081..a21850887 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java +++ b/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java @@ -46,6 +46,7 @@ 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.usermodel.HSSFCell; +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; @@ -791,4 +792,37 @@ public final class TestFormulaParser extends TestCase { assertEquals("ERROR.TYPE", funcPtg.getName()); } + public void testNamedRangeThatLooksLikeCell() { + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet sheet = wb.createSheet("Sheet1"); + HSSFName name = wb.createName(); + name.setReference("Sheet1!B1"); + name.setNameName("pfy1"); + + Ptg[] ptgs; + try { + ptgs = FormulaParser.parse("count(pfy1)", wb); + } catch (IllegalArgumentException e) { + if (e.getMessage().equals("Specified colIx (1012) is out of range")) { + throw new AssertionFailedError("Identified bug 45354"); + } + throw e; + } + assertEquals(2, ptgs.length); + assertEquals(NamePtg.class, ptgs[0].getClass()); + + HSSFCell cell = sheet.createRow(0).createCell((short)0); + cell.setCellFormula("count(pfy1)"); + assertEquals("COUNT(pfy1)", cell.getCellFormula()); + try { + cell.setCellFormula("count(pf1)"); + throw new AssertionFailedError("Expected formula parse execption"); + } catch (FormulaParseException e) { + if (!e.getMessage().equals("Specified named range 'pf1' does not exist in the current workbook.")) { + throw e; + } + } + cell.setCellFormula("count(fp1)"); // plain cell ref, col is in range + + } } diff --git a/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java b/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java index 90a5d8b13..47cfb574c 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java +++ b/src/testcases/org/apache/poi/hssf/model/TestOperandClassTransformer.java @@ -57,6 +57,31 @@ public final class TestOperandClassTransformer extends TestCase { confirmFuncClass(ptgs, 2, "INDEX", Ptg.CLASS_VALUE); } + /** + * Even though count expects args of type R, because A1 is a direct operand of a + * value operator it must get type V + */ + public void testDirectOperandOfValueOperator() { + String formula = "COUNT(A1*1)"; + Ptg[] ptgs = FormulaParser.parse(formula, null); + if (ptgs[0].getPtgClass() == Ptg.CLASS_REF) { + throw new AssertionFailedError("Identified bug 45348"); + } + + confirmTokenClass(ptgs, 0, Ptg.CLASS_VALUE); + confirmTokenClass(ptgs, 3, Ptg.CLASS_VALUE); + } + + /** + * A cell ref passed to a function expecting type V should be converted to type V + */ + public void testRtoV() { + + String formula = "lookup(A1, A3:A52, B3:B52)"; + Ptg[] ptgs = FormulaParser.parse(formula, null); + confirmTokenClass(ptgs, 0, Ptg.CLASS_VALUE); + } + public void testComplexIRR_bug45041() { String formula = "(1+IRR(SUMIF(A:A,ROW(INDIRECT(MIN(A:A)&\":\"&MAX(A:A))),B:B),0))^365-1"; Ptg[] ptgs = FormulaParser.parse(formula, null); @@ -89,8 +114,11 @@ public final class TestOperandClassTransformer extends TestCase { private void confirmTokenClass(Ptg[] ptgs, int i, byte operandClass) { Ptg ptg = ptgs[i]; + if (ptg.isBaseToken()) { + throw new AssertionFailedError("ptg[" + i + "] is a base token"); + } if (operandClass != ptg.getPtgClass()) { - throw new AssertionFailedError("Wrong operand class for function ptg (" + throw new AssertionFailedError("Wrong operand class for ptg (" + ptg.toString() + "). Expected " + getOperandClassName(operandClass) + " but got " + getOperandClassName(ptg.getPtgClass())); } diff --git a/src/testcases/org/apache/poi/hssf/record/formula/TestSheetNameFormatter.java b/src/testcases/org/apache/poi/hssf/record/formula/TestSheetNameFormatter.java index 768c42958..369c09583 100644 --- a/src/testcases/org/apache/poi/hssf/record/formula/TestSheetNameFormatter.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/TestSheetNameFormatter.java @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. ==================================================================== */ - package org.apache.poi.hssf.record.formula; @@ -72,11 +71,14 @@ public final class TestSheetNameFormatter extends TestCase { confirmCellNameMatch("aa1", true); confirmCellNameMatch("A1A", false); confirmCellNameMatch("A1A1", false); + confirmCellNameMatch("Sh3", false); confirmCellNameMatch("SALES20080101", false); // out of range } private static void confirmCellRange(String text, int numberOfPrefixLetters, boolean expected) { - assertEquals(expected, SheetNameFormatter.cellReferenceIsWithinRange(text, numberOfPrefixLetters)); + String prefix = text.substring(0, numberOfPrefixLetters); + String suffix = text.substring(numberOfPrefixLetters); + assertEquals(expected, SheetNameFormatter.cellReferenceIsWithinRange(prefix, suffix)); } /** @@ -93,7 +95,7 @@ public final class TestSheetNameFormatter extends TestCase { confirmCellRange("AAA1", 3, false); confirmCellRange("a111", 1, true); confirmCellRange("Sheet1", 6, false); - confirmCellRange("iV65536", 2, true); // max cell in Excel 97-2003 - confirmCellRange("IW65537", 2, false); + confirmCellRange("iV65536", 2, true); // max cell in Excel 97-2003 + confirmCellRange("IW65537", 2, false); } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index cf4857368..523d42ec9 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -31,8 +31,12 @@ import org.apache.poi.ss.util.Region; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.model.Workbook; +import org.apache.poi.hssf.record.CellValueRecordInterface; import org.apache.poi.hssf.record.EmbeddedObjectRefSubRecord; +import org.apache.poi.hssf.record.FormulaRecord; import org.apache.poi.hssf.record.NameRecord; +import org.apache.poi.hssf.record.Record; +import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate; import org.apache.poi.hssf.record.formula.DeletedArea3DPtg; import org.apache.poi.util.TempFile; @@ -1136,4 +1140,80 @@ public final class TestBugs extends TestCase { ) ); } + + /** + * From the mailing list - ensure we can handle a formula + * containing a zip code, eg ="70164" + * @throws Exception + */ + public void testZipCodeFormulas() throws Exception { + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet s = wb.createSheet(); + s.createRow(0); + HSSFCell c1 = s.getRow(0).createCell((short)0); + HSSFCell c2 = s.getRow(0).createCell((short)1); + + // As number and string + c1.setCellFormula("70164"); + c2.setCellFormula("\"70164\""); + + // Check the formulas + assertEquals("70164.0", c1.getCellFormula()); + assertEquals("\"70164\"", c2.getCellFormula()); + + // And check the values - blank + assertEquals(0.0, c1.getNumericCellValue(), 0.00001); + assertEquals("", c1.getRichStringCellValue().getString()); + assertEquals(0.0, c2.getNumericCellValue(), 0.00001); + assertEquals("", c2.getRichStringCellValue().getString()); + + // Now evaluate + HSSFFormulaEvaluator eval = new HSSFFormulaEvaluator(s, wb); + eval.setCurrentRow(s.getRow(0)); + eval.evaluateFormulaCell(c1); + eval.evaluateFormulaCell(c2); + + // Check + assertEquals(70164.0, c1.getNumericCellValue(), 0.00001); + assertEquals("", c1.getRichStringCellValue().getString()); + assertEquals(0.0, c2.getNumericCellValue(), 0.00001); + + // TODO - why isn't this working? +// assertEquals("70164", c2.getRichStringCellValue().getString()); + + + // Write and read + HSSFWorkbook nwb = writeOutAndReadBack(wb); + HSSFSheet ns = nwb.getSheetAt(0); + HSSFCell nc1 = ns.getRow(0).getCell((short)0); + HSSFCell nc2 = ns.getRow(0).getCell((short)1); + + // Re-check + assertEquals(70164.0, nc1.getNumericCellValue(), 0.00001); + assertEquals("", nc1.getRichStringCellValue().getString()); + assertEquals(0.0, nc2.getNumericCellValue(), 0.00001); + assertEquals("70164", nc2.getRichStringCellValue().getString()); + + // Now check record level stuff too + ns.getSheet().setLoc(0); + int fn = 0; + CellValueRecordInterface cvr; + while((cvr = ns.getSheet().getNextValueRecord()) != null) { + if(cvr instanceof FormulaRecordAggregate) { + FormulaRecordAggregate fr = (FormulaRecordAggregate)cvr; + + if(fn == 0) { + assertEquals(70164.0, fr.getFormulaRecord().getValue(), 0.0001); + assertNull(fr.getStringRecord()); + } else { + assertEquals(0.0, fr.getFormulaRecord().getValue(), 0.0001); + assertNotNull(fr.getStringRecord()); + assertEquals("70164", fr.getStringRecord().getString()); + } + + fn++; + } + } + assertEquals(2, fn); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java index 3b154b256..90971b3c0 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java @@ -169,6 +169,20 @@ public final class TestHSSFSheet extends TestCase { sheet.removeRow(row); } + public void testRemoveZeroRow() { + HSSFWorkbook workbook = new HSSFWorkbook(); + HSSFSheet sheet = workbook.createSheet("Sheet1"); + HSSFRow row = sheet.createRow(0); + try { + sheet.removeRow(row); + } catch (IllegalArgumentException e) { + if (e.getMessage().equals("Invalid row number (-1) outside allowable range (0..65535)")) { + throw new AssertionFailedError("Identified bug 45367"); + } + throw e; + } + } + public void testCloneSheet() { HSSFWorkbook workbook = new HSSFWorkbook(); HSSFSheet sheet = workbook.createSheet("Test Clone"); diff --git a/src/testcases/org/apache/poi/hssf/util/AllHSSFUtilTests.java b/src/testcases/org/apache/poi/hssf/util/AllHSSFUtilTests.java index 1f7aa5f25..ec88e5c43 100755 --- a/src/testcases/org/apache/poi/hssf/util/AllHSSFUtilTests.java +++ b/src/testcases/org/apache/poi/hssf/util/AllHSSFUtilTests.java @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. ==================================================================== */ - + package org.apache.poi.hssf.util; import junit.framework.Test; @@ -28,12 +28,13 @@ import junit.framework.TestSuite; public class AllHSSFUtilTests { public static Test suite() { - TestSuite result = new TestSuite("Tests for org.apache.poi.hssf.util"); - result.addTestSuite(TestAreaReference.class); - result.addTestSuite(TestCellReference.class); - result.addTestSuite(TestRangeAddress.class); - result.addTestSuite(TestRKUtil.class); - result.addTestSuite(TestSheetReferences.class); + TestSuite result = new TestSuite(AllHSSFUtilTests.class.getName()); + result.addTestSuite(TestAreaReference.class); + result.addTestSuite(TestCellReference.class); + result.addTestSuite(TestHSSFColor.class); + result.addTestSuite(TestRangeAddress.class); + result.addTestSuite(TestRKUtil.class); + result.addTestSuite(TestSheetReferences.class); return result; } } diff --git a/src/testcases/org/apache/poi/hssf/util/TestCellReference.java b/src/testcases/org/apache/poi/hssf/util/TestCellReference.java index 648fb9a8e..8ec8f9946 100644 --- a/src/testcases/org/apache/poi/hssf/util/TestCellReference.java +++ b/src/testcases/org/apache/poi/hssf/util/TestCellReference.java @@ -20,6 +20,8 @@ package org.apache.poi.hssf.util; import junit.framework.TestCase; +import org.apache.poi.hssf.util.CellReference.NameType; + public final class TestCellReference extends TestCase { @@ -75,7 +77,6 @@ public final class TestCellReference extends TestCase { confirmCell(cf, "Amazing!", 0, 0, false, false, "'Amazing!'!A1"); } - /* package */ static void confirmCell(CellReference cf, String expSheetName, int expRow, int expCol, boolean expIsRowAbs, boolean expIsColAbs, String expText) { @@ -87,8 +88,22 @@ public final class TestCellReference extends TestCase { assertEquals("text is wrong", expText, cf.formatAsString()); } - public static void main(String [] args) { - System.out.println("Testing org.apache.poi.hssf.util.TestCellReference"); - junit.textui.TestRunner.run(TestCellReference.class); + public void testClassifyCellReference() { + confirmNameType("a1", NameType.CELL); + confirmNameType("pfy1", NameType.NAMED_RANGE); + confirmNameType("pf1", NameType.NAMED_RANGE); // (col) out of cell range + confirmNameType("fp1", NameType.CELL); + confirmNameType("pf$1", NameType.BAD_CELL_OR_NAMED_RANGE); + confirmNameType("_A1", NameType.NAMED_RANGE); + confirmNameType("A_1", NameType.NAMED_RANGE); + confirmNameType("A1_", NameType.NAMED_RANGE); + confirmNameType(".A1", NameType.BAD_CELL_OR_NAMED_RANGE); + confirmNameType("A.1", NameType.NAMED_RANGE); + confirmNameType("A1.", NameType.NAMED_RANGE); + } + + private void confirmNameType(String ref, int expectedResult) { + int actualResult = CellReference.classifyCellReference(ref); + assertEquals(expectedResult, actualResult); } } diff --git a/src/testcases/org/apache/poi/hssf/util/TestHSSFColor.java b/src/testcases/org/apache/poi/hssf/util/TestHSSFColor.java index 42571eb32..d93f5d7b0 100644 --- a/src/testcases/org/apache/poi/hssf/util/TestHSSFColor.java +++ b/src/testcases/org/apache/poi/hssf/util/TestHSSFColor.java @@ -14,13 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. ==================================================================== */ - + package org.apache.poi.hssf.util; import java.util.Hashtable; import junit.framework.TestCase; - +/** + * @author Nick Burch + */ public final class TestHSSFColor extends TestCase { public void testBasics() { assertNotNull(HSSFColor.YELLOW.class);