Deleting Sheet with Local Defined Name (#2284)
Fixes issue #2266. Writer/Xlsx fails when there is no longer a sheet which corresponds to the definition of a local defined name. The code is changed to drop such an orphaned name. Writer/Xls does not fail under the same cicrcumstances, so no correction is needed there. Writer/Ods fails in a different manner, and is corrected to no longer do so.
This commit is contained in:
parent
e02eab29f1
commit
4dd5c06c7b
|
|
@ -5275,31 +5275,6 @@ parameters:
|
|||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Ods/Formula.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Ods\\\\NamedExpressions\\:\\:\\$objWriter has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Ods/NamedExpressions.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Ods\\\\NamedExpressions\\:\\:\\$spreadsheet has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Ods/NamedExpressions.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Ods\\\\NamedExpressions\\:\\:\\$formulaConvertor has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Ods/NamedExpressions.php
|
||||
|
||||
-
|
||||
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Ods\\\\NamedExpressions\\:\\:__construct\\(\\) has parameter \\$formulaConvertor with no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Ods/NamedExpressions.php
|
||||
|
||||
-
|
||||
message: "#^Cannot call method getTitle\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#"
|
||||
count: 3
|
||||
path: src/PhpSpreadsheet/Writer/Ods/NamedExpressions.php
|
||||
|
||||
-
|
||||
message: "#^Parameter \\#1 \\$content of method XMLWriter\\:\\:text\\(\\) expects string, int given\\.$#"
|
||||
count: 2
|
||||
|
|
@ -5860,21 +5835,6 @@ parameters:
|
|||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Xlsx\\\\DefinedNames\\:\\:\\$objWriter has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Xlsx/DefinedNames.php
|
||||
|
||||
-
|
||||
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Writer\\\\Xlsx\\\\DefinedNames\\:\\:\\$spreadsheet has no typehint specified\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Xlsx/DefinedNames.php
|
||||
|
||||
-
|
||||
message: "#^Cannot call method getTitle\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Xlsx/DefinedNames.php
|
||||
|
||||
-
|
||||
message: "#^Parameter \\#2 \\$content of method XMLWriter\\:\\:writeElement\\(\\) expects string\\|null, int given\\.$#"
|
||||
count: 1
|
||||
|
|
|
|||
|
|
@ -10,13 +10,16 @@ use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
|
|||
|
||||
class NamedExpressions
|
||||
{
|
||||
/** @var XMLWriter */
|
||||
private $objWriter;
|
||||
|
||||
/** @var Spreadsheet */
|
||||
private $spreadsheet;
|
||||
|
||||
/** @var Formula */
|
||||
private $formulaConvertor;
|
||||
|
||||
public function __construct(XMLWriter $objWriter, Spreadsheet $spreadsheet, $formulaConvertor)
|
||||
public function __construct(XMLWriter $objWriter, Spreadsheet $spreadsheet, Formula $formulaConvertor)
|
||||
{
|
||||
$this->objWriter = $objWriter;
|
||||
$this->spreadsheet = $spreadsheet;
|
||||
|
|
@ -51,23 +54,29 @@ class NamedExpressions
|
|||
|
||||
private function writeNamedFormula(DefinedName $definedName, Worksheet $defaultWorksheet): void
|
||||
{
|
||||
$title = ($definedName->getWorksheet() !== null) ? $definedName->getWorksheet()->getTitle() : $defaultWorksheet->getTitle();
|
||||
$this->objWriter->writeAttribute('table:name', $definedName->getName());
|
||||
$this->objWriter->writeAttribute(
|
||||
'table:expression',
|
||||
$this->formulaConvertor->convertFormula($definedName->getValue(), $definedName->getWorksheet()->getTitle())
|
||||
$this->formulaConvertor->convertFormula($definedName->getValue(), $title)
|
||||
);
|
||||
$this->objWriter->writeAttribute('table:base-cell-address', $this->convertAddress(
|
||||
$definedName,
|
||||
"'" . (($definedName->getWorksheet() !== null) ? $definedName->getWorksheet()->getTitle() : $defaultWorksheet->getTitle()) . "'!\$A\$1"
|
||||
"'" . $title . "'!\$A\$1"
|
||||
));
|
||||
}
|
||||
|
||||
private function writeNamedRange(DefinedName $definedName): void
|
||||
{
|
||||
$baseCell = '$A$1';
|
||||
$ws = $definedName->getWorksheet();
|
||||
if ($ws !== null) {
|
||||
$baseCell = "'" . $ws->getTitle() . "'!$baseCell";
|
||||
}
|
||||
$this->objWriter->writeAttribute('table:name', $definedName->getName());
|
||||
$this->objWriter->writeAttribute('table:base-cell-address', $this->convertAddress(
|
||||
$definedName,
|
||||
"'" . $definedName->getWorksheet()->getTitle() . "'!\$A\$1"
|
||||
$baseCell
|
||||
));
|
||||
$this->objWriter->writeAttribute('table:cell-range-address', $this->convertAddress($definedName, $definedName->getValue()));
|
||||
}
|
||||
|
|
@ -100,7 +109,10 @@ class NamedExpressions
|
|||
if (empty($worksheet)) {
|
||||
if (($offset === 0) || ($address[$offset - 1] !== ':')) {
|
||||
// We need a worksheet
|
||||
$worksheet = $definedName->getWorksheet()->getTitle();
|
||||
$ws = $definedName->getWorksheet();
|
||||
if ($ws !== null) {
|
||||
$worksheet = $ws->getTitle();
|
||||
}
|
||||
}
|
||||
} else {
|
||||
$worksheet = str_replace("''", "'", trim($worksheet, "'"));
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
namespace PhpOffice\PhpSpreadsheet\Writer\Xlsx;
|
||||
|
||||
use Exception;
|
||||
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
|
||||
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
|
||||
use PhpOffice\PhpSpreadsheet\DefinedName;
|
||||
|
|
@ -11,8 +12,10 @@ use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
|
|||
|
||||
class DefinedNames
|
||||
{
|
||||
/** @var XMLWriter */
|
||||
private $objWriter;
|
||||
|
||||
/** @var Spreadsheet */
|
||||
private $spreadsheet;
|
||||
|
||||
public function __construct(XMLWriter $objWriter, Spreadsheet $spreadsheet)
|
||||
|
|
@ -66,12 +69,22 @@ class DefinedNames
|
|||
private function writeDefinedName(DefinedName $pDefinedName): void
|
||||
{
|
||||
// definedName for named range
|
||||
$local = -1;
|
||||
if ($pDefinedName->getLocalOnly() && $pDefinedName->getScope() !== null) {
|
||||
try {
|
||||
$local = $pDefinedName->getScope()->getParent()->getIndex($pDefinedName->getScope());
|
||||
} catch (Exception $e) {
|
||||
// See issue 2266 - deleting sheet which contains
|
||||
// defined names will cause Exception above.
|
||||
return;
|
||||
}
|
||||
}
|
||||
$this->objWriter->startElement('definedName');
|
||||
$this->objWriter->writeAttribute('name', $pDefinedName->getName());
|
||||
if ($pDefinedName->getLocalOnly() && $pDefinedName->getScope() !== null) {
|
||||
if ($local >= 0) {
|
||||
$this->objWriter->writeAttribute(
|
||||
'localSheetId',
|
||||
$pDefinedName->getScope()->getParent()->getIndex($pDefinedName->getScope())
|
||||
"$local"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -92,7 +105,7 @@ class DefinedNames
|
|||
if (!empty($autoFilterRange)) {
|
||||
$this->objWriter->startElement('definedName');
|
||||
$this->objWriter->writeAttribute('name', '_xlnm._FilterDatabase');
|
||||
$this->objWriter->writeAttribute('localSheetId', $pSheetId);
|
||||
$this->objWriter->writeAttribute('localSheetId', "$pSheetId");
|
||||
$this->objWriter->writeAttribute('hidden', '1');
|
||||
|
||||
// Create absolute coordinate and write as raw text
|
||||
|
|
@ -105,7 +118,7 @@ class DefinedNames
|
|||
$range[1] = Coordinate::absoluteCoordinate($range[1]);
|
||||
$range = implode(':', $range);
|
||||
|
||||
$this->objWriter->writeRawData('\'' . str_replace("'", "''", $pSheet->getTitle()) . '\'!' . $range);
|
||||
$this->objWriter->writeRawData('\'' . str_replace("'", "''", $pSheet->getTitle() ?? '') . '\'!' . $range);
|
||||
|
||||
$this->objWriter->endElement();
|
||||
}
|
||||
|
|
@ -120,7 +133,7 @@ class DefinedNames
|
|||
if ($pSheet->getPageSetup()->isColumnsToRepeatAtLeftSet() || $pSheet->getPageSetup()->isRowsToRepeatAtTopSet()) {
|
||||
$this->objWriter->startElement('definedName');
|
||||
$this->objWriter->writeAttribute('name', '_xlnm.Print_Titles');
|
||||
$this->objWriter->writeAttribute('localSheetId', $pSheetId);
|
||||
$this->objWriter->writeAttribute('localSheetId', "$pSheetId");
|
||||
|
||||
// Setting string
|
||||
$settingString = '';
|
||||
|
|
@ -158,7 +171,7 @@ class DefinedNames
|
|||
if ($pSheet->getPageSetup()->isPrintAreaSet()) {
|
||||
$this->objWriter->startElement('definedName');
|
||||
$this->objWriter->writeAttribute('name', '_xlnm.Print_Area');
|
||||
$this->objWriter->writeAttribute('localSheetId', $pSheetId);
|
||||
$this->objWriter->writeAttribute('localSheetId', "$pSheetId");
|
||||
|
||||
// Print area
|
||||
$printArea = Coordinate::splitRange($pSheet->getPageSetup()->getPrintArea());
|
||||
|
|
@ -205,7 +218,8 @@ class DefinedNames
|
|||
if (empty($worksheet)) {
|
||||
if (($offset === 0) || ($definedRange[$offset - 1] !== ':')) {
|
||||
// We should have a worksheet
|
||||
$worksheet = $pDefinedName->getWorksheet() ? $pDefinedName->getWorksheet()->getTitle() : null;
|
||||
$ws = $pDefinedName->getWorksheet();
|
||||
$worksheet = ($ws === null) ? null : $ws->getTitle();
|
||||
}
|
||||
} else {
|
||||
$worksheet = str_replace("''", "'", trim($worksheet, "'"));
|
||||
|
|
|
|||
|
|
@ -0,0 +1,46 @@
|
|||
<?php
|
||||
|
||||
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
|
||||
|
||||
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as Reader;
|
||||
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
|
||||
|
||||
class Issue2266Test extends AbstractFunctional
|
||||
{
|
||||
/**
|
||||
* @dataProvider providerType
|
||||
*/
|
||||
public function testIssue2266(string $type): void
|
||||
{
|
||||
// Problem deleting sheet containing local defined name.
|
||||
$reader = new Reader();
|
||||
$spreadsheet = $reader->load('tests/data/Writer/XLSX/issue.2266f.xlsx');
|
||||
self::assertCount(2, $spreadsheet->getAllSheets());
|
||||
self::assertCount(1, $spreadsheet->getDefinedNames());
|
||||
$index = 1;
|
||||
$sheet = $spreadsheet->getSheet($index);
|
||||
self::assertSame('Sheet2', $sheet->getTitle());
|
||||
$definedName = $spreadsheet->getDefinedName('LocalName', $sheet);
|
||||
self::assertNotNull($definedName);
|
||||
self::assertTrue($definedName->getLocalOnly());
|
||||
$spreadsheet->removeSheetByIndex($index);
|
||||
|
||||
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $type);
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
|
||||
self::assertCount(1, $reloadedSpreadsheet->getAllSheets());
|
||||
self::assertCount(0, $reloadedSpreadsheet->getDefinedNames());
|
||||
self::assertNotEquals('Sheet2', $reloadedSpreadsheet->getSheet(0)->getTitle());
|
||||
|
||||
$reloadedSpreadsheet->disconnectWorksheets();
|
||||
}
|
||||
|
||||
public function providerType(): array
|
||||
{
|
||||
return [
|
||||
['Xlsx'],
|
||||
['Xls'],
|
||||
['Ods'],
|
||||
];
|
||||
}
|
||||
}
|
||||
Binary file not shown.
Loading…
Reference in New Issue