From cb800acd83a8edd40f51016e963b79b5c2ec8364 Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Fri, 10 Jun 2016 11:52:44 +0000 Subject: [PATCH] bug 59684: correctly parse CellReference(HOMEst163); Also deprecate misspelled methods isRowWithinRange/isColumnWithinRange git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1747687 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/ss/util/CellReference.java | 75 +++++++++++-------- .../apache/poi/ss/util/TestCellReference.java | 13 ++++ 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/src/java/org/apache/poi/ss/util/CellReference.java b/src/java/org/apache/poi/ss/util/CellReference.java index 6eb8035aa2..235f58307c 100644 --- a/src/java/org/apache/poi/ss/util/CellReference.java +++ b/src/java/org/apache/poi/ss/util/CellReference.java @@ -59,25 +59,33 @@ public class CellReference { /** * Matches a run of one or more letters followed by a run of one or more digits. + * Both the letter and number groups are optional. * 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]+)"); + private static final Pattern CELL_REF_PATTERN = Pattern.compile("(\\$?[A-Z]+)?" + "(\\$?[0-9]+)?", Pattern.CASE_INSENSITIVE); + /** + * Matches references only where row and column are included. + * Matches a run of one or more letters followed by a run of one or more digits. + * If a reference does not match this pattern, it might match COLUMN_REF_PATTERN or ROW_REF_PATTERN + * References may optionally include a single '$' before each group, but these are excluded from the Matcher.group(int). + */ + private static final Pattern STRICTLY_CELL_REF_PATTERN = Pattern.compile("\\$?([A-Z]+)" + "\\$?([0-9]+)", Pattern.CASE_INSENSITIVE); /** * Matches a run of one or more letters. The run of letters is group 1. - * The text may optionally be prefixed with a single '$'. + * References may optionally include a single '$' before the group, but these are excluded from the Matcher.group(int). */ - private static final Pattern COLUMN_REF_PATTERN = Pattern.compile("\\$?([A-Za-z]+)"); + private static final Pattern COLUMN_REF_PATTERN = Pattern.compile("\\$?([A-Z]+)", Pattern.CASE_INSENSITIVE); /** - * Matches a run of one or more digits. The run of digits is group 1. - * The text may optionally be prefixed with a single '$'. + * Matches a run of one or more letters. The run of numbers is group 1. + * References may optionally include a single '$' before the group, but these are excluded from the Matcher.group(int). */ private static final Pattern ROW_REF_PATTERN = Pattern.compile("\\$?([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 Pattern NAMED_RANGE_NAME_PATTERN = Pattern.compile("[_A-Z][_.A-Z0-9]*", Pattern.CASE_INSENSITIVE); //private static final String BIFF8_LAST_COLUMN = SpreadsheetVersion.EXCEL97.getLastColumnName(); //private static final int BIFF8_LAST_COLUMN_TEXT_LEN = BIFF8_LAST_COLUMN.length(); //private static final String BIFF8_LAST_ROW = String.valueOf(SpreadsheetVersion.EXCEL97.getMaxRows()); @@ -221,7 +229,7 @@ public class CellReference { // no digits at end of str return validateNamedRangeName(str, ssVersion); } - Matcher cellRefPatternMatcher = CELL_REF_PATTERN.matcher(str); + Matcher cellRefPatternMatcher = STRICTLY_CELL_REF_PATTERN.matcher(str); if (!cellRefPatternMatcher.matches()) { return validateNamedRangeName(str, ssVersion); } @@ -247,14 +255,14 @@ public class CellReference { Matcher colMatcher = COLUMN_REF_PATTERN.matcher(str); if (colMatcher.matches()) { String colStr = colMatcher.group(1); - if (isColumnWithnRange(colStr, ssVersion)) { + if (isColumnWithinRange(colStr, ssVersion)) { return NameType.COLUMN; } } Matcher rowMatcher = ROW_REF_PATTERN.matcher(str); if (rowMatcher.matches()) { String rowStr = rowMatcher.group(1); - if (isRowWithnRange(rowStr, ssVersion)) { + if (isRowWithinRange(rowStr, ssVersion)) { return NameType.ROW; } } @@ -302,13 +310,22 @@ public class CellReference { * @return true if the row and col parameters are within range of a BIFF8 spreadsheet. */ public static boolean cellReferenceIsWithinRange(String colStr, String rowStr, SpreadsheetVersion ssVersion) { - if (!isColumnWithnRange(colStr, ssVersion)) { + if (!isColumnWithinRange(colStr, ssVersion)) { return false; } - return isRowWithnRange(rowStr, ssVersion); + return isRowWithinRange(rowStr, ssVersion); } + /** + * @deprecated 3.15 beta 2. Use {@link #isColumnWithinRange}. + */ public static boolean isColumnWithnRange(String colStr, SpreadsheetVersion ssVersion) { + return isColumnWithinRange(colStr, ssVersion); + } + + public static boolean isColumnWithinRange(String colStr, SpreadsheetVersion ssVersion) { + // Equivalent to 0 <= CellReference.convertColStringToIndex(colStr) <= ssVersion.getLastColumnIndex() + String lastCol = ssVersion.getLastColumnName(); int lastColLength = lastCol.length(); @@ -328,7 +345,14 @@ public class CellReference { return true; } + /** + * @deprecated 3.15 beta 2. Use {@link #isRowWithinRange} + */ public static boolean isRowWithnRange(String rowStr, SpreadsheetVersion ssVersion) { + return isRowWithinRange(rowStr, ssVersion); + } + + public static boolean isRowWithinRange(String rowStr, SpreadsheetVersion ssVersion) { int rowNum = Integer.parseInt(rowStr); if (rowNum < 0) { @@ -349,8 +373,8 @@ public class CellReference { private CellRefParts(String sheetName, String rowRef, String colRef) { this.sheetName = sheetName; - this.rowRef = rowRef; - this.colRef = colRef; + this.rowRef = (rowRef != null) ? rowRef : ""; + this.colRef = (colRef != null) ? colRef : ""; } } @@ -367,27 +391,12 @@ public class CellReference { private static CellRefParts separateRefParts(String reference) { int plingPos = reference.lastIndexOf(SHEET_NAME_DELIMITER); final String sheetName = parseSheetName(reference, plingPos); - String row; - String col; - int start = plingPos+1; - - int length = reference.length(); - - int loc = start; - // skip initial dollars - if (reference.charAt(loc)==ABSOLUTE_REFERENCE_MARKER) { - loc++; - } - // step over column name chars until first digit (or dollars) for row number. - for (; loc < length; loc++) { - char ch = reference.charAt(loc); - if (Character.isDigit(ch) || ch == ABSOLUTE_REFERENCE_MARKER) { - break; - } - } + String cell = reference.substring(plingPos+1).toUpperCase(Locale.ROOT); + Matcher matcher = CELL_REF_PATTERN.matcher(cell); + if (!matcher.matches()) throw new IllegalArgumentException("Invalid CellReference: " + reference); + String col = matcher.group(1); + String row = matcher.group(2); - col = reference.substring(start,loc).toUpperCase(Locale.ROOT); - row = reference.substring(loc); CellRefParts cellRefParts = new CellRefParts(sheetName, row, col); return cellRefParts; } diff --git a/src/testcases/org/apache/poi/ss/util/TestCellReference.java b/src/testcases/org/apache/poi/ss/util/TestCellReference.java index 421e95492a..08aa50674b 100644 --- a/src/testcases/org/apache/poi/ss/util/TestCellReference.java +++ b/src/testcases/org/apache/poi/ss/util/TestCellReference.java @@ -304,4 +304,17 @@ public final class TestCellReference { assertEquals("", CellReference.convertNumToColString(Integer.MAX_VALUE)); assertEquals("FXSHRXW", CellReference.convertNumToColString(Integer.MAX_VALUE-1)); } + + /** + * bug 59684: separateRefParts fails on entire-column references + */ + @Test + public void entireColumnReferences() { + CellReference ref = new CellReference("HOME!$169"); + assertEquals("HOME", ref.getSheetName()); + assertEquals(168, ref.getRow()); + assertEquals(-1, ref.getCol()); + assertTrue("row absolute", ref.isRowAbsolute()); + //assertFalse("column absolute/relative is undefined", ref.isColAbsolute()); + } } -- 2.39.5