diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 9ed4b41b25..b95857b5c2 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -116,7 +116,7 @@ 'no_unneeded_final_method' => true, 'no_unreachable_default_argument_value' => true, 'no_unset_cast' => true, - 'no_unset_on_property' => true, + 'no_unset_on_property' => false, 'no_unused_imports' => true, 'no_useless_else' => true, 'no_useless_return' => true, diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eae723305..bc71585cf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com) and this project adheres to [Semantic Versioning](https://semver.org). +# TBD - 1.29.8 + +### Deprecated + +- Worksheet::getHashCode is no longer needed. + +### Fixed + +- Change hash code for worksheet. Backport of [PR #4207](https://github.com/PHPOffice/PhpSpreadsheet/pull/4207) + # 2024-12-26 - 1.29.7 ### Deprecated diff --git a/src/PhpSpreadsheet/ReferenceHelper.php b/src/PhpSpreadsheet/ReferenceHelper.php index 90eee534db..8cd0c6de45 100644 --- a/src/PhpSpreadsheet/ReferenceHelper.php +++ b/src/PhpSpreadsheet/ReferenceHelper.php @@ -913,7 +913,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet { $cellAddress = $definedName->getValue(); $asFormula = ($cellAddress[0] === '='); - if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashInt() === $worksheet->getHashInt()) { /** * If we delete the entire range that is referenced by a Named Range, MS Excel sets the value to #REF! * PhpSpreadsheet still only does a basic adjustment, so the Named Range will still reference Cells. @@ -932,7 +932,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet private function updateNamedFormula(DefinedName $definedName, Worksheet $worksheet, string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): void { - if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) { + if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashInt() === $worksheet->getHashInt()) { /** * If we delete the entire range that is referenced by a Named Formula, MS Excel sets the value to #REF! * PhpSpreadsheet still only does a basic adjustment, so the Named Formula will still reference Cells. diff --git a/src/PhpSpreadsheet/Spreadsheet.php b/src/PhpSpreadsheet/Spreadsheet.php index 110908550f..e4838bc29f 100644 --- a/src/PhpSpreadsheet/Spreadsheet.php +++ b/src/PhpSpreadsheet/Spreadsheet.php @@ -750,13 +750,17 @@ public function getSheetByNameOrThrow(string $worksheetName): Worksheet * * @return int index */ - public function getIndex(Worksheet $worksheet) + public function getIndex(Worksheet $worksheet, bool $noThrow = false) { + $wsHash = $worksheet->getHashInt(); foreach ($this->workSheetCollection as $key => $value) { - if ($value->getHashCode() === $worksheet->getHashCode()) { + if ($value->getHashInt() === $wsHash) { return $key; } } + if ($noThrow) { + return -1; + } throw new Exception('Sheet does not exist.'); } diff --git a/src/PhpSpreadsheet/Worksheet/BaseDrawing.php b/src/PhpSpreadsheet/Worksheet/BaseDrawing.php index da229c3b1c..6396ac7c62 100644 --- a/src/PhpSpreadsheet/Worksheet/BaseDrawing.php +++ b/src/PhpSpreadsheet/Worksheet/BaseDrawing.php @@ -444,7 +444,7 @@ public function getHashCode() return md5( $this->name . $this->description . - (($this->worksheet === null) ? '' : $this->worksheet->getHashCode()) . + (($this->worksheet === null) ? '' : (string) $this->worksheet->getHashInt()) . $this->coordinates . $this->offsetX . $this->offsetY . diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 29221e9910..caa86ec3ec 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -20,7 +20,6 @@ use PhpOffice\PhpSpreadsheet\Comment; use PhpOffice\PhpSpreadsheet\DefinedName; use PhpOffice\PhpSpreadsheet\Exception; -use PhpOffice\PhpSpreadsheet\IComparable; use PhpOffice\PhpSpreadsheet\ReferenceHelper; use PhpOffice\PhpSpreadsheet\RichText\RichText; use PhpOffice\PhpSpreadsheet\Shared; @@ -31,7 +30,7 @@ use PhpOffice\PhpSpreadsheet\Style\NumberFormat; use PhpOffice\PhpSpreadsheet\Style\Style; -class Worksheet implements IComparable +class Worksheet { // Break types public const BREAK_NONE = 0; @@ -338,17 +337,10 @@ class Worksheet implements IComparable */ private $tabColor; - /** - * Dirty flag. - * - * @var bool - */ - private $dirty = true; - /** * Hash. * - * @var string + * @var int */ private $hash; @@ -368,6 +360,7 @@ public function __construct(?Spreadsheet $parent = null, $title = 'Worksheet') { // Set parent and title $this->parent = $parent; + $this->hash = spl_object_id($this); $this->setTitle($title, false); // setTitle can change $pTitle $this->setCodeName($this->getTitle()); @@ -424,6 +417,12 @@ public function __destruct() $this->rowDimensions = []; } + public function __wakeup(): void + { + $this->hash = spl_object_id($this); + $this->parent = null; + } + /** * Return the cell collection. * @@ -943,7 +942,6 @@ public function setTitle($title, $updateFormulaCellReferences = true, $validate // Set title $this->title = $title; - $this->dirty = true; if ($this->parent && $this->parent->getCalculationEngine()) { // New title @@ -1088,7 +1086,6 @@ public function getProtection() public function setProtection(Protection $protection) { $this->protection = $protection; - $this->dirty = true; return $this; } @@ -3137,7 +3134,7 @@ private function validateNamedRange(string $definedName, bool $returnNullIfInval if ($namedRange->getLocalOnly()) { $worksheet = $namedRange->getWorksheet(); - if ($worksheet === null || $this->getHashCode() !== $worksheet->getHashCode()) { + if ($worksheet === null || $this->getHashInt() !== $worksheet->getHashInt()) { if ($returnNullIfInvalid) { return null; } @@ -3278,17 +3275,20 @@ public function garbageCollect() } /** - * Get hash code. + * @deprecated 3.5.0 use getHashInt instead. * * @return string Hash code */ public function getHashCode() { - if ($this->dirty) { - $this->hash = md5($this->title . $this->autoFilter . ($this->protection->isProtectionEnabled() ? 't' : 'f') . __CLASS__); - $this->dirty = false; - } + return (string) $this->hash; + } + /** + * @return int Hash code + */ + public function getHashInt() + { return $this->hash; } @@ -3620,6 +3620,7 @@ public function __clone() } } } + $this->hash = spl_object_id($this); } /** diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Style.php b/src/PhpSpreadsheet/Writer/Xlsx/Style.php index baafdc334d..7504e9e30e 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Style.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Style.php @@ -142,6 +142,7 @@ public function writeStyles(Spreadsheet $spreadsheet) // dxf for ($i = 0; $i < $this->getParentWriter()->getStylesConditionalHashTable()->count(); ++$i) { + /** @var ?Conditional */ $thisstyle = $this->getParentWriter()->getStylesConditionalHashTable()->getByIndex($i); if ($thisstyle !== null) { $this->writeCellStyleDxf($objWriter, $thisstyle->getStyle()); diff --git a/tests/PhpSpreadsheetTests/Shared/DateTest.php b/tests/PhpSpreadsheetTests/Shared/DateTest.php index 9bcd6ff296..2e0effaea1 100644 --- a/tests/PhpSpreadsheetTests/Shared/DateTest.php +++ b/tests/PhpSpreadsheetTests/Shared/DateTest.php @@ -5,6 +5,7 @@ use DateTimeZone; use PhpOffice\PhpSpreadsheet\Exception; use PhpOffice\PhpSpreadsheet\Shared\Date; +use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Style\NumberFormat; use PHPUnit\Framework\TestCase; @@ -230,7 +231,7 @@ public function testVarious(): void $date = Date::PHPToExcel('2020-01-01'); self::assertEquals(43831.0, $date); - $spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet(); + $spreadsheet = new Spreadsheet(); $sheet = $spreadsheet->getActiveSheet(); $sheet->setCellValue('B1', 'x'); $val = $sheet->getCell('B1')->getValue(); @@ -271,5 +272,6 @@ public function testVarious(): void ->getNumberFormat() ->setFormatCode('yyyy-mm-dd'); self::assertFalse(Date::isDateTime($cella4)); + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Shared/StringHelperInvalidCharTest.php b/tests/PhpSpreadsheetTests/Shared/StringHelperInvalidCharTest.php index 5467949993..82b5cade6d 100644 --- a/tests/PhpSpreadsheetTests/Shared/StringHelperInvalidCharTest.php +++ b/tests/PhpSpreadsheetTests/Shared/StringHelperInvalidCharTest.php @@ -40,5 +40,6 @@ public function testInvalidChar(): void $sheet->getCell("A$row")->getValue() ); } + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Style/BorderRangeTest.php b/tests/PhpSpreadsheetTests/Style/BorderRangeTest.php index aec5520d93..31f5b74d92 100644 --- a/tests/PhpSpreadsheetTests/Style/BorderRangeTest.php +++ b/tests/PhpSpreadsheetTests/Style/BorderRangeTest.php @@ -60,6 +60,7 @@ public function testBorderRangeInAction(): void } } } + $spreadsheet->disconnectWorksheets(); } public function testBorderRangeDirectly(): void @@ -69,5 +70,6 @@ public function testBorderRangeDirectly(): void $sheet = $spreadsheet->getActiveSheet(); $style = $sheet->getStyle('A1:C1')->getBorders()->getTop()->setBorderStyle(Border::BORDER_THIN); self::assertSame('A1:C1', $style->getSelectedCells(), 'getSelectedCells should not change after a style operation on a border range'); + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Style/BorderTest.php b/tests/PhpSpreadsheetTests/Style/BorderTest.php index cf6cd3a964..b1bda09548 100644 --- a/tests/PhpSpreadsheetTests/Style/BorderTest.php +++ b/tests/PhpSpreadsheetTests/Style/BorderTest.php @@ -29,6 +29,7 @@ public function testAllBorders(): void self::assertSame(Border::BORDER_THIN, $borders->getRight()->getBorderStyle()); self::assertSame(Border::BORDER_THIN, $borders->getLeft()->getBorderStyle()); self::assertSame(Border::BORDER_NONE, $borders->getDiagonal()->getBorderStyle()); + $spreadsheet->disconnectWorksheets(); } public function testAllBordersArray(): void @@ -43,6 +44,7 @@ public function testAllBordersArray(): void self::assertSame(Border::BORDER_THIN, $borders->getRight()->getBorderStyle()); self::assertSame(Border::BORDER_THIN, $borders->getLeft()->getBorderStyle()); self::assertSame(Border::BORDER_NONE, $borders->getDiagonal()->getBorderStyle()); + $spreadsheet->disconnectWorksheets(); } public function testAllBordersArrayNotSupervisor(): void @@ -84,6 +86,7 @@ public function testOutline(): void self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle()); self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle()); self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle()); + $spreadsheet->disconnectWorksheets(); } public function testInside(): void @@ -113,6 +116,7 @@ public function testInside(): void self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle()); self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle()); self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle()); + $spreadsheet->disconnectWorksheets(); } public function testHorizontal(): void @@ -142,6 +146,7 @@ public function testHorizontal(): void self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle()); self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle()); self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle()); + $spreadsheet->disconnectWorksheets(); } public function testVertical(): void @@ -171,6 +176,7 @@ public function testVertical(): void self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle()); self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle()); self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle()); + $spreadsheet->disconnectWorksheets(); } public function testNoSupervisorAllBorders(): void @@ -215,6 +221,7 @@ public function testGetSharedComponentPseudo(): void $sheet = $spreadsheet->getActiveSheet(); $sheet->getStyle('A1')->getBorders()->getHorizontal()->setBorderStyle(Border::BORDER_MEDIUM); $sheet->getStyle('A1')->getBorders()->getHorizontal()->getSharedComponent(); + $spreadsheet->disconnectWorksheets(); } public function testBorderStyle(): void @@ -231,6 +238,7 @@ public function testBorderStyle(): void $border->setBorderStyle(Border::BORDER_THIN)->setColor(new Color('FFFF0000')); self::assertEquals('FFFF0000', $border->getColor()->getARGB()); self::assertEquals(Border::BORDER_THIN, $border->getBorderStyle()); + $spreadsheet->disconnectWorksheets(); } public function testDiagonalDirection(): void @@ -243,5 +251,6 @@ public function testDiagonalDirection(): void self::assertSame(Border::BORDER_MEDIUM, $borders->getDiagonal()->getBorderStyle()); self::assertSame(Borders::DIAGONAL_BOTH, $borders->getDiagonalDirection()); + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Style/ConditionalFormatting/Wizard/WizardFactoryTest.php b/tests/PhpSpreadsheetTests/Style/ConditionalFormatting/Wizard/WizardFactoryTest.php index 06b900c07e..289bbbbfce 100644 --- a/tests/PhpSpreadsheetTests/Style/ConditionalFormatting/Wizard/WizardFactoryTest.php +++ b/tests/PhpSpreadsheetTests/Style/ConditionalFormatting/Wizard/WizardFactoryTest.php @@ -67,6 +67,7 @@ public function testWizardFromConditional(string $sheetName, string $cellAddress $wizard = Wizard::fromConditional($conditional); self::assertEquals($expectedWizads[$index], get_class($wizard)); } + $spreadsheet->disconnectWorksheets(); } public static function conditionalProvider(): array diff --git a/tests/PhpSpreadsheetTests/Style/StyleTest.php b/tests/PhpSpreadsheetTests/Style/StyleTest.php index 13998546aa..45289933ed 100644 --- a/tests/PhpSpreadsheetTests/Style/StyleTest.php +++ b/tests/PhpSpreadsheetTests/Style/StyleTest.php @@ -22,6 +22,7 @@ public function testStyleOddMethods(): void $styleArray = ['alignment' => ['textRotation' => 45]]; $outArray = $cell1style->getStyleArray($styleArray); self::assertEquals($styleArray, $outArray['quotePrefix']); + $spreadsheet->disconnectWorksheets(); } public function testStyleColumn(): void @@ -56,6 +57,7 @@ public function testStyleColumn(): void self::assertTrue($sheet->getStyle('A1')->getFont()->getItalic()); self::assertTrue($sheet->getStyle('B2')->getFont()->getItalic()); self::assertFalse($sheet->getStyle('C3')->getFont()->getItalic()); + $spreadsheet->disconnectWorksheets(); } public function testStyleIsReused(): void @@ -79,6 +81,7 @@ public function testStyleIsReused(): void $spreadsheet->garbageCollect(); self::assertCount(3, $spreadsheet->getCellXfCollection()); + $spreadsheet->disconnectWorksheets(); } public function testStyleRow(): void @@ -113,6 +116,7 @@ public function testStyleRow(): void self::assertFalse($sheet->getStyle('A1')->getFont()->getItalic()); self::assertTrue($sheet->getStyle('B2')->getFont()->getItalic()); self::assertTrue($sheet->getStyle('C3')->getFont()->getItalic()); + $spreadsheet->disconnectWorksheets(); } public function testIssue1712A(): void @@ -135,6 +139,7 @@ public function testIssue1712A(): void ->setRGB($rgb); self::assertEquals($rgb, $sheet->getCell('A1')->getStyle()->getFill()->getStartColor()->getRGB()); self::assertEquals($rgb, $sheet->getCell('B1')->getStyle()->getFill()->getStartColor()->getRGB()); + $spreadsheet->disconnectWorksheets(); } public function testIssue1712B(): void @@ -157,6 +162,7 @@ public function testIssue1712B(): void $sheet->fromArray(['OK', 'KO']); self::assertEquals($rgb, $sheet->getCell('A1')->getStyle()->getFill()->getStartColor()->getRGB()); self::assertEquals($rgb, $sheet->getCell('B1')->getStyle()->getFill()->getStartColor()->getRGB()); + $spreadsheet->disconnectWorksheets(); } public function testStyleLoopUpwards(): void @@ -182,6 +188,7 @@ public function testStyleLoopUpwards(): void self::assertFalse($sheet->getStyle('A1')->getFont()->getBold()); self::assertFalse($sheet->getStyle('B2')->getFont()->getBold()); self::assertTrue($sheet->getStyle('C3')->getFont()->getBold()); + $spreadsheet->disconnectWorksheets(); } public function testStyleCellAddressObject(): void @@ -193,6 +200,7 @@ public function testStyleCellAddressObject(): void $style->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_DATE_YYYYMMDDSLASH); self::assertSame(NumberFormat::FORMAT_DATE_YYYYMMDDSLASH, $style->getNumberFormat()->getFormatCode()); + $spreadsheet->disconnectWorksheets(); } public function testStyleCellRangeObject(): void @@ -206,5 +214,6 @@ public function testStyleCellRangeObject(): void $style->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_DATE_YYYYMMDDSLASH); self::assertSame(NumberFormat::FORMAT_DATE_YYYYMMDDSLASH, $style->getNumberFormat()->getFormatCode()); + $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoSizeTest.php b/tests/PhpSpreadsheetTests/Worksheet/AutoSizeTest.php index e774b04cc2..b4fda314fe 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoSizeTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoSizeTest.php @@ -11,16 +11,14 @@ class AutoSizeTest extends TestCase { - protected Spreadsheet $spreadsheet; + private Spreadsheet $spreadsheet; - protected Worksheet $worksheet; + private Worksheet $worksheet; protected function setUp(): void { - parent::setUp(); - - $spreadsheet = new Spreadsheet(); - $this->worksheet = $spreadsheet->getActiveSheet(); + $this->spreadsheet = new Spreadsheet(); + $this->worksheet = $this->spreadsheet->getActiveSheet(); $this->worksheet->setCellValue('A1', 'YEAR') ->setCellValue('B1', 'QUARTER') @@ -42,7 +40,13 @@ protected function setUp(): void } } - protected function setTable(): Table + protected function tearDown(): void + { + $this->spreadsheet->disconnectWorksheets(); + unset($this->spreadsheet, $this->worksheet); + } + + private function setTable(): Table { $table = new Table('A1:D5', 'Sales_Data'); $tableStyle = new TableStyle(); @@ -53,7 +57,7 @@ protected function setTable(): Table return $table; } - protected function readColumnSizes(): array + private function readColumnSizes(): array { $columnSizes = []; $toColumn = $this->worksheet->getHighestColumn(); diff --git a/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php b/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php new file mode 100644 index 0000000000..b66a9dd8de --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php @@ -0,0 +1,65 @@ +getActiveSheet(); + $sheet1->getCell('A1')->setValue(10); + $sheet2 = clone $sheet1; + $sheet2->getCell('A1')->setValue(20); + self::assertSame(0, $spreadsheet->getIndex($sheet1)); + $idx = $spreadsheet->getIndex($sheet2, true); + self::assertSame(-1, $idx); + $sheet2->setTitle('clone'); + $spreadsheet->addSheet($sheet2); + $idx = $spreadsheet->getIndex($sheet2, true); + self::assertSame(1, $idx); + self::assertSame(10, $spreadsheet->getSheet(0)->getCell('A1')->getValue()); + self::assertSame(20, $spreadsheet->getSheet(1)->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); + } + + public function testGetCloneIndex(): void + { + $this->expectException(SpreadsheetException::class); + $this->expectExceptionMessage('Sheet does not exist'); + $spreadsheet = new Spreadsheet(); + $sheet1 = $spreadsheet->getActiveSheet(); + $sheet1->getCell('A1')->setValue(10); + $sheet2 = clone $sheet1; + $spreadsheet->getSheet($spreadsheet->getIndex($sheet2)) + ->setCellValue('A1', 100); + } + + public function testSerialize1(): void + { + // If worksheet attached to spreadsheet, can't serialize it. + $this->expectException(SpreadsheetException::class); + $this->expectExceptionMessage('cannot be serialized'); + $spreadsheet = new Spreadsheet(); + $sheet1 = $spreadsheet->getActiveSheet(); + serialize($sheet1); + } + + public function testSerialize2(): void + { + $sheet1 = new Worksheet(); + $sheet1->getCell('A1')->setValue(10); + $serialized = serialize($sheet1); + /** @var Worksheet */ + $newSheet = unserialize($serialized); + self::assertSame(10, $newSheet->getCell('A1')->getValue()); + self::assertNotEquals($newSheet->getHashInt(), $sheet1->getHashInt()); + } +}