Skip to content

Commit

Permalink
Accommodating Slash with preg_quote - Structured Reference Column Nam…
Browse files Browse the repository at this point in the history
…es (#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.
  • Loading branch information
oleibman committed May 27, 2023
1 parent 4f6d1f7 commit 5a6d8b9
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ private function adjustRowReference(string $columnName, string $reference, Cell
{
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) {
Expand Down Expand Up @@ -328,7 +328,7 @@ private function getColumnsForColumnReference(string $reference, int $startRow,
$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);
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Worksheet/Table.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private function updateStructuredReferences(string $name): void

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);
Expand All @@ -196,7 +196,7 @@ private function updateStructuredReferencesInCells(Worksheet $worksheet, string

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();
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Worksheet/Table/Column.php
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public static function updateStructuredReferences(?Worksheet $workSheet, ?string

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);
Expand All @@ -241,7 +241,7 @@ private static function updateStructuredReferencesInCells(Worksheet $worksheet,

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();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Engine;

use PhpOffice\PhpSpreadsheet\Calculation\Engine\Operands\StructuredReference;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\Table;
use PHPUnit\Framework\TestCase;

class StructuredReferenceSlashTest extends TestCase
{
protected ?Spreadsheet $spreadSheet;

protected const COLUMN_FORMULA = '=[@Sales Amount]*[@[% Commission]]';

// Note that column headings may contain a non-breaking space, while the formula may not;
// these still need to match.
// As compared to StructuredReferenceTest, the last column
// "Commission/Amount" contains a slash. See PR #3513.
protected array $tableData = [
["Sales\u{a0}Person", 'Region', "Sales\u{a0}Amount", "%\u{a0}Commission", 'Commission/Amount'],
['Joe', 'North', 260, '10%', self::COLUMN_FORMULA],
['Robert', 'South', 660, '15%', self::COLUMN_FORMULA],
['Michelle', 'East', 940, '15%', self::COLUMN_FORMULA],
['Erich', 'West', 410, '12%', self::COLUMN_FORMULA],
['Dafna', 'North', 800, '15%', self::COLUMN_FORMULA],
['Rob', 'South', 900, '15%', self::COLUMN_FORMULA],
['Total'],
];

protected function getSpreadsheet(): Spreadsheet
{
$spreadsheet = $this->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]]'],
];
}
}
26 changes: 26 additions & 0 deletions tests/PhpSpreadsheetTests/Worksheet/Table/FormulaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,32 @@ public function testCellFormulaUpdateOnHeadingColumnChange(): void
$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();
Expand Down
1 change: 1 addition & 0 deletions tests/PhpSpreadsheetTests/Worksheet/Table/TableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public static function invalidTableNamesProvider(): array
['R11C11'],
['123'],
['=Table'],
['Name/Slash'],
['ிக'], // starting with UTF-8 combined character
[bin2hex(random_bytes(255))], // random string with length greater than 255
];
Expand Down

0 comments on commit 5a6d8b9

Please sign in to comment.