From aeb21f7537a1a07247c27e41750622b2fbd85caf Mon Sep 17 00:00:00 2001 From: Nathan Dench Date: Tue, 4 May 2021 14:45:04 +1000 Subject: [PATCH 1/4] R1C1 conversion should handle absolute A1 references --- src/PhpSpreadsheet/Cell/AddressHelper.php | 19 ++++++++++++++++--- .../data/Cell/A1ConversionToR1C1Relative.php | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/PhpSpreadsheet/Cell/AddressHelper.php b/src/PhpSpreadsheet/Cell/AddressHelper.php index b0e34e253c..e5f4e952f7 100644 --- a/src/PhpSpreadsheet/Cell/AddressHelper.php +++ b/src/PhpSpreadsheet/Cell/AddressHelper.php @@ -102,14 +102,27 @@ public static function convertToR1C1( ?int $currentRowNumber = null, ?int $currentColumnNumber = null ): string { - $validityCheck = preg_match('/^\$?([A-Z]{1,3})\$?(\d{1,7})$/i', $address, $cellReference); + $validityCheck = preg_match('/^(\$?[A-Z]{1,3})(\$?\d{1,7})$/i', $address, $cellReference); if ($validityCheck === 0) { throw new Exception('Invalid A1-format Cell Reference'); } - $columnId = Coordinate::columnIndexFromString($cellReference[1]); - $rowId = (int) $cellReference[2]; + if ($cellReference[1][0] === '$') { + $columnId = Coordinate::columnIndexFromString(substr($cellReference[1], 1)); + // Column must be absolute address + $currentColumnNumber = null; + } else { + $columnId = Coordinate::columnIndexFromString($cellReference[1]); + } + + if ($cellReference[2][0] === '$') { + $rowId = (int) substr($cellReference[2], 1); + // Row must be absolute address + $currentRowNumber = null; + } else { + $rowId = (int) $cellReference[2]; + } if ($currentRowNumber !== null) { if ($rowId === $currentRowNumber) { diff --git a/tests/data/Cell/A1ConversionToR1C1Relative.php b/tests/data/Cell/A1ConversionToR1C1Relative.php index 76a6aee879..dd9b23918f 100644 --- a/tests/data/Cell/A1ConversionToR1C1Relative.php +++ b/tests/data/Cell/A1ConversionToR1C1Relative.php @@ -2,18 +2,33 @@ return [ ['R[2]C[2]', 'O18', 16, 13], + ['R18C15', '$O$18', 16, 13], ['R[-2]C[2]', 'O14', 16, 13], + ['R[-2]C15', '$O14', 16, 13], ['R[2]C[-2]', 'K18', 16, 13], + ['R18C[-2]', 'K$18', 16, 13], ['R[-2]C[-2]', 'K14', 16, 13], ['RC[3]', 'P16', 16, 13], + ['R16C[3]', 'P$16', 16, 13], ['RC[-3]', 'J16', 16, 13], + ['RC10', '$J16', 16, 13], ['R[4]C', 'M20', 16, 13], + ['R[4]C13', '$M20', 16, 13], ['R[-4]C', 'M12', 16, 13], + ['R12C', 'M$12', 16, 13], ['RC', 'E5', 5, 5], + ['R5C5', '$E$5', 5, 5], ['R5C', 'E5', null, 5], + ['R5C5', '$E5', null, 5], + ['R5C', 'E$5', null, 5], ['RC5', 'E5', 5, null], + ['RC5', '$E5', 5, null], + ['R5C5', 'E$5', 5, null], ['R5C[2]', 'E5', null, 3], + ['R5C5', '$E5', null, 3], + ['R5C[2]', 'E$5', null, 3], ['R[2]C5', 'E5', 3, null], + ['R5C5', '$E$5', 3, null], ['R5C[-2]', 'E5', null, 7], ['R[-2]C5', 'E5', 7, null], ]; From c7f65bb8a4c86ebaead8f04e94a5c7824ad2d2c3 Mon Sep 17 00:00:00 2001 From: Nathan Dench Date: Tue, 4 May 2021 15:21:44 +1000 Subject: [PATCH 2/4] Update CHANGELOG.md with R1C1 conversion change --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7940a6a29b..1b41ac2854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Nothing. ### Fixed +- Correctly handle absolute A1 references when converting to R1C1 format [PR #2060](https://github.com/PHPOffice/PhpSpreadsheet/pull/2060) - Correct default fill style for conditional without a pattern defined [Issue #2035](https://github.com/PHPOffice/PhpSpreadsheet/issues/2035) [PR #2050](https://github.com/PHPOffice/PhpSpreadsheet/pull/2050) - Fixed issue where array key check for existince before accessing arrays in Xlsx.php. [PR #1970](https://github.com/PHPOffice/PhpSpreadsheet/pull/1970) - Fixed issue with quoted strings in number format mask rendered with toFormattedString() [Issue 1972#](https://github.com/PHPOffice/PhpSpreadsheet/issues/1972) [PR #1978](https://github.com/PHPOffice/PhpSpreadsheet/pull/1978) From c2fa05994f2c6ab7613192dd919c5f3919bf7bf9 Mon Sep 17 00:00:00 2001 From: Nathan Dench Date: Thu, 6 May 2021 09:57:22 +1000 Subject: [PATCH 3/4] Remove complexity from AddressHelper::convertToR1C1 --- src/PhpSpreadsheet/Cell/AddressHelper.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/PhpSpreadsheet/Cell/AddressHelper.php b/src/PhpSpreadsheet/Cell/AddressHelper.php index e5f4e952f7..91f85bed6f 100644 --- a/src/PhpSpreadsheet/Cell/AddressHelper.php +++ b/src/PhpSpreadsheet/Cell/AddressHelper.php @@ -102,26 +102,22 @@ public static function convertToR1C1( ?int $currentRowNumber = null, ?int $currentColumnNumber = null ): string { - $validityCheck = preg_match('/^(\$?[A-Z]{1,3})(\$?\d{1,7})$/i', $address, $cellReference); + $validityCheck = preg_match('/^(\$?)([A-Z]{1,3})(\$?)(\d{1,7})$/i', $address, $cellReference); if ($validityCheck === 0) { throw new Exception('Invalid A1-format Cell Reference'); } - if ($cellReference[1][0] === '$') { - $columnId = Coordinate::columnIndexFromString(substr($cellReference[1], 1)); + $columnId = Coordinate::columnIndexFromString($cellReference[2]); + if ($cellReference[1] === '$') { // Column must be absolute address $currentColumnNumber = null; - } else { - $columnId = Coordinate::columnIndexFromString($cellReference[1]); } - if ($cellReference[2][0] === '$') { - $rowId = (int) substr($cellReference[2], 1); + $rowId = (int) $cellReference[4]; + if ($cellReference[3] === '$') { // Row must be absolute address $currentRowNumber = null; - } else { - $rowId = (int) $cellReference[2]; } if ($currentRowNumber !== null) { From 1adc1889e1e27dd9b570827983a769b18e6cbf8b Mon Sep 17 00:00:00 2001 From: Nathan Dench Date: Fri, 7 May 2021 13:00:43 +1000 Subject: [PATCH 4/4] Use named regex groups and constants for regex strings --- src/PhpSpreadsheet/Cell/AddressHelper.php | 10 +++++----- src/PhpSpreadsheet/Cell/Coordinate.php | 6 ++++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/PhpSpreadsheet/Cell/AddressHelper.php b/src/PhpSpreadsheet/Cell/AddressHelper.php index 91f85bed6f..632c046fdd 100644 --- a/src/PhpSpreadsheet/Cell/AddressHelper.php +++ b/src/PhpSpreadsheet/Cell/AddressHelper.php @@ -102,20 +102,20 @@ public static function convertToR1C1( ?int $currentRowNumber = null, ?int $currentColumnNumber = null ): string { - $validityCheck = preg_match('/^(\$?)([A-Z]{1,3})(\$?)(\d{1,7})$/i', $address, $cellReference); + $validityCheck = preg_match(Coordinate::A1_COORDINATE_REGEX, $address, $cellReference); if ($validityCheck === 0) { throw new Exception('Invalid A1-format Cell Reference'); } - $columnId = Coordinate::columnIndexFromString($cellReference[2]); - if ($cellReference[1] === '$') { + $columnId = Coordinate::columnIndexFromString($cellReference['col_ref']); + if ($cellReference['absolute_col'] === '$') { // Column must be absolute address $currentColumnNumber = null; } - $rowId = (int) $cellReference[4]; - if ($cellReference[3] === '$') { + $rowId = (int) $cellReference['row_ref']; + if ($cellReference['absolute_row'] === '$') { // Row must be absolute address $currentRowNumber = null; } diff --git a/src/PhpSpreadsheet/Cell/Coordinate.php b/src/PhpSpreadsheet/Cell/Coordinate.php index 0b3917f265..58d2573e66 100644 --- a/src/PhpSpreadsheet/Cell/Coordinate.php +++ b/src/PhpSpreadsheet/Cell/Coordinate.php @@ -13,6 +13,8 @@ */ abstract class Coordinate { + public const A1_COORDINATE_REGEX = '/^(?\$?)(?[A-Z]{1,3})(?\$?)(?\d{1,7})$/i'; + /** * Default range variable constant. * @@ -29,8 +31,8 @@ abstract class Coordinate */ public static function coordinateFromString($pCoordinateString) { - if (preg_match('/^([$]?[A-Z]{1,3})([$]?\\d{1,7})$/', $pCoordinateString, $matches)) { - return [$matches[1], $matches[2]]; + if (preg_match(self::A1_COORDINATE_REGEX, $pCoordinateString, $matches)) { + return [$matches['absolute_col'] . $matches['col_ref'], $matches['absolute_row'] . $matches['row_ref']]; } elseif (self::coordinateIsRange($pCoordinateString)) { throw new Exception('Cell coordinate string can not be a range of cells'); } elseif ($pCoordinateString == '') {