Change hash code for worksheet

Backport of [PR PHPOffice#4207](PHPOffice#4207)
oleibman · Jan 7, 2025 · b5ef72c · b5ef72c
1 parent 51635f6
commit b5ef72c
Show file tree

Hide file tree

Showing 15 changed files with 142 additions and 33 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -40,5 +40,6 @@ public function testInvalidChar(): void @@
                     $sheet->getCell("A$row")->getValue()
                 );
             }
+            $spreadsheet->disconnectWorksheets();
         }
     }