Skip to content

Commit

Permalink
fix: Allow to read data from a table located in a different sheet (#3659
Browse files Browse the repository at this point in the history
)

* fix: Allow to read data from a table located in a different sheet

* Add Test Case, Correct Calculation

Calculate correct result, at least for calculations with a single answer. For an array of answers, result will be similar to other functions which return an array (dynamic arrays not yet fully supported).

* Rename test class

* Allow Appropriate Use of Table as DefinedName in Calculations

See Issue3569Test, which behaves like Excel.

* Remove Dead Code

Scrutinizer is correct here.

* Memory Leak

Worksheet points to Table, Table points back to Worksheet. Circular reference prevents garbage collection. Remove Table collection in Worksheet at destruct time to avoid this problem.

* PhpUnit10 Failure

Avoiding memory leak triggered segfault in Phpunit 10. Research and fix later.

* Add toString to StructuredReference

Absence of such a method seems to cause problems for untaken IF branches in Calculation.

* Remove Dead Assignments

Correctly detected by Scrutinizer.

---------

Co-authored-by: Kevin Verschaeve <[email protected]>
Co-authored-by: oleibman <[email protected]>
  • Loading branch information
3 people authored Aug 19, 2023
1 parent 37a8a56 commit 4971801
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 3 deletions.
21 changes: 21 additions & 0 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\DefinedName;
use PhpOffice\PhpSpreadsheet\NamedRange;
use PhpOffice\PhpSpreadsheet\ReferenceHelper;
use PhpOffice\PhpSpreadsheet\Shared;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
Expand Down Expand Up @@ -5176,6 +5177,26 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $cell = null)

$this->debugLog->writeDebugLog('Evaluating Defined Name %s', $definedName);
$namedRange = DefinedName::resolveName($definedName, $pCellWorksheet);
// If not Defined Name, try as Table.
if ($namedRange === null && $this->spreadsheet !== null) {
$table = $this->spreadsheet->getTableByName($definedName);
if ($table !== null) {
$tableRange = Coordinate::getRangeBoundaries($table->getRange());
if ($table->getShowHeaderRow()) {
++$tableRange[0][1];
}
if ($table->getShowTotalsRow()) {
--$tableRange[1][1];
}
$tableRangeString =
'$' . $tableRange[0][0]
. '$' . $tableRange[0][1]
. ':'
. '$' . $tableRange[1][0]
. '$' . $tableRange[1][1];
$namedRange = new NamedRange($definedName, $table->getWorksheet(), $tableRangeString);
}
}
if ($namedRange === null) {
return $this->raiseFormulaError("undefined name '$definedName'");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,13 @@ public function parse(Cell $cell): string
{
$this->getTableStructure($cell);
$cellRange = ($this->isRowReference()) ? $this->getRowReference($cell) : $this->getColumnReference();
$sheetName = '';
$worksheet = $this->table->getWorksheet();
if ($worksheet !== null && $worksheet !== $cell->getWorksheet()) {
$sheetName = "'" . $worksheet->getTitle() . "'!";
}

return $cellRange;
return $sheetName . $cellRange;
}

private function isRowReference(): bool
Expand Down Expand Up @@ -115,7 +120,12 @@ private function getTableStructure(Cell $cell): void
$this->totalsRow = ($this->table->getShowTotalsRow()) ? (int) $tableRange[1][1] : null;
$this->lastDataRow = ($this->table->getShowTotalsRow()) ? (int) $tableRange[1][1] - 1 : $tableRange[1][1];

$this->columns = $this->getColumns($cell, $tableRange);
$cellParam = $cell;
$worksheet = $this->table->getWorksheet();
if ($worksheet !== null && $worksheet !== $cell->getWorksheet()) {
$cellParam = $worksheet->getCell('A1');
}
$this->columns = $this->getColumns($cellParam, $tableRange);
}

/**
Expand Down Expand Up @@ -146,6 +156,13 @@ private function getTableByName(Cell $cell): Table
{
$table = $cell->getWorksheet()->getTableByName($this->tableName);

if ($table === null) {
$spreadsheet = $cell->getWorksheet()->getParent();
if ($spreadsheet !== null) {
$table = $spreadsheet->getTableByName($this->tableName);
}
}

if ($table === null) {
throw new Exception("Table {$this->tableName} for Structured Reference cannot be located");
}
Expand Down Expand Up @@ -324,7 +341,7 @@ private function getColumnsForColumnReference(string $reference, int $startRow,
{
$columnsSelected = false;
foreach ($this->columns as $columnId => $columnName) {
$columnName = str_replace("\u{a0}", ' ', $columnName);
$columnName = str_replace("\u{a0}", ' ', $columnName ?? '');
$cellFrom = "{$columnId}{$startRow}";
$cellTo = "{$columnId}{$endRow}";
$cellReference = ($cellFrom === $cellTo) ? $cellFrom : "{$cellFrom}:{$cellTo}";
Expand All @@ -341,4 +358,9 @@ private function getColumnsForColumnReference(string $reference, int $startRow,

return $reference;
}

public function __toString(): string
{
return $this->value;
}
}
14 changes: 14 additions & 0 deletions src/PhpSpreadsheet/Spreadsheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
use PhpOffice\PhpSpreadsheet\Style\Style;
use PhpOffice\PhpSpreadsheet\Worksheet\Iterator;
use PhpOffice\PhpSpreadsheet\Worksheet\Table;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;

Expand Down Expand Up @@ -1685,4 +1686,17 @@ public function resetThemeFonts(): void
}
}
}

public function getTableByName(string $tableName): ?Table
{
$table = null;
foreach ($this->workSheetCollection as $sheet) {
$table = $sheet->getTableByName($tableName);
if ($table !== null) {
break;
}
}

return $table;
}
}
1 change: 1 addition & 0 deletions src/PhpSpreadsheet/Worksheet/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ public function __destruct()

$this->disconnectCells();
$this->rowDimensions = [];
//$this->removeTableCollection(); // problem with phpunit10
}

/**
Expand Down
68 changes: 68 additions & 0 deletions tests/PhpSpreadsheetTests/Worksheet/Table/Issue3635Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Worksheet\Table;

use PhpOffice\PhpSpreadsheet\Worksheet\Table;

class Issue3635Test extends SetupTeardown
{
public function testNewExample(): void
{
$sheet = $this->getSheet();
$sheet->setTitle('Feuil1');
$sheet->fromArray(
[
['Numbers'],
[1],
[2],
[3],
[4],
[5],
],
null,
'B2',
true
);
$table = new Table('B2:B7', 'Tableau2');
$sheet->addTable($table);
$sheet->getCell('E2')->setValue('=IF(E3>0,SUM(SUM(Tableau2),SUM(Tableau2)),"NOPE")');
$sheet->getCell('G2')->setValue('=IF(G3>0,SUM(SUM(Tableau2),SUM(Tableau2)),"NOPE")');
$sheet->getCell('G3')->setValue(1);
self::assertSame('NOPE', $sheet->getCell('E2')->getCalculatedValue());
self::assertSame(30, $sheet->getCell('G2')->getCalculatedValue());
}

public function testOriginalExample(): void
{
$sheet = $this->getSheet();
$formula = '=IF([@BAUTEIL]="EK","Entrauchungsklappe",(IF([@BAUTEIL]="VE","Entrauchungsventilator",(IF([@BAUTEIL]="JK","Jalousieklappe",(IF([@BAUTEIL]="LK","Lichtkuppel","ok")))))))';
$sheet->fromArray(
[
['BAUTEIL', 'Result'],
['EK', $formula],
['VE', $formula],
['EK', $formula],
['JK', $formula],
['LK', $formula],
['None', $formula],
[null, $formula],
[null, $formula],
[null, $formula],
],
null,
'A1',
true
);
$table = new Table('A1:B10', 'Tableau3');
$sheet->addTable($table);
self::assertSame('Entrauchungsklappe', $sheet->getCell('B2')->getCalculatedValue());
self::assertSame('Entrauchungsventilator', $sheet->getCell('B3')->getCalculatedValue());
self::assertSame('Entrauchungsklappe', $sheet->getCell('B4')->getCalculatedValue());
self::assertSame('Jalousieklappe', $sheet->getCell('B5')->getCalculatedValue());
self::assertSame('Lichtkuppel', $sheet->getCell('B6')->getCalculatedValue());
self::assertSame('ok', $sheet->getCell('B7')->getCalculatedValue());
self::assertSame('ok', $sheet->getCell('B8')->getCalculatedValue());
self::assertSame('ok', $sheet->getCell('B9')->getCalculatedValue());
self::assertSame('ok', $sheet->getCell('B10')->getCalculatedValue());
}
}
45 changes: 45 additions & 0 deletions tests/PhpSpreadsheetTests/Worksheet/Table/Issue3659Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Worksheet\Table;

use PhpOffice\PhpSpreadsheet\Worksheet\Table;

class Issue3659Test extends SetupTeardown
{
public function testTableOnOtherSheet(): void
{
$spreadsheet = $this->getSpreadsheet();
$sheet = $this->getSheet();
$sheet->setTitle('Feuil1');
$tableSheet = $spreadsheet->createSheet();
$tableSheet->setTitle('sheet_with_table');
$tableSheet->fromArray(
[
['MyCol', 'Colonne2', 'Colonne3'],
[10, 20],
[2],
[3],
[4],
],
null,
'B1',
true
);
$table = new Table('B1:D5', 'Tableau1');
$tableSheet->addTable($table);
$sheet->setSelectedCells('F7');
$tableSheet->setSelectedCells('F8');
self::assertSame($sheet, $spreadsheet->getActiveSheet());
$sheet->getCell('A1')->setValue('=SUM(Tableau1[MyCol])');
$sheet->getCell('A2')->setValue('=SUM(Tableau1[])');
$sheet->getCell('A3')->setValue('=SUM(Tableau1)');
$sheet->getCell('A4')->setValue('=CONCAT(Tableau1)');
self::assertSame(19, $sheet->getCell('A1')->getCalculatedValue());
self::assertSame(39, $sheet->getCell('A2')->getCalculatedValue());
self::assertSame(39, $sheet->getCell('A3')->getCalculatedValue());
self::assertSame('1020234', $sheet->getCell('A4')->getCalculatedValue(), 'Header row not included');
self::assertSame('F7', $sheet->getSelectedCells());
self::assertSame('F8', $tableSheet->getSelectedCells());
self::assertSame($sheet, $spreadsheet->getActiveSheet());
}
}

0 comments on commit 4971801

Please sign in to comment.