From ed62526acad6dc0d8da3a5f14e27d207379ce1e3 Mon Sep 17 00:00:00 2001 From: Mark Baker Date: Sun, 14 Mar 2021 19:58:10 +0100 Subject: [PATCH] First step extracting INDIRECT() and OFFSET() to their own classes (#1921) * First step extracting INDIRECT() and OFFSET() to their own classes * Start building unit tests for OFFSET() and INDEX() * Named ranges should be handled by the Calculation Engine, not by the implementation of the Excel INDIRECT() function * When calling the calculation engine to get the range of cells to return, INDIRECT() and OFFSET() should use the instance of the calculation engine for the current workbook to benefit from cached results in that range There's a couple of minor bugfixes in here; but it's basically just refactoring of the INDIRECT() and OFFSET() Excel functions into their own classes - still needs a lot of work on unit testing; and there's a lot more that could be improved in the code itself (including handling of the a1 flag for R1C1 format in INDIRECT() --- samples/Calculations/LookupRef/INDIRECT.php | 33 ++++ samples/Calculations/LookupRef/OFFSET.php | 33 ++++ .../Calculation/Calculation.php | 6 +- src/PhpSpreadsheet/Calculation/LookupRef.php | 153 ++++-------------- .../Calculation/LookupRef/Indirect.php | 75 +++++++++ .../Calculation/LookupRef/Offset.php | 136 ++++++++++++++++ .../Functions/LookupRef/IndirectTest.php | 55 +++++++ .../Functions/LookupRef/OffsetTest.php | 32 ++++ tests/data/Calculation/LookupRef/INDIRECT.php | 16 ++ tests/data/Calculation/LookupRef/OFFSET.php | 8 + 10 files changed, 426 insertions(+), 121 deletions(-) create mode 100644 samples/Calculations/LookupRef/INDIRECT.php create mode 100644 samples/Calculations/LookupRef/OFFSET.php create mode 100644 src/PhpSpreadsheet/Calculation/LookupRef/Indirect.php create mode 100644 src/PhpSpreadsheet/Calculation/LookupRef/Offset.php create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/OffsetTest.php create mode 100644 tests/data/Calculation/LookupRef/INDIRECT.php create mode 100644 tests/data/Calculation/LookupRef/OFFSET.php diff --git a/samples/Calculations/LookupRef/INDIRECT.php b/samples/Calculations/LookupRef/INDIRECT.php new file mode 100644 index 00000000..ffbada9a --- /dev/null +++ b/samples/Calculations/LookupRef/INDIRECT.php @@ -0,0 +1,33 @@ +log('Returns the cell specified by a text string.'); + +// Create new PhpSpreadsheet object +$spreadsheet = new Spreadsheet(); +$worksheet = $spreadsheet->getActiveSheet(); + +$data = [ + [8, 9, 0], + [3, 4, 5], + [9, 1, 3], + [4, 6, 2], +]; +$worksheet->fromArray($data, null, 'C1'); + +$spreadsheet->addNamedRange(new NamedRange('NAMED_RANGE_FOR_CELL_D4', $worksheet, '="$D$4"')); + +$worksheet->getCell('A1')->setValue('=INDIRECT("C1")'); +$worksheet->getCell('A2')->setValue('=INDIRECT("D"&4)'); +$worksheet->getCell('A3')->setValue('=INDIRECT("E"&ROW())'); +$worksheet->getCell('A4')->setValue('=SUM(INDIRECT("$C$4:$E$4"))'); +$worksheet->getCell('A5')->setValue('=INDIRECT(NAMED_RANGE_FOR_CELL_D4)'); + +for ($row = 1; $row <= 5; ++$row) { + $cell = $worksheet->getCell("A{$row}"); + $helper->log("A{$row}: {$cell->getValue()} => {$cell->getCalculatedValue()}"); +} diff --git a/samples/Calculations/LookupRef/OFFSET.php b/samples/Calculations/LookupRef/OFFSET.php new file mode 100644 index 00000000..ae613ec5 --- /dev/null +++ b/samples/Calculations/LookupRef/OFFSET.php @@ -0,0 +1,33 @@ +log('Returns a cell range that is a specified number of rows and columns from a cell or range of cells.'); + +// Create new PhpSpreadsheet object +$spreadsheet = new Spreadsheet(); +$worksheet = $spreadsheet->getActiveSheet(); + +$data = [ + [null, 'Week 1', 'Week 2', 'Week 3', 'Week 4'], + ['Sunday', 4500, 2200, 3800, 1500], + ['Monday', 5500, 6100, 5200, 4800], + ['Tuesday', 7000, 6200, 5000, 7100], + ['Wednesday', 8000, 4000, 3900, 7600], + ['Thursday', 5900, 5500, 6900, 7100], + ['Friday', 4900, 6300, 6900, 5200], + ['Saturday', 3500, 3900, 5100, 4100], +]; +$worksheet->fromArray($data, null, 'A3'); + +$worksheet->getCell('H1')->setValue('=OFFSET(A3, 3, 1)'); +$worksheet->getCell('H2')->setValue('=SUM(OFFSET(A3, 3, 1, 1, 4))'); +$worksheet->getCell('H3')->setValue('=SUM(OFFSET(B3:E3, 3, 0))'); +$worksheet->getCell('H4')->setValue('=SUM(OFFSET(E3, 1, -3, 7))'); + +for ($row = 1; $row <= 4; ++$row) { + $cell = $worksheet->getCell("H{$row}"); + $helper->log("H{$row}: {$cell->getValue()} => {$cell->getCalculatedValue()}"); +} diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 1db4722b..c88eff31 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -1408,7 +1408,7 @@ class Calculation ], 'INDIRECT' => [ 'category' => Category::CATEGORY_LOOKUP_AND_REFERENCE, - 'functionCall' => [LookupRef::class, 'INDIRECT'], + 'functionCall' => [LookupRef\Indirect::class, 'INDIRECT'], 'argumentCount' => '1,2', 'passCellReference' => true, ], @@ -1881,7 +1881,7 @@ class Calculation ], 'OFFSET' => [ 'category' => Category::CATEGORY_LOOKUP_AND_REFERENCE, - 'functionCall' => [LookupRef::class, 'OFFSET'], + 'functionCall' => [LookupRef\Offset::class, 'OFFSET'], 'argumentCount' => '3-5', 'passCellReference' => true, 'passByReference' => [true], @@ -2702,7 +2702,7 @@ class Calculation * Get an instance of this class. * * @param ?Spreadsheet $spreadsheet Injected spreadsheet for working with a PhpSpreadsheet Spreadsheet object, - * or NULL to create a standalone claculation engine + * or NULL to create a standalone calculation engine */ public static function getInstance(?Spreadsheet $spreadsheet = null): self { diff --git a/src/PhpSpreadsheet/Calculation/LookupRef.php b/src/PhpSpreadsheet/Calculation/LookupRef.php index d2cc4f94..17115a06 100644 --- a/src/PhpSpreadsheet/Calculation/LookupRef.php +++ b/src/PhpSpreadsheet/Calculation/LookupRef.php @@ -4,12 +4,13 @@ namespace PhpOffice\PhpSpreadsheet\Calculation; use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\Address; use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\HLookup; +use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\Indirect; use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\Lookup; use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\Matrix; +use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\Offset; use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\RowColumnInformation; use PhpOffice\PhpSpreadsheet\Calculation\LookupRef\VLookup; use PhpOffice\PhpSpreadsheet\Cell\Cell; -use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class LookupRef @@ -181,56 +182,22 @@ class LookupRef * Excel Function: * =INDIRECT(cellAddress) * + * @Deprecated 1.18.0 + * + * @see Use the INDIRECT() method in the LookupRef\Indirect class instead + * * NOTE - INDIRECT() does not yet support the optional a1 parameter introduced in Excel 2010 * * @param null|array|string $cellAddress $cellAddress The cell address of the current cell (containing this formula) * @param Cell $pCell The current cell (containing this formula) * - * @return mixed The cells referenced by cellAddress + * @return array|string An array containing a cell or range of cells, or a string on error * * @TODO Support for the optional a1 parameter introduced in Excel 2010 */ public static function INDIRECT($cellAddress = null, ?Cell $pCell = null) { - $cellAddress = Functions::flattenSingleValue($cellAddress); - if ($cellAddress === null || $cellAddress === '') { - return Functions::REF(); - } - - $cellAddress1 = $cellAddress; - $cellAddress2 = null; - if (strpos($cellAddress, ':') !== false) { - [$cellAddress1, $cellAddress2] = explode(':', $cellAddress); - } - - if ( - (!preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $cellAddress1, $matches)) || - (($cellAddress2 !== null) && (!preg_match('/^' . Calculation::CALCULATION_REGEXP_CELLREF . '$/i', $cellAddress2, $matches))) - ) { - if (!preg_match('/^' . Calculation::CALCULATION_REGEXP_DEFINEDNAME . '$/i', $cellAddress1, $matches)) { - return Functions::REF(); - } - - if (strpos($cellAddress, '!') !== false) { - [$sheetName, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true); - $sheetName = trim($sheetName, "'"); - $pSheet = $pCell->getWorksheet()->getParent()->getSheetByName($sheetName); - } else { - $pSheet = $pCell->getWorksheet(); - } - - return Calculation::getInstance()->extractNamedRange($cellAddress, $pSheet, false); - } - - if (strpos($cellAddress, '!') !== false) { - [$sheetName, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true); - $sheetName = trim($sheetName, "'"); - $pSheet = $pCell->getWorksheet()->getParent()->getSheetByName($sheetName); - } else { - $pSheet = $pCell->getWorksheet(); - } - - return Calculation::getInstance()->extractCellRange($cellAddress, $pSheet, false); + return Indirect::INDIRECT($cellAddress, $pCell); } /** @@ -243,87 +210,33 @@ class LookupRef * Excel Function: * =OFFSET(cellAddress, rows, cols, [height], [width]) * - * @param null|string $cellAddress The reference from which you want to base the offset. Reference must refer to a cell or - * range of adjacent cells; otherwise, OFFSET returns the #VALUE! error value. - * @param mixed $rows The number of rows, up or down, that you want the upper-left cell to refer to. - * Using 5 as the rows argument specifies that the upper-left cell in the reference is - * five rows below reference. Rows can be positive (which means below the starting reference) - * or negative (which means above the starting reference). - * @param mixed $columns The number of columns, to the left or right, that you want the upper-left cell of the result - * to refer to. Using 5 as the cols argument specifies that the upper-left cell in the - * reference is five columns to the right of reference. Cols can be positive (which means - * to the right of the starting reference) or negative (which means to the left of the - * starting reference). - * @param mixed $height The height, in number of rows, that you want the returned reference to be. Height must be a positive number. - * @param mixed $width The width, in number of columns, that you want the returned reference to be. Width must be a positive number. + * @Deprecated 1.18.0 * - * @return string A reference to a cell or range of cells + * @see Use the OFFSET() method in the LookupRef\Offset class instead + * + * @param null|string $cellAddress The reference from which you want to base the offset. + * Reference must refer to a cell or range of adjacent cells; + * otherwise, OFFSET returns the #VALUE! error value. + * @param mixed $rows The number of rows, up or down, that you want the upper-left cell to refer to. + * Using 5 as the rows argument specifies that the upper-left cell in the + * reference is five rows below reference. Rows can be positive (which means + * below the starting reference) or negative (which means above the starting + * reference). + * @param mixed $columns The number of columns, to the left or right, that you want the upper-left cell + * of the result to refer to. Using 5 as the cols argument specifies that the + * upper-left cell in the reference is five columns to the right of reference. + * Cols can be positive (which means to the right of the starting reference) + * or negative (which means to the left of the starting reference). + * @param mixed $height The height, in number of rows, that you want the returned reference to be. + * Height must be a positive number. + * @param mixed $width The width, in number of columns, that you want the returned reference to be. + * Width must be a positive number. + * + * @return array|string An array containing a cell or range of cells, or a string on error */ public static function OFFSET($cellAddress = null, $rows = 0, $columns = 0, $height = null, $width = null, ?Cell $pCell = null) { - $rows = Functions::flattenSingleValue($rows); - $columns = Functions::flattenSingleValue($columns); - $height = Functions::flattenSingleValue($height); - $width = Functions::flattenSingleValue($width); - if ($cellAddress === null) { - return 0; - } - - if (!is_object($pCell)) { - return Functions::REF(); - } - - $sheetName = null; - if (strpos($cellAddress, '!')) { - [$sheetName, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true); - $sheetName = trim($sheetName, "'"); - } - if (strpos($cellAddress, ':')) { - [$startCell, $endCell] = explode(':', $cellAddress); - } else { - $startCell = $endCell = $cellAddress; - } - [$startCellColumn, $startCellRow] = Coordinate::coordinateFromString($startCell); - [$endCellColumn, $endCellRow] = Coordinate::coordinateFromString($endCell); - - $startCellRow += $rows; - $startCellColumn = Coordinate::columnIndexFromString($startCellColumn) - 1; - $startCellColumn += $columns; - - if (($startCellRow <= 0) || ($startCellColumn < 0)) { - return Functions::REF(); - } - $endCellColumn = Coordinate::columnIndexFromString($endCellColumn) - 1; - if (($width != null) && (!is_object($width))) { - $endCellColumn = $startCellColumn + $width - 1; - } else { - $endCellColumn += $columns; - } - $startCellColumn = Coordinate::stringFromColumnIndex($startCellColumn + 1); - - if (($height != null) && (!is_object($height))) { - $endCellRow = $startCellRow + $height - 1; - } else { - $endCellRow += $rows; - } - - if (($endCellRow <= 0) || ($endCellColumn < 0)) { - return Functions::REF(); - } - $endCellColumn = Coordinate::stringFromColumnIndex($endCellColumn + 1); - - $cellAddress = $startCellColumn . $startCellRow; - if (($startCellColumn != $endCellColumn) || ($startCellRow != $endCellRow)) { - $cellAddress .= ':' . $endCellColumn . $endCellRow; - } - - if ($sheetName !== null) { - $pSheet = $pCell->getWorksheet()->getParent()->getSheetByName($sheetName); - } else { - $pSheet = $pCell->getWorksheet(); - } - - return Calculation::getInstance()->extractCellRange($cellAddress, $pSheet, false); + return Offset::OFFSET($cellAddress, $rows, $columns, $height, $width, $pCell); } /** @@ -370,6 +283,10 @@ class LookupRef * Excel Function: * =MATCH(lookup_value, lookup_array, [match_type]) * + * @Deprecated 1.18.0 + * + * @see Use the MATCH() method in the LookupRef\ExcelMatch class instead + * * @param mixed $lookupValue The value that you want to match in lookup_array * @param mixed $lookupArray The range of cells being searched * @param mixed $matchType The number -1, 0, or 1. -1 means above, 0 means exact match, 1 means below. diff --git a/src/PhpSpreadsheet/Calculation/LookupRef/Indirect.php b/src/PhpSpreadsheet/Calculation/LookupRef/Indirect.php new file mode 100644 index 00000000..690b32e4 --- /dev/null +++ b/src/PhpSpreadsheet/Calculation/LookupRef/Indirect.php @@ -0,0 +1,75 @@ +getParent() : null) + ->extractCellRange($cellAddress, $pSheet, false); + } + + private static function extractWorksheet($cellAddress, Cell $pCell): array + { + $sheetName = ''; + if (strpos($cellAddress, '!') !== false) { + [$sheetName, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true); + $sheetName = trim($sheetName, "'"); + } + + $pSheet = ($sheetName !== '') + ? $pCell->getWorksheet()->getParent()->getSheetByName($sheetName) + : $pCell->getWorksheet(); + + return [$cellAddress, $pSheet]; + } +} diff --git a/src/PhpSpreadsheet/Calculation/LookupRef/Offset.php b/src/PhpSpreadsheet/Calculation/LookupRef/Offset.php new file mode 100644 index 00000000..25ec498b --- /dev/null +++ b/src/PhpSpreadsheet/Calculation/LookupRef/Offset.php @@ -0,0 +1,136 @@ +getParent() : null) + ->extractCellRange($cellAddress, $pSheet, false); + } + + private static function extractWorksheet($cellAddress, Cell $pCell): array + { + $sheetName = ''; + if (strpos($cellAddress, '!') !== false) { + [$sheetName, $cellAddress] = Worksheet::extractSheetTitle($cellAddress, true); + $sheetName = trim($sheetName, "'"); + } + + $pSheet = ($sheetName !== '') + ? $pCell->getWorksheet()->getParent()->getSheetByName($sheetName) + : $pCell->getWorksheet(); + + return [$cellAddress, $pSheet]; + } + + private static function adjustEndCellColumnForWidth(string $endCellColumn, $width, int $startCellColumn, $columns) + { + $endCellColumn = Coordinate::columnIndexFromString($endCellColumn) - 1; + if (($width !== null) && (!is_object($width))) { + $endCellColumn = $startCellColumn + (int) $width - 1; + } else { + $endCellColumn += (int) $columns; + } + + return $endCellColumn; + } + + private static function adustEndCellRowForHeight($height, int $startCellRow, $rows, $endCellRow): int + { + if (($height !== null) && (!is_object($height))) { + $endCellRow = $startCellRow + (int) $height - 1; + } else { + $endCellRow += (int) $rows; + } + + return $endCellRow; + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php new file mode 100644 index 00000000..f1018e7f --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndirectTest.php @@ -0,0 +1,55 @@ +getMockBuilder(Calculation::class) +// ->setMethods(['getInstance', 'extractCellRange']) +// ->disableOriginalConstructor() +// ->getMock(); +// $calculation->method('getInstance') +// ->willReturn($calculation); +// $calculation->method('extractCellRange') +// ->willReturn([]); +// +// $worksheet = $this->getMockBuilder(Cell::class) +// ->setMethods(['getParent']) +// ->disableOriginalConstructor() +// ->getMock(); +// +// $cell = $this->getMockBuilder(Cell::class) +// ->setMethods(['getWorksheet']) +// ->disableOriginalConstructor() +// ->getMock(); +// $cell->method('getWorksheet') +// ->willReturn($worksheet); + + $result = LookupRef::INDIRECT($cellReference); + self::assertSame($expectedResult, $result); + } + + public function providerINDIRECT() + { + return require 'tests/data/Calculation/LookupRef/INDIRECT.php'; + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/OffsetTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/OffsetTest.php new file mode 100644 index 00000000..631af08d --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/OffsetTest.php @@ -0,0 +1,32 @@ +