Break Some More Circular References (#3716)

* Break Some More Circular References

After identifying a circular reference between Worksheet and Table, which could lead to memory leaks, I identified several other similar relationships. I have added a destructor, or added added code to an existing destructor, for those possibilities. This didn't actually lead to any lowering of the memory usage of the unit test suite, so maybe there's no practical benefit, but it does seem like it should be theoretically useful. I am not aware of any practical way to unit test a destructor, although the added code will be exercised many times in most of our unit tests.

The only change here not involving a destructor is to `Worksheet::addChart` (it came to my attention because Worksheet and Chart have a circular reference). It is currently defined with two arguments, the second allowing you to place the chart in a specific location in the containing ArrayObject. However, as Phpstan and Scrutinizer both warn, the code to support this option will fail if it is ever executed (can't use array_splice on an ArrayObject); needless to say, this possibility is not exercised when executing the test suite. It could be made to work, but I really can't see any kind of use case to support this parameter. So I'm removing the second parameter. In theory, this is a breaking change, but, since the code can never work, in practice it won't be.

* Update Worksheet.php

* Update Worksheet.php

src/PhpSpreadsheet/Cell/CellAddress.php

@@ -25,6 +25,11 @@ class CellAddress implements Stringable
         $this->worksheet = $worksheet;
     }
 
+    public function __destruct()
+    {
+        $this->worksheet = null;
+    }
+
     private static function validateColumnAndRow(mixed $columnId, mixed $rowId): void
     {
         if (!is_numeric($columnId) || $columnId <= 0 || !is_numeric($rowId) || $rowId <= 0) {

src/PhpSpreadsheet/Cell/ColumnRange.php

@@ -22,6 +22,11 @@ class ColumnRange implements AddressRange, Stringable
         $this->worksheet = $worksheet;
     }
 
+    public function __destruct()
+    {
+        $this->worksheet = null;
+    }
+
     public static function fromColumnIndexes(int $from, int $to, ?Worksheet $worksheet = null): self
     {
         return new self(Coordinate::stringFromColumnIndex($from), Coordinate::stringFromColumnIndex($to), $worksheet);

src/PhpSpreadsheet/Cell/RowRange.php

@@ -19,6 +19,11 @@ class RowRange implements AddressRange, Stringable
         $this->worksheet = $worksheet;
     }
 
+    public function __destruct()
+    {
+        $this->worksheet = null;
+    }
+
     public static function fromArray(array $array, ?Worksheet $worksheet = null): self
     {
         [$from, $to] = $array;

src/PhpSpreadsheet/Chart/Chart.php

@@ -160,6 +160,11 @@ class Chart
         $this->borderLines = new GridLines();
     }
 
+    public function __destruct()
+    {
+        $this->worksheet = null;
+    }
+
     /**
      * Get Name.
      *

src/PhpSpreadsheet/Collection/Cells.php

@@ -453,6 +453,7 @@ class Cells
     public function __destruct()
     {
         $this->cache->deleteMultiple($this->getAllCacheKeys());
+        $this->parent = null;
     }
 
     /**

src/PhpSpreadsheet/DefinedName.php

@@ -66,6 +66,12 @@ abstract class DefinedName
         $this->isFormula = self::testIfFormula($this->value);
     }
 
+    public function __destruct()
+    {
+        $this->worksheet = null;
+        $this->scope = null;
+    }
+
     /**
      * Create a new defined name, either a range or a formula.
      */

src/PhpSpreadsheet/Spreadsheet.php

@@ -500,6 +500,7 @@ class Spreadsheet implements JsonSerializable
         $this->calculationEngine = null;
         $this->cellXfCollection = [];
         $this->cellStyleXfCollection = [];
+        $this->definedNames = [];
     }
 
     /**

src/PhpSpreadsheet/Worksheet/AutoFilter.php

@@ -64,6 +64,11 @@ class AutoFilter implements Stringable
         $this->workSheet = $worksheet;
     }
 
+    public function __destruct()
+    {
+        $this->workSheet = null;
+    }
+
     /**
      * Get AutoFilter Parent Worksheet.
      *

src/PhpSpreadsheet/Worksheet/BaseDrawing.php

@@ -164,6 +164,11 @@ class BaseDrawing implements IComparable
         $this->imageIndex = self::$imageCounter;
     }
 
+    public function __destruct()
+    {
+        $this->worksheet = null;
+    }
+
     public function getImageIndex(): int
     {
         return $this->imageIndex;

src/PhpSpreadsheet/Worksheet/MemoryDrawing.php

@@ -66,6 +66,7 @@ class MemoryDrawing extends BaseDrawing
             @imagedestroy($this->imageResource);
             $this->imageResource = null;
         }
+        $this->worksheet = null;
     }
 
     public function __clone()

src/PhpSpreadsheet/Worksheet/Worksheet.php

@@ -423,8 +423,7 @@ class Worksheet implements IComparable
         Calculation::getInstance($this->parent)->clearCalculationCacheForWorksheet($this->title);
 
         $this->disconnectCells();
-        $this->rowDimensions = [];
-        $this->tableCollection = new ArrayObject();
+        unset($this->rowDimensions, $this->columnDimensions, $this->tableCollection, $this->drawingCollection, $this->chartCollection, $this->autoFilter);
     }
 
     /**
@@ -589,21 +588,10 @@ class Worksheet implements IComparable
         return $this->chartCollection;
     }
 
-    /**
-     * Add chart.
-     *
-     * @param null|int $chartIndex Index where chart should go (0,1,..., or null for last)
-     */
-    public function addChart(Chart $chart, $chartIndex = null): Chart
+    public function addChart(Chart $chart): Chart
     {
         $chart->setWorksheet($this);
-        if ($chartIndex === null) {
-            $this->chartCollection[] = $chart;
-        } else {
-            // Insert the chart at the requested index
-            // @phpstan-ignore-next-line
-            array_splice(/** @scrutinizer ignore-type */ $this->chartCollection, $chartIndex, 0, [$chart]);
-        }
+        $this->chartCollection[] = $chart;
 
         return $chart;
     }