From 5a6d8b9285b1cd32170ca4585cdda175860b9cf7 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 27 May 2023 07:40:30 -0700 Subject: [PATCH] Accommodating Slash with preg_quote - Structured Reference Column Names (#3583) PR #3513, developed by @SaidkhojaIftikhor, has been stuck for some time awaiting tests. This is the second of three PRs to replace that one. This accomodates the use of slash as a delimiter in column names in Tables and Structured References. The source changes are very simple. Additional tests exercise all the source changes. There is also a preg_quote call when a table is renamed. I have also changed it to accomodate slash, because it's the right thing to do. But ... I can't think how to test it. PhpSpreadsheet will not allow you to set a table name to a string containing a slash (a test is added to confirm), and, if I manually update the Xml in an Xlsx spreadsheet so that the name does contain a slash, Excel will, understandably, complain that the file is corrupt. --- .../Engine/Operands/StructuredReference.php | 6 +- src/PhpSpreadsheet/Worksheet/Table.php | 4 +- src/PhpSpreadsheet/Worksheet/Table/Column.php | 4 +- .../Engine/StructuredReferenceSlashTest.php | 115 ++++++++++++++++++ .../Worksheet/Table/FormulaTest.php | 26 ++++ .../Worksheet/Table/TableTest.php | 1 + 6 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Calculation/Engine/StructuredReferenceSlashTest.php diff --git a/src/PhpSpreadsheet/Calculation/Engine/Operands/StructuredReference.php b/src/PhpSpreadsheet/Calculation/Engine/Operands/StructuredReference.php index 266f1b2bb..59cc3e3d2 100644 --- a/src/PhpSpreadsheet/Calculation/Engine/Operands/StructuredReference.php +++ b/src/PhpSpreadsheet/Calculation/Engine/Operands/StructuredReference.php @@ -190,8 +190,8 @@ final class StructuredReference implements Operand { if ($columnName !== '') { $cellReference = $columnId . $cell->getRow(); - $pattern1 = '/\[' . preg_quote($columnName) . '\]/miu'; - $pattern2 = '/@' . preg_quote($columnName) . '/miu'; + $pattern1 = '/\[' . preg_quote($columnName, '/') . '\]/miu'; + $pattern2 = '/@' . preg_quote($columnName, '/') . '/miu'; if (preg_match($pattern1, $reference) === 1) { $reference = preg_replace($pattern1, $cellReference, $reference); } elseif (preg_match($pattern2, $reference) === 1) { @@ -328,7 +328,7 @@ final class StructuredReference implements Operand $cellFrom = "{$columnId}{$startRow}"; $cellTo = "{$columnId}{$endRow}"; $cellReference = ($cellFrom === $cellTo) ? $cellFrom : "{$cellFrom}:{$cellTo}"; - $pattern = '/\[' . preg_quote($columnName) . '\]/mui'; + $pattern = '/\[' . preg_quote($columnName, '/') . '\]/mui'; if (preg_match($pattern, $reference) === 1) { $columnsSelected = true; $reference = preg_replace($pattern, $cellReference, $reference); diff --git a/src/PhpSpreadsheet/Worksheet/Table.php b/src/PhpSpreadsheet/Worksheet/Table.php index dc2a4f8a5..1bc8dff45 100644 --- a/src/PhpSpreadsheet/Worksheet/Table.php +++ b/src/PhpSpreadsheet/Worksheet/Table.php @@ -180,7 +180,7 @@ class Table private function updateStructuredReferencesInCells(Worksheet $worksheet, string $newName): void { - $pattern = '/' . preg_quote($this->name) . '\[/mui'; + $pattern = '/' . preg_quote($this->name, '/') . '\[/mui'; foreach ($worksheet->getCoordinates(false) as $coordinate) { $cell = $worksheet->getCell($coordinate); @@ -196,7 +196,7 @@ class Table private function updateStructuredReferencesInNamedFormulae(Spreadsheet $spreadsheet, string $newName): void { - $pattern = '/' . preg_quote($this->name) . '\[/mui'; + $pattern = '/' . preg_quote($this->name, '/') . '\[/mui'; foreach ($spreadsheet->getNamedFormulae() as $namedFormula) { $formula = $namedFormula->getValue(); diff --git a/src/PhpSpreadsheet/Worksheet/Table/Column.php b/src/PhpSpreadsheet/Worksheet/Table/Column.php index 30630c0d4..32dd4c4f8 100644 --- a/src/PhpSpreadsheet/Worksheet/Table/Column.php +++ b/src/PhpSpreadsheet/Worksheet/Table/Column.php @@ -225,7 +225,7 @@ class Column private static function updateStructuredReferencesInCells(Worksheet $worksheet, string $oldTitle, string $newTitle): void { - $pattern = '/\[(@?)' . preg_quote($oldTitle) . '\]/mui'; + $pattern = '/\[(@?)' . preg_quote($oldTitle, '/') . '\]/mui'; foreach ($worksheet->getCoordinates(false) as $coordinate) { $cell = $worksheet->getCell($coordinate); @@ -241,7 +241,7 @@ class Column private static function updateStructuredReferencesInNamedFormulae(Spreadsheet $spreadsheet, string $oldTitle, string $newTitle): void { - $pattern = '/\[(@?)' . preg_quote($oldTitle) . '\]/mui'; + $pattern = '/\[(@?)' . preg_quote($oldTitle, '/') . '\]/mui'; foreach ($spreadsheet->getNamedFormulae() as $namedFormula) { $formula = $namedFormula->getValue(); diff --git a/tests/PhpSpreadsheetTests/Calculation/Engine/StructuredReferenceSlashTest.php b/tests/PhpSpreadsheetTests/Calculation/Engine/StructuredReferenceSlashTest.php new file mode 100644 index 000000000..8b652999b --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Engine/StructuredReferenceSlashTest.php @@ -0,0 +1,115 @@ +spreadSheet = new Spreadsheet(); + $workSheet = $spreadsheet->getActiveSheet(); + $workSheet->fromArray($this->tableData, null, 'A1'); + + $table = new Table('A1:E8', 'DeptSales'); + $table->setShowTotalsRow(true); + $table->getColumn('A')->setTotalsRowLabel('Total'); + $workSheet->addTable($table); + + return $spreadsheet; + } + + protected function tearDown(): void + { + if ($this->spreadSheet !== null) { + $this->spreadSheet->disconnectWorksheets(); + $this->spreadSheet = null; + } + + parent::tearDown(); + } + + /** + * @dataProvider structuredReferenceProviderColumnData + */ + public function testStructuredReferenceColumns(string $expectedCellRange, string $structuredReference): void + { + $spreadsheet = $this->getSpreadsheet(); + $structuredReferenceObject = new StructuredReference($structuredReference); + $cellRange = $structuredReferenceObject->parse($spreadsheet->getActiveSheet()->getCell('E5')); + self::assertSame($expectedCellRange, $cellRange); + } + + /** + * @dataProvider structuredReferenceProviderRowData + */ + public function testStructuredReferenceRows(string $expectedCellRange, string $structuredReference): void + { + $spreadsheet = $this->getSpreadsheet(); + $structuredReferenceObject = new StructuredReference($structuredReference); + $cellRange = $structuredReferenceObject->parse($spreadsheet->getActiveSheet()->getCell('E5')); + self::assertSame($expectedCellRange, $cellRange); + } + + public static function structuredReferenceProviderColumnData(): array + { + return [ + // Full table, with no column specified, means data only, not headers or totals + 'Full table Unqualified' => ['A2:E7', '[]'], + 'Full table Qualified' => ['A2:E7', 'DeptSales[]'], + // No item identifier, but with a column identifier, means data and header for the column, but no totals + 'Column with no Item Identifier #1' => ['A2:A7', 'DeptSales[[Sales Person]]'], + 'Column with no Item Identifier #2' => ['B2:B7', 'DeptSales[Region]'], + // Item identifier with no column specified + 'Item Identifier only #1' => ['A1:E1', 'DeptSales[#Headers]'], + 'Item Identifier only #2' => ['A1:E1', 'DeptSales[[#Headers]]'], + 'Item Identifier only #3' => ['A8:E8', 'DeptSales[#Totals]'], + 'Item Identifier only #4' => ['A2:E7', 'DeptSales[#Data]'], + // Item identifiers and column identifiers + 'Full column' => ['C1:C8', 'DeptSales[[#All],[Sales Amount]]'], + 'Column Header' => ['D1', 'DeptSales[[#Headers],[% Commission]]'], + 'Column Total' => ['B8', 'DeptSales[[#Totals],[Region]]'], + 'Column Range All' => ['C1:D8', 'DeptSales[[#All],[Sales Amount]:[% Commission]]'], + 'Column Range Data' => ['D2:E7', 'DeptSales[[#Data],[% Commission]:[Commission/Amount]]'], + 'Column Range Headers' => ['B1:E1', 'DeptSales[[#Headers],[Region]:[Commission/Amount]]'], + 'Column Range Totals' => ['C8:E8', 'DeptSales[[#Totals],[Sales Amount]:[Commission/Amount]]'], + 'Column Range Headers and Data' => ['D1:D7', 'DeptSales[[#Headers],[#Data],[% Commission]]'], + 'Column Range No Item Identifier' => ['A2:B7', 'DeptSales[[Sales Person]:[Region]]'], + // ['C2:C7,E2:E7', 'DeptSales[Sales Amount],DeptSales[Commission Amount]'], + // ['B2:C7', 'DeptSales[[Sales Person]:[Sales Amount]] DeptSales[[Region]:[% Commission]]'], + ]; + } + + public static function structuredReferenceProviderRowData(): array + { + return [ + ['E5', 'DeptSales[[#This Row], [Commission/Amount]]'], + ['E5', 'DeptSales[@Commission/Amount]'], + ['E5', 'DeptSales[@[Commission/Amount]]'], + ['C5:D5', 'DeptSales[@[Sales Amount]:[% Commission]]'], + ]; + } +} diff --git a/tests/PhpSpreadsheetTests/Worksheet/Table/FormulaTest.php b/tests/PhpSpreadsheetTests/Worksheet/Table/FormulaTest.php index 69e54446e..25dc27025 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/Table/FormulaTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/Table/FormulaTest.php @@ -85,6 +85,32 @@ class FormulaTest extends TestCase $spreadsheet->disconnectWorksheets(); } + public function testCellFormulaUpdateOnHeadingColumnChangeSlash(): void + { + $reader = new Xlsx(); + $filename = 'tests/data/Worksheet/Table/TableFormulae.xlsx'; + $spreadsheet = $reader->load($filename); + $worksheet = $spreadsheet->getActiveSheet(); + + // Verify original formulae + // Row Formula + self::assertSame("=DeptSales[[#This Row],[Sales\u{a0}Amount]]*DeptSales[[#This Row],[% Commission]]", $worksheet->getCell('E2')->getValue()); + // Totals Formula + self::assertSame('=SUBTOTAL(109,DeptSales[Commission Amount])', $worksheet->getCell('E8')->getValue()); + + $worksheet->getCell('D1')->setValue('Commission %age'); + $worksheet->getCell('E1')->setValue('Commission/Amount'); + $worksheet->getCell('E1')->setValue('Commission/Amount2'); + + // Verify modified formulae + // Row Formula + self::assertSame("=DeptSales[[#This Row],[Sales\u{a0}Amount]]*DeptSales[[#This Row],[Commission %age]]", $worksheet->getCell('E2')->getValue()); + // Totals Formula + self::assertSame('=SUBTOTAL(109,DeptSales[Commission/Amount2])', $worksheet->getCell('E8')->getValue()); + + $spreadsheet->disconnectWorksheets(); + } + public function testNamedFormulaUpdateOnHeadingColumnChange(): void { $reader = new Xlsx(); diff --git a/tests/PhpSpreadsheetTests/Worksheet/Table/TableTest.php b/tests/PhpSpreadsheetTests/Worksheet/Table/TableTest.php index 8c7a4e8ea..5ff0daed1 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/Table/TableTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/Table/TableTest.php @@ -75,6 +75,7 @@ class TableTest extends SetupTeardown ['R11C11'], ['123'], ['=Table'], + ['Name/Slash'], ['ிக'], // starting with UTF-8 combined character [bin2hex(random_bytes(255))], // random string with length greater than 255 ];